Re: [PATCH 0/3] ASoC: Clean-up W=1 build warnings​ - part3

[Lee Jones <lee.jones@linaro.org>]

On Tue, 07 Jul 2020, Pierre-Louis Bossart wrote:
> On 7/7/20 2:39 PM, Jerome Brunet wrote:
> > 
> > On Tue 07 Jul 2020 at 21:23, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
> > 
> > > This is a much smaller set of cleanups, all related to warnings thrown
> > > by the use of GENMASK() with an unsigned variable. I just made the
> > > warning go away but I wonder if there's a better fix in the definition
> > > of GENMASK() itself?
> > 
> > Looking at the patch I was going to ask the same thing.
> > It does not make much sense to me to force GENMASK arguments to be
> > integer (instead of unsigned integer) to then check there are positive ...
> 
> Agree, it's just that the following macro isn't exactly simple to change:
> 
> #define GENMASK_INPUT_CHECK(h, l) \
> 	(BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
> 		__builtin_constant_p((l) > (h)), (l) > (h), 0)))
> 
> I couldn't find a means to avoid the comparison.
> 
> I just realized this is a fairly recent addition in 295bcca84916
> ('linux/bits.h: add compile time sanity check of GENMASK inputs'), adding
> the author Rikard Falkeborn in CC:
> 
> include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0
> is always false [-Wtype-limits]
>    26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))

Linus recently complained about the type-limits warning, saying that
it was invalid.  He preferred the warning to be bumped from W=1 to
W=2, although I haven't seen a patch doing this yet.

Rikard also tried to fix GENMASK directly; however, Linus did not
approve of this either.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog