[Intel-gfx] [PATCH] drm/i915/dp: use logical operators with boolean type

[Paulo Zanoni]

Em qui, 2019-05-02 às 11:29 +0300, Jani Nikula escreveu:
> Using arithmetic operators with booleans is confusing. Switch to logical
> operators.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4e7b8d..ef4992f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5094,7 +5094,7 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
>  	enum port port = intel_dig_port->base.port;
>  	enum tc_port_type old_type = intel_dig_port->tc_type;
>  
> -	WARN_ON(is_legacy + is_typec + is_tbt != 1);
> +	WARN_ON(is_legacy || is_typec || !is_tbt);

This changes the meaning. You're interpreting this as:

    WARN_ON(is_legacy + is_typec + (is_tbt != 1))

while the original intent of the code is to be:

    WARN_ON((is_legacy + is_typec + is_tbt) != 1)

and a quick check on operator precedence tables leads me to think the
original code is indeed correct.

We're asserting exactly one of these bools enabled, so the logic
operation would be something like:

WARN_ON((is_legacy && (is_typec || is_tbt)) ||
        (is_typec && (is_legacy || is_tbt)) ||
 	(is_tbt && (is_legacy || is_typec)) ||
	(!is_legacy && !is_typec && !is_tbt))

I would still prefer the arithmetic operation.    

>  
>  	if (is_legacy)
>  		intel_dig_port->tc_type = TC_PORT_LEGACY;