| Message ID | 1410249158-18192-1-git-send-email-sjoerd.simons@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[adding Lee Jones to cc list since I'm referring on a series he posted] Hello Sjoerd, On 09/09/2014 09:52 AM, Sjoerd Simons wrote: > For i2c devices in OF the modalias exposed to userspace is i2c:<node > type>, for the Maxtouch driver this is i2c:maxtouch. > > Add maxtouch to the i2c id table such that userspace can correctly > load the module for the device and drop the OF table as it's not > needed for i2c devices. > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > --- > drivers/input/touchscreen/atmel_mxt_ts.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index db178ed..57ff26d 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -2267,16 +2267,11 @@ static int mxt_resume(struct device *dev) > > static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume); > > -static const struct of_device_id mxt_of_match[] = { > - { .compatible = "atmel,maxtouch", }, > - {}, > -}; > -MODULE_DEVICE_TABLE(of, mxt_of_match); > - > static const struct i2c_device_id mxt_id[] = { > { "qt602240_ts", 0 }, > { "atmel_mxt_ts", 0 }, > { "atmel_mxt_tp", 0 }, > + { "maxtouch", 0 }, > { "mXT224", 0 }, > { } > }; > @@ -2286,7 +2281,6 @@ static struct i2c_driver mxt_driver = { > .driver = { > .name = "atmel_mxt_ts", > .owner = THIS_MODULE, > - .of_match_table = of_match_ptr(mxt_of_match), > .pm = &mxt_pm_ops, > }, > .probe = mxt_probe, > I see that Lee is working to allow the I2C subsystem to not need an I2C ID table to match [0]. I'll let Lee to comment what the future plans are and if his series are going to solve your issue since I'm not that familiar with the I2C core. Best regards, Javier [0]: https://lkml.org/lkml/2014/6/20/199 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/09/14 11:21, Javier Martinez Canillas wrote: > On 09/09/2014 09:52 AM, Sjoerd Simons wrote: >> For i2c devices in OF the modalias exposed to userspace is i2c:<node >> type>, for the Maxtouch driver this is i2c:maxtouch. >> >> Add maxtouch to the i2c id table such that userspace can correctly >> load the module for the device and drop the OF table as it's not >> needed for i2c devices. > I see that Lee is working to allow the I2C subsystem to not need an I2C ID > table to match [0]. I'll let Lee to comment what the future plans are and if > his series are going to solve your issue since I'm not that familiar with the > I2C core. > > Best regards, > Javier > > [0]: https://lkml.org/lkml/2014/6/20/199 I can see the benefit of not having the duplication. Am I correct that you're saying that it might make more sense to remove the i2c ids rather than the OF table, if Lee's changes are accepted? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2014-09-09 at 11:29 +0100, Nick Dyer wrote: > On 09/09/14 11:21, Javier Martinez Canillas wrote: > > On 09/09/2014 09:52 AM, Sjoerd Simons wrote: > >> For i2c devices in OF the modalias exposed to userspace is i2c:<node > >> type>, for the Maxtouch driver this is i2c:maxtouch. > >> > >> Add maxtouch to the i2c id table such that userspace can correctly > >> load the module for the device and drop the OF table as it's not > >> needed for i2c devices. > > I see that Lee is working to allow the I2C subsystem to not need an I2C ID > > table to match [0]. I'll let Lee to comment what the future plans are and if > > his series are going to solve your issue since I'm not that familiar with the > > I2C core. > > I can see the benefit of not having the duplication. Am I correct that > you're saying that it might make more sense to remove the i2c ids rather > than the OF table, if Lee's changes are accepted? You would still need the i2C table for non-OF platforms ofcourse. I'm not sure what happens with the modalias as exposed to userspace with Lee's patchset, if that gets changed to prefer an of: type instead of the current i2c: prefixed ones this patch (at that point) isn't needed.
On 09/09/14 08:52, Sjoerd Simons wrote: > For i2c devices in OF the modalias exposed to userspace is i2c:<node > type>, for the Maxtouch driver this is i2c:maxtouch. > > Add maxtouch to the i2c id table such that userspace can correctly > load the module for the device and drop the OF table as it's not > needed for i2c devices. > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> I've tested this and it does work. In fact, it seems that you can use compatible = "i2c:atmel,maxtouch"; because the "atmel," is ignored in the matching. Before I could ack this patch, we will need to update this dts file: arch/arm/boot/dts/s5pv210-goni.dts and this documentation file: Documentation/devicetree/bindings/input/atmel,maxtouch.txt thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 09 Sep 2014, Javier Martinez Canillas wrote: > [adding Lee Jones to cc list since I'm referring on a series he posted] > > Hello Sjoerd, > > On 09/09/2014 09:52 AM, Sjoerd Simons wrote: > > For i2c devices in OF the modalias exposed to userspace is i2c:<node > > type>, for the Maxtouch driver this is i2c:maxtouch. > > > > Add maxtouch to the i2c id table such that userspace can correctly > > load the module for the device and drop the OF table as it's not > > needed for i2c devices. > > > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > > --- > > drivers/input/touchscreen/atmel_mxt_ts.c | 8 +------- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > > index db178ed..57ff26d 100644 > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > > @@ -2267,16 +2267,11 @@ static int mxt_resume(struct device *dev) > > > > static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume); > > > > -static const struct of_device_id mxt_of_match[] = { > > - { .compatible = "atmel,maxtouch", }, > > - {}, > > -}; > > -MODULE_DEVICE_TABLE(of, mxt_of_match); > > - > > static const struct i2c_device_id mxt_id[] = { > > { "qt602240_ts", 0 }, > > { "atmel_mxt_ts", 0 }, > > { "atmel_mxt_tp", 0 }, > > + { "maxtouch", 0 }, > > { "mXT224", 0 }, > > { } > > }; > > @@ -2286,7 +2281,6 @@ static struct i2c_driver mxt_driver = { > > .driver = { > > .name = "atmel_mxt_ts", > > .owner = THIS_MODULE, > > - .of_match_table = of_match_ptr(mxt_of_match), > > .pm = &mxt_pm_ops, > > }, > > .probe = mxt_probe, > > > > I see that Lee is working to allow the I2C subsystem to not need an I2C ID > table to match [0]. I'll let Lee to comment what the future plans are and if > his series are going to solve your issue since I'm not that familiar with the > I2C core. It's wrong to expect DT to probe these devices without a compatible string. It does so at the moment, but this is a bi-product and not the correct method.
Hey Lee, On Wed, 2014-09-10 at 10:28 +0100, Lee Jones wrote: > On Tue, 09 Sep 2014, Javier Martinez Canillas wrote: > > > [adding Lee Jones to cc list since I'm referring on a series he posted] > > > > Hello Sjoerd, > > > > On 09/09/2014 09:52 AM, Sjoerd Simons wrote: > > > For i2c devices in OF the modalias exposed to userspace is i2c:<node > > > type>, for the Maxtouch driver this is i2c:maxtouch. > > > > > > Add maxtouch to the i2c id table such that userspace can correctly > > > load the module for the device and drop the OF table as it's not > > > needed for i2c devices. > > > > > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > > > --- > > > drivers/input/touchscreen/atmel_mxt_ts.c | 8 +------- > > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > > > index db178ed..57ff26d 100644 > > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > > > @@ -2267,16 +2267,11 @@ static int mxt_resume(struct device *dev) > > > > > > static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume); > > > > > > -static const struct of_device_id mxt_of_match[] = { > > > - { .compatible = "atmel,maxtouch", }, > > > - {}, > > > -}; > > > -MODULE_DEVICE_TABLE(of, mxt_of_match); > > > - > > > static const struct i2c_device_id mxt_id[] = { > > > { "qt602240_ts", 0 }, > > > { "atmel_mxt_ts", 0 }, > > > { "atmel_mxt_tp", 0 }, > > > + { "maxtouch", 0 }, > > > { "mXT224", 0 }, > > > { } > > > }; > > > @@ -2286,7 +2281,6 @@ static struct i2c_driver mxt_driver = { > > > .driver = { > > > .name = "atmel_mxt_ts", > > > .owner = THIS_MODULE, > > > - .of_match_table = of_match_ptr(mxt_of_match), > > > .pm = &mxt_pm_ops, > > > }, > > > .probe = mxt_probe, > > > > > > > I see that Lee is working to allow the I2C subsystem to not need an I2C ID > > table to match [0]. I'll let Lee to comment what the future plans are and if > > his series are going to solve your issue since I'm not that familiar with the > > I2C core. > > It's wrong to expect DT to probe these devices without a compatible > string. It does so at the moment, but this is a bi-product and not > the correct method. Ok, which means removing the mxt_of_match table in this patch is wrong.. I'll fix that for for a V2. However that makes adding the "maxtouch" string to the i2c device table somewhat cumbersome as it only gets added in this case to ensure module-autoloading can happen as the modalias presented to userspace is going still going to be i2c:maxtouch. Tbh, the bigger problem this is pointing out is that for I2C devices with only an OF compability tring module auto-loading is broken...
Hello Lee, On 09/11/2014 10:00 AM, Sjoerd Simons wrote: >> > > >> > > -static const struct of_device_id mxt_of_match[] = { >> > > - { .compatible = "atmel,maxtouch", }, >> > > - {}, >> > > -}; >> > > -MODULE_DEVICE_TABLE(of, mxt_of_match); >> > > - >> > > static const struct i2c_device_id mxt_id[] = { >> > > { "qt602240_ts", 0 }, >> > > { "atmel_mxt_ts", 0 }, >> > > { "atmel_mxt_tp", 0 }, >> > > + { "maxtouch", 0 }, >> > > { "mXT224", 0 }, >> > > { } >> > > }; >> > > @@ -2286,7 +2281,6 @@ static struct i2c_driver mxt_driver = { >> > > .driver = { >> > > .name = "atmel_mxt_ts", >> > > .owner = THIS_MODULE, >> > > - .of_match_table = of_match_ptr(mxt_of_match), >> > > .pm = &mxt_pm_ops, >> > > }, >> > > .probe = mxt_probe, >> > > >> > >> > I see that Lee is working to allow the I2C subsystem to not need an I2C ID >> > table to match [0]. I'll let Lee to comment what the future plans are and if >> > his series are going to solve your issue since I'm not that familiar with the >> > I2C core. >> >> It's wrong to expect DT to probe these devices without a compatible >> string. It does so at the moment, but this is a bi-product and not >> the correct method. > > Ok, which means removing the mxt_of_match table in this patch is wrong.. > I'll fix that for for a V2. > > However that makes adding the "maxtouch" string to the i2c device table > somewhat cumbersome as it only gets added in this case to ensure > module-autoloading can happen as the modalias presented to userspace is > going still going to be i2c:maxtouch. > > Tbh, the bigger problem this is pointing out is that for I2C devices > with only an OF compability tring module auto-loading is broken... > To expand on what Sjoerd already said and just to be sure everyone is on the same page. The problem is that right now the driver reports the following modalias: # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias i2c:maxtouch but if you look at the module information, that is not a valid alias: # modinfo atmel_mxt_ts | grep alias alias: i2c:mXT224 alias: i2c:atmel_mxt_tp alias: i2c:atmel_mxt_ts alias: i2c:qt602240_ts alias: of:N*T*Catmel,maxtouch* which means that udev/kmod can't load the module automatically based on the alias information. The aliases are filled by both MODULE_DEVICE_TABLE(i2c, mxt_id) and MODULE_DEVICE_TABLE(of, mxt_of_match) so after Sjoerd's patch: # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias i2c:maxtouch # modinfo atmel_mxt_ts | grep alias alias: i2c:mXT224 alias: i2c:maxtouch alias: i2c:atmel_mxt_tp alias: i2c:atmel_mxt_ts alias: i2c:qt602240_ts which matches the reported uevent so the module will be auto-loaded. This is because the I2C subsystem hardcodes i2c:<client->name>, if you look at drivers/i2c/i2c-core.c: /* uevent helps with hotplug: modprobe -q $(MODALIAS) */ static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env) { ... if (add_uevent_var(env, "MODALIAS=%s%s", I2C_MODULE_PREFIX, client->name)) ... } I've looked at Lee's series and AFAICT that remains the same so I second Sjoerd that module auto-loading will continue to be broken. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/09/14 09:38, Javier Martinez Canillas wrote: > To expand on what Sjoerd already said and just to be sure everyone is on the > same page. > > The problem is that right now the driver reports the following modalias: > > # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias > i2c:maxtouch > > but if you look at the module information, that is not a valid alias: > > # modinfo atmel_mxt_ts | grep alias > alias: i2c:mXT224 > alias: i2c:atmel_mxt_tp > alias: i2c:atmel_mxt_ts > alias: i2c:qt602240_ts > alias: of:N*T*Catmel,maxtouch* > > which means that udev/kmod can't load the module automatically based on the > alias information. > > The aliases are filled by both MODULE_DEVICE_TABLE(i2c, mxt_id) and > MODULE_DEVICE_TABLE(of, mxt_of_match) so after Sjoerd's patch: > > # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias > i2c:maxtouch > > # modinfo atmel_mxt_ts | grep alias > alias: i2c:mXT224 > alias: i2c:maxtouch > alias: i2c:atmel_mxt_tp > alias: i2c:atmel_mxt_ts > alias: i2c:qt602240_ts > > which matches the reported uevent so the module will be auto-loaded. > > This is because the I2C subsystem hardcodes i2c:<client->name>, if you look at > drivers/i2c/i2c-core.c: > > /* uevent helps with hotplug: modprobe -q $(MODALIAS) */ > static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env) > { > ... > if (add_uevent_var(env, "MODALIAS=%s%s", > I2C_MODULE_PREFIX, client->name)) > ... > } > > I've looked at Lee's series and AFAICT that remains the same so I second > Sjoerd that module auto-loading will continue to be broken. Thanks for the clear explanation. The i2c aliases are a bit confusing. The original device the driver was written for was called qt602240, which was renamed by Atmel to mXT224 when the chip series was called "maXTouch". The driver now actually supports many other chips which aren't listed (more than 20 devices that I've personally tested). I could add them all, but it would be an extremely long list. It may be preferable to use the generic name maXTouch. So I think the sensible thing to do here would be to add "maxtouch" to the i2c list to fix the module autoload issue. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Nick, On 09/11/2014 11:19 AM, Nick Dyer wrote: > > Thanks for the clear explanation. > > The i2c aliases are a bit confusing. The original device the driver was > written for was called qt602240, which was renamed by Atmel to mXT224 when > the chip series was called "maXTouch". The driver now actually supports > many other chips which aren't listed (more than 20 devices that I've > personally tested). I could add them all, but it would be an extremely long > list. It may be preferable to use the generic name maXTouch. > > So I think the sensible thing to do here would be to add "maxtouch" to the > i2c list to fix the module autoload issue. > While this will actually fix the module auto-load issue on the atmel driver, I'm concerned that the I2C core is not reporting the correct module 'of:N*T*Catmel,maxtouch*' alias when probing using DT. Since as Lee said on his cover letter for the mentioned series [0], an I2C ID table shouldn't be mandatory for drivers that only support DT based platforms (e.g: a driver that depends on OF) but in that case I2C module auto-loading would not work AFAICT. Best regards, Javier [0]: https://lkml.org/lkml/2014/6/20/199 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Funny timing. I am just reviewing the series from Lee and also stumbled over modaliases, too... On Thu, Sep 11, 2014 at 10:19:54AM +0100, Nick Dyer wrote: > On 11/09/14 09:38, Javier Martinez Canillas wrote: > > To expand on what Sjoerd already said and just to be sure everyone is on the > > same page. > > > > The problem is that right now the driver reports the following modalias: > > > > # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias > > i2c:maxtouch > > > > but if you look at the module information, that is not a valid alias: > > > > # modinfo atmel_mxt_ts | grep alias > > alias: i2c:mXT224 > > alias: i2c:atmel_mxt_tp > > alias: i2c:atmel_mxt_ts > > alias: i2c:qt602240_ts > > alias: of:N*T*Catmel,maxtouch* > > > > which means that udev/kmod can't load the module automatically based on the > > alias information. > > > > The aliases are filled by both MODULE_DEVICE_TABLE(i2c, mxt_id) and > > MODULE_DEVICE_TABLE(of, mxt_of_match) so after Sjoerd's patch: > > > > # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias > > i2c:maxtouch > > > > # modinfo atmel_mxt_ts | grep alias > > alias: i2c:mXT224 > > alias: i2c:maxtouch > > alias: i2c:atmel_mxt_tp > > alias: i2c:atmel_mxt_ts > > alias: i2c:qt602240_ts > > > > which matches the reported uevent so the module will be auto-loaded. > > > > This is because the I2C subsystem hardcodes i2c:<client->name>, if you look at > > drivers/i2c/i2c-core.c: > > > > /* uevent helps with hotplug: modprobe -q $(MODALIAS) */ > > static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env) > > { > > ... > > if (add_uevent_var(env, "MODALIAS=%s%s", > > I2C_MODULE_PREFIX, client->name)) > > ... > > } > > > > I've looked at Lee's series and AFAICT that remains the same so I second > > Sjoerd that module auto-loading will continue to be broken. I think the above code in the i2c core needs a fix. Will have a closer look after lunch. > The i2c aliases are a bit confusing. The original device the driver was > written for was called qt602240, which was renamed by Atmel to mXT224 when > the chip series was called "maXTouch". The driver now actually supports > many other chips which aren't listed (more than 20 devices that I've > personally tested). I could add them all, but it would be an extremely long > list. It may be preferable to use the generic name maXTouch. This is probably true for some more I2C devices. Like RTCs being compatible or, most prominent, EEPROMS. I don't want to have a list of all vendors producing 24c02s if they are all compatible. If generic entries are frowned upon, I'd agree on a "first come, first served" policy: Somebody provides one compatible-property with the vendor which happens to be on that board, and the others have to reuse that compatible-property since they are, well, compatible. > So I think the sensible thing to do here would be to add "maxtouch" to the > i2c list to fix the module autoload issue. This is a workaround. It would make sense, however, to add it because we want to support i2c_board_info structures. Regards, Wolfram
Hello Wolfram, On 09/11/2014 01:08 PM, Wolfram Sang wrote: > > Funny timing. I am just reviewing the series from Lee and also stumbled > over modaliases, too... > > On Thu, Sep 11, 2014 at 10:19:54AM +0100, Nick Dyer wrote: >> On 11/09/14 09:38, Javier Martinez Canillas wrote: >> > To expand on what Sjoerd already said and just to be sure everyone is on the >> > same page. >> > >> > The problem is that right now the driver reports the following modalias: >> > >> > # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias >> > i2c:maxtouch >> > >> > but if you look at the module information, that is not a valid alias: >> > >> > # modinfo atmel_mxt_ts | grep alias >> > alias: i2c:mXT224 >> > alias: i2c:atmel_mxt_tp >> > alias: i2c:atmel_mxt_ts >> > alias: i2c:qt602240_ts >> > alias: of:N*T*Catmel,maxtouch* >> > >> > which means that udev/kmod can't load the module automatically based on the >> > alias information. >> > >> > The aliases are filled by both MODULE_DEVICE_TABLE(i2c, mxt_id) and >> > MODULE_DEVICE_TABLE(of, mxt_of_match) so after Sjoerd's patch: >> > >> > # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias >> > i2c:maxtouch >> > >> > # modinfo atmel_mxt_ts | grep alias >> > alias: i2c:mXT224 >> > alias: i2c:maxtouch >> > alias: i2c:atmel_mxt_tp >> > alias: i2c:atmel_mxt_ts >> > alias: i2c:qt602240_ts >> > >> > which matches the reported uevent so the module will be auto-loaded. >> > >> > This is because the I2C subsystem hardcodes i2c:<client->name>, if you look at >> > drivers/i2c/i2c-core.c: >> > >> > /* uevent helps with hotplug: modprobe -q $(MODALIAS) */ >> > static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env) >> > { >> > ... >> > if (add_uevent_var(env, "MODALIAS=%s%s", >> > I2C_MODULE_PREFIX, client->name)) >> > ... >> > } >> > >> > I've looked at Lee's series and AFAICT that remains the same so I second >> > Sjoerd that module auto-loading will continue to be broken. > > I think the above code in the i2c core needs a fix. Will have a closer > look after lunch. > Agreed, I just posted an RFC patch [0] with the fix but as Sjoerd pointed out on an internal review, changing that will regress all the drivers that were relying on the old behavior. >> The i2c aliases are a bit confusing. The original device the driver was >> written for was called qt602240, which was renamed by Atmel to mXT224 when >> the chip series was called "maXTouch". The driver now actually supports >> many other chips which aren't listed (more than 20 devices that I've >> personally tested). I could add them all, but it would be an extremely long >> list. It may be preferable to use the generic name maXTouch. > > This is probably true for some more I2C devices. Like RTCs being > compatible or, most prominent, EEPROMS. I don't want to have a list of > all vendors producing 24c02s if they are all compatible. If generic > entries are frowned upon, I'd agree on a "first come, first served" policy: > Somebody provides one compatible-property with the vendor which happens > to be on that board, and the others have to reuse that > compatible-property since they are, well, compatible. > Agreed. >> So I think the sensible thing to do here would be to add "maxtouch" to the >> i2c list to fix the module autoload issue. > > This is a workaround. It would make sense, however, to add it because we > want to support i2c_board_info structures. > I think it really depends if an IP block can be used on non-DT platforms (which I think is true for this trackpad) but if a driver is for an IP block that can only be used on a DT-only platform (e.g: a PMIC that is controlled over I2C and is only compatible with a DT-only SoC) then I don't think we need to support the i2c_board_info structure and can get rid of the I2C ID table on these drivers once Lee series land. > Regards, > > Wolfram > Best regards, Javier [0]: https://patchwork.ozlabs.org/patch/388200/ -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > This is a workaround. It would make sense, however, to add it because we > > want to support i2c_board_info structures. > > > > I think it really depends if an IP block can be used on non-DT platforms > (which I think is true for this trackpad) but if a driver is for an IP block > that can only be used on a DT-only platform (e.g: a PMIC that is controlled > over I2C and is only compatible with a DT-only SoC) then I don't think we need > to support the i2c_board_info structure and can get rid of the I2C ID table on > these drivers once Lee series land. That is exactly what I meant. It should be only added if there is a reason other than "workaround". If you say, it doesn't make sense on non-DT, then it should not be added.
Hello Wolfram, On 09/11/2014 01:35 PM, Wolfram Sang wrote: > >> > This is a workaround. It would make sense, however, to add it because we >> > want to support i2c_board_info structures. >> > >> >> I think it really depends if an IP block can be used on non-DT platforms >> (which I think is true for this trackpad) but if a driver is for an IP block >> that can only be used on a DT-only platform (e.g: a PMIC that is controlled >> over I2C and is only compatible with a DT-only SoC) then I don't think we need >> to support the i2c_board_info structure and can get rid of the I2C ID table on >> these drivers once Lee series land. > > That is exactly what I meant. It should be only added if there is a > reason other than "workaround". If you say, it doesn't make sense on > non-DT, then it should not be added. > Sorry for explaining myself badly. I just tried to say that this is a decision that has to be made on a per-driver basis but I don't really know if makes sense or not in this case since I don't know if this device is (or could be) shipped on non-DT platforms. Nick is in a much better position to answer that question. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/09/14 12:41, Javier Martinez Canillas wrote: > On 09/11/2014 01:35 PM, Wolfram Sang wrote: >>>> This is a workaround. It would make sense, however, to add it because we >>>> want to support i2c_board_info structures. >>>> >>> >>> I think it really depends if an IP block can be used on non-DT platforms >>> (which I think is true for this trackpad) but if a driver is for an IP block >>> that can only be used on a DT-only platform (e.g: a PMIC that is controlled >>> over I2C and is only compatible with a DT-only SoC) then I don't think we need >>> to support the i2c_board_info structure and can get rid of the I2C ID table on >>> these drivers once Lee series land. >> >> That is exactly what I meant. It should be only added if there is a >> reason other than "workaround". If you say, it doesn't make sense on >> non-DT, then it should not be added. > > Sorry for explaining myself badly. I just tried to say that this is a decision > that has to be made on a per-driver basis but I don't really know if makes > sense or not in this case since I don't know if this device is (or could be) > shipped on non-DT platforms. Nick is in a much better position to answer that > question. There are plenty of out-of-tree users of this driver which don't use DT. The most significant use I'm aware of is on Chromium OS systems. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index db178ed..57ff26d 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -2267,16 +2267,11 @@ static int mxt_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume); -static const struct of_device_id mxt_of_match[] = { - { .compatible = "atmel,maxtouch", }, - {}, -}; -MODULE_DEVICE_TABLE(of, mxt_of_match); - static const struct i2c_device_id mxt_id[] = { { "qt602240_ts", 0 }, { "atmel_mxt_ts", 0 }, { "atmel_mxt_tp", 0 }, + { "maxtouch", 0 }, { "mXT224", 0 }, { } }; @@ -2286,7 +2281,6 @@ static struct i2c_driver mxt_driver = { .driver = { .name = "atmel_mxt_ts", .owner = THIS_MODULE, - .of_match_table = of_match_ptr(mxt_of_match), .pm = &mxt_pm_ops, }, .probe = mxt_probe,
For i2c devices in OF the modalias exposed to userspace is i2c:<node type>, for the Maxtouch driver this is i2c:maxtouch. Add maxtouch to the i2c id table such that userspace can correctly load the module for the device and drop the OF table as it's not needed for i2c devices. Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> --- drivers/input/touchscreen/atmel_mxt_ts.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)