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@xxxxxxxxxxxxxxx> 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
