Re: [PATCH v1 1/1] PCI: brcmstb: Use GENMASK() as __GENMASK() is for internal use only

[Andy Shevchenko]

On Wed, Oct 27, 2021 at 12:00:16PM +0200, Krzysztof Wilczyński wrote:
> > Use GENMASK() as __GENMASK() is for internal use only.
> 
> To add, for posterity, that using __GENMASK() bypasses the
> GENMASK_INPUT_CHECK() macro that adds extra validation.

In general, yes, but here we have a variable...

> > -	u32 val = __GENMASK(31, msi->legacy_shift);
> > +	u32 val = GENMASK(31, msi->legacy_shift);

...which make me thing that the whole construction is ugly
(and I truly believe the code is very ugly here, because
 the idea behind GENMASK() is to be used with constants).

So, what about

	u32 val = ~(BIT(msi->legacy_shift) - 1);

instead?

> Thank you!
> 
> Reviewed-by: Krzysztof Wilczyński <kw@xxxxxxxxx>

Thank you!

-- 
With Best Regards,
Andy Shevchenko