Hello Wolfram, On 7/31/19 9:44 PM, Wolfram Sang wrote: > Hi Javier, > > thank you for providing the extra information. > > (And Kieran, thanks for the patch!) > >> The other option is to remove i2c_of_match_device() and don't make OF match >> to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI >> case, since i2c_device_match() just calls acpi_driver_match_device() directly >> and doesn't have a wrapper function that fallbacks to sysfs matching. >> >> In this case an I2C device ID table would be required if the devices have to >> be instantiated through sysfs. That way the I2C table would be used both for >> auto-loading and also to match the device when it doesn't have an of_node. > > That would probably mean that only a minority of drivers will not add an I2C > device ID table because it is easy to add an you get the sysfs feature? > I believe so yes. > Then we are back again with the situation that most drivers will have > multiple tables. With the minor change that the I2C device id table is > not required anymore by the core, but it will be just very useful to > have? Or? > Yes, it won't be needed anymore if you are only instantiating all your devices from your firmware interface (e.g: OF, ACPI). >> If the former is the correct way to solve this then the patch looks good to me. >> >> Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> > > For this actual patch from Kieran, I'd like to hear an opinion from the > people maintaining modpost. The aproach looks okay to me, yet I can't > tell how "easy" we are with adding new types like 'i2c_of'. > As Masahiro-san mentioned, this approach will still require to add a new macro MODULE_DEVICE_TABLE(i2c_of, bar_of_match) so the OF device table is used twice. One to expose the "of:N*T*Cfoo,bar" and another one to expose it as "i2c:bar". I expect that many developers would miss adding this macro for new drivers that are DT-only and so sysfs instantiation would not work there. So whatever is the approach taken we should clearly document all this so drivers authors are aware. > Thanks everyone, > > Wolfram > Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat