Re: [PATCH v7 0/4] Extend device_get_match_data() to struct bus_type

Andy Shevchenko
2023-08-10T15:10:20Z

On Wed, Aug 09, 2023 at 06:25:51PM +0100, Jonathan Cameron wrote:
> On Tue, 8 Aug 2023 15:18:52 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Aug 07, 2023 at 01:37:12PM -0700, Dmitry Torokhov wrote:
> > > On Mon, Aug 07, 2023 at 05:54:07PM +0300, Andy Shevchenko wrote:
> > > > On Sun, Aug 06, 2023 at 02:29:50PM +0100, Jonathan Cameron wrote:
> > > > > On Sat, 5 Aug 2023 17:42:21 +0000
> > > > > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > > > > On Fri, 4 Aug 2023 17:17:24 +0100
> > > > > > > Biju Das <biju.das.jz@bp.renesas.com> wrote:

...

> > > > > > + * Besides the fact that some drivers abuse the device ID driver_data type
> > > > > > + * and claim it to be integer, for the bus specific ID tables the driver_data
> > > > > > + * may be defined as kernel_ulong_t. For these tables 0 is a valid response,
> > > > > > + * but not for this function. It's recommended to convert those either to avoid
> > > > > > + * 0 or use a real pointer to the predefined driver data.
> > > >
> > > > > We still need to maintain consistency across the two tables, which
> > > > > is a stronger requirement than avoiding 0.
> > > >
> > > > True. Any suggestion how to amend the above comment? Because the documentation
> > > > makes sense on its own (may be split from the series?).
> > > >
> > > > > Some drivers already do that by forcing the enum used to start at 1 which
> > > > > doesn't solver the different data types issue.
> > > >
> > > > And some maintainers do not want to see non-enum values in i2c ID table.
> > > > *Shrug*.
> > >
> > > So in legacy ID lookup path we can safely assume that values below 4096
> > > are scalars and return NULL from the new device_get_match_data(). This
> > > way current drivers using the values as indices or doing direct
> > > comparisons against them can continue doing manual look up and using
> > > them as they see fit. And we can convert the drivers at our leisure.
> >
> > It's a good idea, but I believe will be received as hack.
> > But why not to try? We indeed have an error pointers for the last page
> > and NULL (which is only up to 16 IIRC) and reserved space in the first
> > page. To be more robust I would check all enums that are being used
> > in I2C ID tables for maximum value and if that is less than 16, use
> > ZERO_OR_NULL_PTR() instead of custom stuff.
> >
> See iio/adc/max1363 example that has 37ish.
>
> Could tidy that one up first and hopefully not find any others that
> are in subsystems not keen on the move away from enums.

Oh, yep, this needs a treewide audit and fixes around before going further.
As you said.

--
With Best Regards,
Andy Shevchenko