On Wed, Oct 14, 2015 at 01:58:59PM +0100, Dave Gordon wrote:
> 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); \
> })
I like this.
> 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).
I guess we could fix for_each_dsi_port to only enumerate A and C with
something like
#define for_each_dsi_port(port, mask) \
for (port = PORT_A, port <= PORT_C; port == PORT_A ? PORT_C : port++) \
if (...)
I still think that Ville's approach should catch even more fallout (like
mixing up values and registers for write maybe), but this is a great
interim hack.
Patch wanted ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
