Re: [PATCH v2 2/4] staging: iio: adt7316: remove shift/offset macros

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Mon, Mar 09, 2026 at 06:32:36PM -0700, Michael Harris wrote:
> On 3/6/26 6:53 AM, Andy Shevchenko wrote:

...

> >>  #include <linux/i2c.h>
> >>  #include <linux/rtc.h>
> >>  #include <linux/module.h>
> >> +#include <linux/bitfield.h>
> >
> > Try to squeeze it to make a longest (but sparse AFAICS) ordered list of
> > inclusions. With given context
> >
> >   #include <linux/bitfield.h>
> >   #include <linux/i2c.h>
> >   #include <linux/rtc.h>
> >   #include <linux/module.h>
> >
> > gives 3 out of 4 in order.
> 
> Placing it there would put it right below slab.h, sysfs.h, and list.h.
> The includes as a whole are fairly messy and unorganized and I don't
> think there's a clean area where I could insert it. Fully reordering it
> would be out of scope for this patch series. If you prefer, I can put it
> at the top of the includes since it would be the highest alphabetically.

As I pointed out "with the given context", meaning that you might find better
place. The idea is to have the longest ordered chain even if it's interrupted
by some unordered items.

...

> >> -            data = msb << ADT7316_T_VALUE_FLOAT_OFFSET;
> >> +            data = FIELD_PREP(ADT7316_AD_MSB_MASK, msb);
> >>              data |= lsb & ADT7316_LSB_IN_TEMP_MASK;
> >
> > Ditto and so on...
> 
> The main purpose of this patch was to delete the offset/shift macros,
> so I only applied the bitfield macros to areas that were using those
> offsets or shifts. I left that existing bitwise operation there because
> it wasn't affected by the offsets or shifts. If you'd prefer, I can
> update it in this case for the sake of symmetry.

The idea is to be consistent. If we change to bitfield,h somewhere else,
it probably better to cover the whole driver at the same time.

-- 
With Best Regards,
Andy Shevchenko