RE: [PATCH] regulator: Use bitfield values for range selectors

From: David Laight
Date: Mon Jun 12 2023 - 03:50:42 EST


From: Chen-Yu Tsai
> Sent: 12 June 2023 04:39
>
> On Sun, Jun 11, 2023 at 1:10 AM David Laight <David.Laight@xxxxxxxxxx> wrote:
> >
> > From: Chen-Yu Tsai
> > > Sent: 08 June 2023 08:57
> > >
> > > Right now the regulator helpers expect raw register values for the range
> > > selectors. This is different from the voltage selectors, which are
> > > normalized as bitfield values. This leads to a bit of confusion. Also,
> > > raw values are harder to copy from datasheets or match up with them,
> > > as datasheets will typically have bitfield values.
> > >
> > > Make the helpers expect bitfield values, and convert existing users.
> > > Include bitops.h explicitly for ffs(), and reorder the header include
> > > statements. While at it, also replace module.h with export.h, since the
> > > only use is EXPORT_SYMBOL_GPL.
> > >
> > ...
> > > static const unsigned int atc260x_ldo_voltage_range_sel[] = {
> > > - 0x0, 0x20,
> > > + 0x0, 0x1,
> > > };
> >
> > Is there any way the change can be done so that un-edited
> > modules fail to compile?
> > Otherwise the whole thing is an accident waiting to happen.
>
> I think we could change the field name in the regulator description?
> But unsuspecting end users / developers might just edit the name and not
> see that the scheme has changed.
>
> Or we could add a sanity check at runtime that checks the values during
> regulator registration. How does that sound?
>
> Mark, is this something you'd like?

Can you change the name of the function the values are passed to?
Or maybe change the type to 'unsigned char' (assuming bit numbers
are small).

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)