Den tis 23 juni 2020 12:21John Garry <john.garry@huawei.com> skrev:

> On 23/06/2020 10:35, Rikard Falkeborn wrote:
> >
> >     I'd say that GENMASK_INPUT_CHECK() should be able to handle a l=0 and
> >     h=unsigned value, so I doubt this warn.
> >
> >     Using GENMASK((int)cmdq->q.llq.max_n_shift, 0) resolves it, but it
> >     looks
> >     like GENMASK_INPUT_CHECK() could be improved.
> >
> >
> > Indeed it could, it is fixed in -next.
>
> ok, thanks for the pointer, but I still see this on today's -next with
> this patch:
>
> make W=1 drivers/iommu/arm-smmu-v3.o
>
>
Oh, ok thanks for reporting. I guess different gcc versions have different
behaviour. I guess we'll have to change the comparison to (!((h) == (l) ||
(h) > (l))) instead (not sure I got all parenthesis and logic correct but
you get the idea).

I'm travelling and wont have time to look at this until next week though.

Rikard



In file included from ./include/linux/bits.h:23:0,
>                  from ./include/linux/ioport.h:15,
>                  from ./include/linux/acpi.h:12,
>                  from drivers/iommu/arm-smmu-v3.c:12:
> drivers/iommu/arm-smmu-v3.c: In function ‘arm_smmu_cmdq_issue_cmdlist’:
> ./include/linux/bits.h:27:7: warning: comparison of unsigned expression
> < 0 is always false [-Wtype-limits]
>    (l) > (h), 0)))
>        ^
> ./include/linux/build_bug.h:16:62: note: in definition of macro
> ‘BUILD_BUG_ON_ZERO’
> #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
>                                                               ^
> ./include/linux/bits.h:40:3: note: in expansion of macro
> ‘GENMASK_INPUT_CHECK’
>   (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
>    ^~~~~~~~~~~~~~~~~~~
> drivers/iommu/arm-smmu-v3.c:1404:18: note: in expansion of macro ‘GENMASK’
>   u32 prod_mask = GENMASK(cmdq->q.llq.max_n_shift, 0);
>
> That's gcc 7.5.0 .
>
> Cheers,
> John
>
>