Subject: Re: [PATCH] phy: allwinner: Fix GENMASK misuse

From: Paul Kocialkowski <[email-masked]>
Date: Fri, 8 Nov 2019 09:29:07 +0100

Hi,

On Thu 07 Nov 19, 23:39, Russell King - ARM Linux admin wrote:
> On Thu, Nov 07, 2019 at 09:46:45PM +0100, Rikard Falkeborn wrote:
> > Arguments are supposed to be ordered high then low.
> > 
> > Signed-off-by: Rikard Falkeborn <[email-masked]>
> > ---
> > Spotted while trying to add compile time checks of GENMASK arguments.
> > Patch has only been compile tested.
> 
> My feeling, personally, is that GENMASK() really isn't worth the pain
> it causes.  Can we instead get rid of this thing and just use easier
> to understand and less error-prone hex masks please?

One advantage it has is that is matches the order in which bit fields are
usually given in datasheets, so I personally found that it makes verification
of fields much more straightforward and immediate.

My 2 cents are that it makes sense for hardware registers.

Note that I have recently introduced a SHIFT_AND_MASK_BITS macro[0] for a V4L2
driver, that I (and Mauro) would like to move to linux/bits.h eventually.

> I don't care what anyone else says, personally I'm going to stick with
> using hex masks as I find them way easier to get right first time than
> a problematical opaque macro - and I really don't want the effort of
> finding out that I've got the arguments wrong when I build it.  It's
> just _way_ easier and less error prone to use a hex mask straight off.

I guess it's a matter of personal habit.

[0]: https://git.linuxtv.org/media_tree.git/commit/?id=06eff2150d4db991ca236f3d05a9dc0101475aea

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel