Re: [RFC PATCH] intel: Use upper_32_bits and lower_32_bits

[Joe Perches <joe@perches.com>]

On Mon, 2017-01-09 at 12:55 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 07 January 2017 18:33
> > Shifting and masking various types can be made a bit
> > simpler to read by using the available kernel macros.
> 
> ...
> > -		ew32(TDBAH, (tdba >> 32));
> > -		ew32(TDBAL, (tdba & 0x00000000ffffffffULL));
> > +		ew32(TDBAH, upper_32_bits(tdba));
> > +		ew32(TDBAL, lower_32_bits(tdba));
> 
> Personally I find the original code easier to understand
> since I don't have to look up another silly macro.

It's already a pretty common usage and I believe
the naming is fairly obvious.  Also you don't have
to count the "f" characters to see how many bits
are being used.  After about 6 consecutive chars,
it can be error prone.

The leading zeros?  ugh.  The ULL too.

$ git grep -w -E "upper_32_bits|lower_32_bits" | wc -l
1569

> I'd normally not even explicitly mask the low bits
> relying on the implicit truncation of the assignment.

Relying on implicit behaviors can be noisy when
compilers complain about implicit conversions and
truncations.

> At least modern compilers aren't stupid enough to add two
> 'mask with 0xff' instructions for:
> 	*uchar_ptr = (unsigned char)(foo & 0xff);

I agree it's visual noise.