| Message ID | 20170221181254.14748-3-javier@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 21, 2017 at 03:12:54PM -0300, Javier Martinez Canillas wrote: > The driver doesn't have a struct of_device_id table but supported devices > are registered via Device Trees. This is working on the assumption that a > I2C device registered via OF will always match a legacy I2C device ID and > that the MODALIAS reported will always be of the form i2c:<device>. > > But this could change in the future so the correct approach is to have an > OF device ID table if the devices are registered via OF. > > The compatible strings don't have a vendor prefix because that's how it's > used currently, and changing this will be a Device Tree ABI break. Are you saying that all legacy I2C names now form DT ABI? Even for drivers that do not have of_match_table or OF MODULE_DEVICE_TABLE not binding documentation? I think this is a bit too much. > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > --- > > drivers/input/keyboard/qt1070.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c > index 5a5778729e37..76bb51309a78 100644 > --- a/drivers/input/keyboard/qt1070.c > +++ b/drivers/input/keyboard/qt1070.c > @@ -274,9 +274,18 @@ static const struct i2c_device_id qt1070_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, qt1070_id); > > +#ifdef CONFIG_OF > +static const struct of_device_id qt1070_of_match[] = { > + { .compatible = "qt1070", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, qt1070_of_match); > +#endif > + > static struct i2c_driver qt1070_driver = { > .driver = { > .name = "qt1070", > + .of_match_table = of_match_ptr(qt1070_of_match), > .pm = &qt1070_pm_ops, > }, > .id_table = qt1070_id, > -- > 2.9.3 >
On Thu, Feb 23, 2017 at 12:25:24AM -0800, Dmitry Torokhov wrote: > On Tue, Feb 21, 2017 at 03:12:54PM -0300, Javier Martinez Canillas wrote: > > The driver doesn't have a struct of_device_id table but supported devices > > are registered via Device Trees. This is working on the assumption that a > > I2C device registered via OF will always match a legacy I2C device ID and > > that the MODALIAS reported will always be of the form i2c:<device>. > > > > But this could change in the future so the correct approach is to have an > > OF device ID table if the devices are registered via OF. > > > > The compatible strings don't have a vendor prefix because that's how it's > > used currently, and changing this will be a Device Tree ABI break. > > Are you saying that all legacy I2C names now form DT ABI? Even for > drivers that do not have of_match_table or OF MODULE_DEVICE_TABLE not > binding documentation? > > I think this is a bit too much. Ah, I see that it is actually used in various DTSes. OK, I think we still need the proper compatible ("atmel,qt1070") along with the legacy compatible string. > > > > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > > --- > > > > drivers/input/keyboard/qt1070.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c > > index 5a5778729e37..76bb51309a78 100644 > > --- a/drivers/input/keyboard/qt1070.c > > +++ b/drivers/input/keyboard/qt1070.c > > @@ -274,9 +274,18 @@ static const struct i2c_device_id qt1070_id[] = { > > }; > > MODULE_DEVICE_TABLE(i2c, qt1070_id); > > > > +#ifdef CONFIG_OF > > +static const struct of_device_id qt1070_of_match[] = { > > + { .compatible = "qt1070", }, > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(of, qt1070_of_match); > > +#endif > > + > > static struct i2c_driver qt1070_driver = { > > .driver = { > > .name = "qt1070", > > + .of_match_table = of_match_ptr(qt1070_of_match), > > .pm = &qt1070_pm_ops, > > }, > > .id_table = qt1070_id, > > -- > > 2.9.3 > > > > -- > Dmitry
Hello Dmitry, On 02/23/2017 05:27 AM, Dmitry Torokhov wrote: > On Thu, Feb 23, 2017 at 12:25:24AM -0800, Dmitry Torokhov wrote: >> On Tue, Feb 21, 2017 at 03:12:54PM -0300, Javier Martinez Canillas wrote: >>> The driver doesn't have a struct of_device_id table but supported devices >>> are registered via Device Trees. This is working on the assumption that a >>> I2C device registered via OF will always match a legacy I2C device ID and >>> that the MODALIAS reported will always be of the form i2c:<device>. >>> >>> But this could change in the future so the correct approach is to have an >>> OF device ID table if the devices are registered via OF. >>> >>> The compatible strings don't have a vendor prefix because that's how it's >>> used currently, and changing this will be a Device Tree ABI break. >> >> Are you saying that all legacy I2C names now form DT ABI? Even for >> drivers that do not have of_match_table or OF MODULE_DEVICE_TABLE not >> binding documentation? >> >> I think this is a bit too much. > > Ah, I see that it is actually used in various DTSes. > Yes, I'm only posting patches for drivers whose I2C device ID .name are either used by a DTS or its .name mentioned in a binding as a compatible. The idea is to eventually fix the I2C core to report a proper of MODALIAS so people won't have to add a duplicated I2C device ID table only for it. > OK, I think we still need the proper compatible ("atmel,qt1070") along > with the legacy compatible string. > I didn't add it because no DTS or DT binding doc mentions the complete compatible string, so I would had to guess the vendor prefix. Yes, it's quite likely "atmel", but how can I tell if that's really the case? IOW, I just want to make sure that no driver module auto-loading will regress once the I2C core starts reporting MODALIAS=of:N*T*Cqt1070 instead MODALIAS=i2c:qt1070. So I would prefer if someone who cares about this driver can propose a patch on top to add the compatible with a vendor prefix. >> >>> >>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> >>> --- >>> Best regards,
diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c index 5a5778729e37..76bb51309a78 100644 --- a/drivers/input/keyboard/qt1070.c +++ b/drivers/input/keyboard/qt1070.c @@ -274,9 +274,18 @@ static const struct i2c_device_id qt1070_id[] = { }; MODULE_DEVICE_TABLE(i2c, qt1070_id); +#ifdef CONFIG_OF +static const struct of_device_id qt1070_of_match[] = { + { .compatible = "qt1070", }, + { }, +}; +MODULE_DEVICE_TABLE(of, qt1070_of_match); +#endif + static struct i2c_driver qt1070_driver = { .driver = { .name = "qt1070", + .of_match_table = of_match_ptr(qt1070_of_match), .pm = &qt1070_pm_ops, }, .id_table = qt1070_id,
The driver doesn't have a struct of_device_id table but supported devices are registered via Device Trees. This is working on the assumption that a I2C device registered via OF will always match a legacy I2C device ID and that the MODALIAS reported will always be of the form i2c:<device>. But this could change in the future so the correct approach is to have an OF device ID table if the devices are registered via OF. The compatible strings don't have a vendor prefix because that's how it's used currently, and changing this will be a Device Tree ABI break. Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> --- drivers/input/keyboard/qt1070.c | 9 +++++++++ 1 file changed, 9 insertions(+)