On Mon, Aug 21, 2023 at 12:17:34PM +0300, Andy Shevchenko wrote: > On Sun, Aug 20, 2023 at 08:55:18AM -0700, Guenter Roeck wrote: > > On Sun, Aug 20, 2023 at 02:55:08PM +0000, Biju Das wrote: > > > > On Sun, Aug 20, 2023 at 01:49:08PM +0100, Biju Das wrote: > > ... > > > > While doing this Jonathan found an issue where it won't work if OF table is > > > using pointer and ID is using enum in the match data. Moreover,the proposed > > > API returns NULL for non-match. > > > > I think that is may be problem because many drivers don't have a value > > in the driver_data field. Maybe that doesn't matter because such drivers > > would normally not call device_get_match_data() or i2c_match_id(), > > but it would create some ambiguity because those functions would no > > longer work for all cases. > > Are you talking here about the cases where 0 / NULL makes things optional? > Like when driver data is defined, we use its value, otherwise apply some > defaults? If so, where do you see a problem? > The fact that you have to change lots of drivers to make this work should prove my point. > > > So Andy proposed to convert the valid enums to non-zero or use a pointer. > > > > > There are _lots_ of drivers where the chip ID is in driver_data and starts > > with 0, or with the field not used. It is not my call to make, but I really > > think that demanding that this field is always != 0 is just wrong. > > Those drivers are hackishly abusing OF ID tables. Those, that have _no_ OF ID > tables are okay. This is what you are saying. That doesn't make it true. With the same logic I could claim that drivers providing a pointer in i2c_device_id would hackishly abuse i2c_device_id tables. Guenter > > Moreover this approach allows to have the driver data to be const, and keep it > that way which now is problematic and in some cases may even be broken (due to > wrong assumptions made by drivers). I.o.w. this makes code more robust against > the possible mangling of driver data contents at runtime. > > -- > With Best Regards, > Andy Shevchenko > >
