Re: [PATCH] drm/i915/bxt: Fix wrongly placed ')' in I915_READ()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17/09/15 14:20, Damien Lespiau wrote:
Not the first time! not the last time?

There is a possibility to use gcc 5's -Wbool-compare to try and compare
(reg) in those macros to a constant and gcc will warn that the
comparison between a boolean expression and a constant is always either
true or false. Maybe.

Since boolean true (1) cannot be a valid argument to this macro, it could contain a compile-time check that the parameter is not 1; if boolean false (0) happens not to be a valid register address (BSpec says MMIO 0 is reserved) the macro could check that the argument is neither of these values, and the compiler might then detect that all possible paths lead to a compile-time error. Something like this?

#define I915_READ(reg)                                                 \
        ({                                                             \
            if (__builtin_constant_p(reg)) {                           \
                BUILD_BUG_ON((reg) == false);                          \
                BUILD_BUG_ON((reg) == true);                           \
            }                                                          \
            dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), true);  \
        })

Interestingly, that reported three errors, all in intel_dsi.c where the port-selection macros use 0s to fill in dummy elements when less than 3 ports are being used.

In function ‘intel_dsi_get_hw_state’
In function ‘intel_dsi_port_disable’
In function ‘intel_dsi_port_enable’

Other than that, there weren't any cases where a bool constant was being passed to this specific macro (as of today).

.Dave.

Cc: Imre Deak <imre.deak@xxxxxxxxx>
Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_ddi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 4823184..5b600bf 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2882,7 +2882,7 @@ static bool bxt_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
  	 * here just read out lanes 0/1 and output a note if lanes 2/3 differ.
  	 */
  	hw_state->pcsdw12 = I915_READ(BXT_PORT_PCS_DW12_LN01(port));
-	if (I915_READ(BXT_PORT_PCS_DW12_LN23(port) != hw_state->pcsdw12))
+	if (I915_READ(BXT_PORT_PCS_DW12_LN23(port)) != hw_state->pcsdw12)
  		DRM_DEBUG_DRIVER("lane stagger config different for lane 01 (%08x) and 23 (%08x)\n",
  				 hw_state->pcsdw12,
  				 I915_READ(BXT_PORT_PCS_DW12_LN23(port)));


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux