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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 7 Aug 2023 17:54:07 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> wrote:  
> > > > On Fri,  4 Aug 2023 17:17:24 +0100
> > > > Biju Das <biju.das.jz@xxxxxxxxxxxxxx> 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?).

For bus ID tables it is fine right now as long as no one checks for NULL.
I guess adding this to the i2c_get_match_data and spi equivalent wrapper
functions might avoid someone shooting themselves in the foot (I've done it
for starters more than once).

> 
> > 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*.
> 

That leaves us stuck unless we move to a form where the i2c ID table isn't
used if there is an of_device_id table (or maybe we invent yet another table
and if that is present it is used for dt and were i2c_device_id is used and
hence becomes an opt in?  That will also be tricky however.

Jonathan



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux