[PATCH 06/10] input: touchscreen: edt-ft5x06: fix driver autoprobing

[PATCH 06/10] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

i2c devices match against struct i2c_device_id
even for CONFIG_OF case, so adding a struct of_device_id
doesn't change anything. As a result, currently, edt-ft5x06
will not autoprobe if built as a module.

To fix the issue and still maintain backwards compatibility
with all DTS files currently in tree, we're just moving
all ids from of_device_id to i2c_device_id while also
adding the following specific ids which should be used
from now on:

	{ "edt-ft5206", 0, }
	{ "edt-ft5306", 0, }
	{ "edt-ft5406", 0, }

Cc: Lothar Waßmann <lw@karo-electronics.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: <devicetree@vger.kernel.org>
Cc: <linux-input@vger.kernel.org>
Cc: <stable@vger.kernel.org> # v3.15+
Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt           |  9 ++++++---
 drivers/input/touchscreen/edt-ft5x06.c                  | 17 ++++++-----------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 76db967..50bd5d2 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -14,9 +14,12 @@ bindings.
 
 
 Required properties:
- - compatible:  "edt,edt-ft5206"
-           or:  "edt,edt-ft5306"
-           or:  "edt,edt-ft5406"
+ - compatible:  "edt-ft5206"
+	   or:  "edt-ft5306"
+	   or:  "edt-ft5406"
+	   or:  "edt,edt-ft5206" (deprecated)
+           or:  "edt,edt-ft5306" (deprecated)
+           or:  "edt,edt-ft5406" (deprecated)
 
  - reg:         I2C slave address of the chip (0x38)
  - interrupt-parent: a phandle pointing to the interrupt controller
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index ee3434f..f161ff9 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1119,25 +1119,20 @@ static SIMPLE_DEV_PM_OPS(edt_ft5x06_ts_pm_ops,
 
 static const struct i2c_device_id edt_ft5x06_ts_id[] = {
 	{ "edt-ft5x06", 0, },
+	{ "edt-ft5206", 0, },
+	{ "edt-ft5306", 0, },
+	{ "edt-ft5406", 0, },
+	{ "edt,edt-ft5206", 0, },
+	{ "edt,edt-ft5306", 0, },
+	{ "edt,edt-ft5406", 0, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, edt_ft5x06_ts_id);
 
-#ifdef CONFIG_OF
-static const struct of_device_id edt_ft5x06_of_match[] = {
-	{ .compatible = "edt,edt-ft5206", },
-	{ .compatible = "edt,edt-ft5306", },
-	{ .compatible = "edt,edt-ft5406", },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, edt_ft5x06_of_match);
-#endif
-
 static struct i2c_driver edt_ft5x06_ts_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "edt_ft5x06",
-		.of_match_table = of_match_ptr(edt_ft5x06_of_match),
 		.pm = &edt_ft5x06_ts_pm_ops,
 	},
 	.id_table = edt_ft5x06_ts_id,
-- 
2.1.0.GIT

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/10] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Dmitry Torokhov]

Hi Felipe,

On Tue, Nov 18, 2014 at 12:07:04PM -0600, Felipe Balbi wrote:
> i2c devices match against struct i2c_device_id
> even for CONFIG_OF case, so adding a struct of_device_id
> doesn't change anything. As a result, currently, edt-ft5x06
> will not autoprobe if built as a module.

Why doe snot it autoprobe? We properly declare MODULE_DEVICE_TABLE for
OF, is it because we are missing some data in device uevent?

> 
> To fix the issue and still maintain backwards compatibility
> with all DTS files currently in tree, we're just moving
> all ids from of_device_id to i2c_device_id while also
> adding the following specific ids which should be used
> from now on:
> 
> 	{ "edt-ft5206", 0, }
> 	{ "edt-ft5306", 0, }
> 	{ "edt-ft5406", 0, }

Is this a tee-wide change? Link to the discussion?

Thanks.

-- 
Dmitry

Re: [PATCH 06/10] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]

On Tue, Nov 18, 2014 at 10:38:47AM -0800, Dmitry Torokhov wrote:
> Hi Felipe,
> 
> On Tue, Nov 18, 2014 at 12:07:04PM -0600, Felipe Balbi wrote:
> > i2c devices match against struct i2c_device_id
> > even for CONFIG_OF case, so adding a struct of_device_id
> > doesn't change anything. As a result, currently, edt-ft5x06
> > will not autoprobe if built as a module.
> 
> Why doe snot it autoprobe? We properly declare MODULE_DEVICE_TABLE for
> OF, is it because we are missing some data in device uevent?

because of of_i2c_register_devices(). Maybe Wolfram can give a better
explanation here, but it just doesn't match through of_device_id.

Apply this:

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 2f90ac6..f0dc16e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -449,8 +449,10 @@ static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 						const struct i2c_client *client)
 {
 	while (id->name[0]) {
-		if (strcmp(client->name, id->name) == 0)
+		if (strcmp(client->name, id->name) == 0) {
+			dev_info(&client->dev, "i2c_device_id match\n");
 			return id;
+		}
 		id++;
 	}
 	return NULL;
@@ -465,8 +467,10 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
 		return 0;
 
 	/* Attempt an OF style match */
-	if (of_driver_match_device(dev, drv))
+	if (of_driver_match_device(dev, drv)) {
+		dev_info(dev, "of driver match\n");
 		return 1;
+	}
 
 	/* Then ACPI style match */
 	if (acpi_driver_match_device(dev, drv))
@@ -1081,6 +1085,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.bus = &i2c_bus_type;
 	client->dev.type = &i2c_client_type;
 	client->dev.of_node = info->of_node;
+	dev_info(&adap->dev, "%s: of_node %p\n", __func__, info->of_node);
 	ACPI_COMPANION_SET(&client->dev, info->acpi_node.companion);
 
 	i2c_dev_set_name(adap, client);
@@ -1411,6 +1416,7 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
 
 		info.irq = irq_of_parse_and_map(node, 0);
 		info.of_node = of_node_get(node);
+		dev_info(&adap->dev, "%s: of_node %p\n", __func__, info.of_node);
 		info.archdata = &dev_ad;
 
 		if (of_get_property(node, "wakeup-source", NULL))

then boot the board and you get http://hastebin.com/oqemezajez

Interesting, it's matching against of but only when I modprobe.

Let me debug this one a little more.

> > To fix the issue and still maintain backwards compatibility
> > with all DTS files currently in tree, we're just moving
> > all ids from of_device_id to i2c_device_id while also
> > adding the following specific ids which should be used
> > from now on:
> > 
> > 	{ "edt-ft5206", 0, }
> > 	{ "edt-ft5306", 0, }
> > 	{ "edt-ft5406", 0, }
> 
> Is this a tee-wide change? Link to the discussion?

nope, just found it with my AM437x Starter Kit.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 06/10] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2830 bytes --]

Hi,

On Tue, Nov 18, 2014 at 01:09:22PM -0600, Felipe Balbi wrote:
> On Tue, Nov 18, 2014 at 10:38:47AM -0800, Dmitry Torokhov wrote:
> > Hi Felipe,
> > 
> > On Tue, Nov 18, 2014 at 12:07:04PM -0600, Felipe Balbi wrote:
> > > i2c devices match against struct i2c_device_id
> > > even for CONFIG_OF case, so adding a struct of_device_id
> > > doesn't change anything. As a result, currently, edt-ft5x06
> > > will not autoprobe if built as a module.
> > 
> > Why doe snot it autoprobe? We properly declare MODULE_DEVICE_TABLE for
> > OF, is it because we are missing some data in device uevent?
> 
> because of of_i2c_register_devices(). Maybe Wolfram can give a better
> explanation here, but it just doesn't match through of_device_id.
> 
> Apply this:
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 2f90ac6..f0dc16e 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -449,8 +449,10 @@ static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
>  						const struct i2c_client *client)
>  {
>  	while (id->name[0]) {
> -		if (strcmp(client->name, id->name) == 0)
> +		if (strcmp(client->name, id->name) == 0) {
> +			dev_info(&client->dev, "i2c_device_id match\n");
>  			return id;
> +		}
>  		id++;
>  	}
>  	return NULL;
> @@ -465,8 +467,10 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
>  		return 0;
>  
>  	/* Attempt an OF style match */
> -	if (of_driver_match_device(dev, drv))
> +	if (of_driver_match_device(dev, drv)) {
> +		dev_info(dev, "of driver match\n");
>  		return 1;
> +	}
>  
>  	/* Then ACPI style match */
>  	if (acpi_driver_match_device(dev, drv))
> @@ -1081,6 +1085,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
>  	client->dev.bus = &i2c_bus_type;
>  	client->dev.type = &i2c_client_type;
>  	client->dev.of_node = info->of_node;
> +	dev_info(&adap->dev, "%s: of_node %p\n", __func__, info->of_node);
>  	ACPI_COMPANION_SET(&client->dev, info->acpi_node.companion);
>  
>  	i2c_dev_set_name(adap, client);
> @@ -1411,6 +1416,7 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
>  
>  		info.irq = irq_of_parse_and_map(node, 0);
>  		info.of_node = of_node_get(node);
> +		dev_info(&adap->dev, "%s: of_node %p\n", __func__, info.of_node);
>  		info.archdata = &dev_ad;
>  
>  		if (of_get_property(node, "wakeup-source", NULL))
> 
> then boot the board and you get http://hastebin.com/oqemezajez
> 
> Interesting, it's matching against of but only when I modprobe.
> 
> Let me debug this one a little more.

this is a simpler patch:

+MODULE_ALIAS("i2c:edt-ft5206");
+MODULE_ALIAS("i2c:edt-ft5306");
+MODULE_ALIAS("i2c:edt-ft5406");

I'll send it in a bit.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

Because with OF we can pass more specific
compatible flags (such as edt-ft5306) instead
of generic edt-ft5x06, when i2c-core's
of_i2c_register_devices() tries to request_module(),
it'll request it with a non-existent specific module
alias.

In order to have this driver autoprobing again, we
just need to add missing MODULE_ALIAS() entries to
edt-ft5x06 driver.

Thanks to Dmitry for noticing that it actually should
autoprobe even with of_device_id.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: <linux-input@vger.kernel.org>
Cc: <stable@vger.kernel.org> # v3.15+
Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index ee3434f..bcbf688 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1147,6 +1147,9 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
 
 module_i2c_driver(edt_ft5x06_ts_driver);
 
+MODULE_ALIAS("i2c:edt-ft5206");
+MODULE_ALIAS("i2c:edt-ft5306");
+MODULE_ALIAS("i2c:edt-ft5406");
 MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
 MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
 MODULE_LICENSE("GPL");
-- 
2.1.0.GIT

Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On Tue, Nov 18, 2014 at 01:27:42PM -0600, Felipe Balbi wrote:
> Because with OF we can pass more specific
> compatible flags (such as edt-ft5306) instead
> of generic edt-ft5x06, when i2c-core's
> of_i2c_register_devices() tries to request_module(),
> it'll request it with a non-existent specific module
> alias.
> 
> In order to have this driver autoprobing again, we
> just need to add missing MODULE_ALIAS() entries to
> edt-ft5x06 driver.
> 
> Thanks to Dmitry for noticing that it actually should
> autoprobe even with of_device_id.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: <linux-input@vger.kernel.org>
> Cc: <stable@vger.kernel.org> # v3.15+
> Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---


btw, with this, patches 7 through 10 can be ignored.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Dmitry Torokhov]

On Tue, Nov 18, 2014 at 01:27:42PM -0600, Felipe Balbi wrote:
> Because with OF we can pass more specific
> compatible flags (such as edt-ft5306) instead
> of generic edt-ft5x06, when i2c-core's
> of_i2c_register_devices() tries to request_module(),
> it'll request it with a non-existent specific module
> alias.
> 
> In order to have this driver autoprobing again, we
> just need to add missing MODULE_ALIAS() entries to
> edt-ft5x06 driver.
> 
> Thanks to Dmitry for noticing that it actually should
> autoprobe even with of_device_id.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: <linux-input@vger.kernel.org>
> Cc: <stable@vger.kernel.org> # v3.15+
> Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/input/touchscreen/edt-ft5x06.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index ee3434f..bcbf688 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -1147,6 +1147,9 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
>  
>  module_i2c_driver(edt_ft5x06_ts_driver);
>  
> +MODULE_ALIAS("i2c:edt-ft5206");
> +MODULE_ALIAS("i2c:edt-ft5306");
> +MODULE_ALIAS("i2c:edt-ft5406");
>  MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
>  MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
>  MODULE_LICENSE("GPL");
> -- 
> 2.1.0.GIT
> 

Here is what I see if I run modinfo on the module:

dtor@dtor-ws:~/kernel/work$ modinfo
drivers/input/touchscreen/edt-ft5x06.ko
filename:
/home/dtor/kernel/work/drivers/input/touchscreen/edt-ft5x06.ko
license:        GPL
description:    EDT FT5x06 I2C Touchscreen Driver
author:         Simon Budig <simon.budig@kernelconcepts.de>
alias:          i2c:edt-ft5x06
alias:          of:N*T*Cedt,edt-ft5406*
alias:          of:N*T*Cedt,edt-ft5306*
alias:          of:N*T*Cedt,edt-ft5206*
depends:        i2c-core
intree:         Y
vermagic:       3.17.0+ SMP preempt mod_unload 

As you can see we already have what I consider proper modaliases for the
driver. Why don't they work? Is it because modprobe doe snot know how to
handle OF-style modaliases or device's uevents are missing OF data in
them?

Thanks.

-- 
Dmitry

Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2660 bytes --]

On Tue, Nov 18, 2014 at 11:59:02AM -0800, Dmitry Torokhov wrote:
> On Tue, Nov 18, 2014 at 01:27:42PM -0600, Felipe Balbi wrote:
> > Because with OF we can pass more specific
> > compatible flags (such as edt-ft5306) instead
> > of generic edt-ft5x06, when i2c-core's
> > of_i2c_register_devices() tries to request_module(),
> > it'll request it with a non-existent specific module
> > alias.
> > 
> > In order to have this driver autoprobing again, we
> > just need to add missing MODULE_ALIAS() entries to
> > edt-ft5x06 driver.
> > 
> > Thanks to Dmitry for noticing that it actually should
> > autoprobe even with of_device_id.
> > 
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: <linux-input@vger.kernel.org>
> > Cc: <stable@vger.kernel.org> # v3.15+
> > Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  drivers/input/touchscreen/edt-ft5x06.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > index ee3434f..bcbf688 100644
> > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > @@ -1147,6 +1147,9 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
> >  
> >  module_i2c_driver(edt_ft5x06_ts_driver);
> >  
> > +MODULE_ALIAS("i2c:edt-ft5206");
> > +MODULE_ALIAS("i2c:edt-ft5306");
> > +MODULE_ALIAS("i2c:edt-ft5406");
> >  MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
> >  MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
> >  MODULE_LICENSE("GPL");
> > -- 
> > 2.1.0.GIT
> > 
> 
> Here is what I see if I run modinfo on the module:
> 
> dtor@dtor-ws:~/kernel/work$ modinfo
> drivers/input/touchscreen/edt-ft5x06.ko
> filename:
> /home/dtor/kernel/work/drivers/input/touchscreen/edt-ft5x06.ko
> license:        GPL
> description:    EDT FT5x06 I2C Touchscreen Driver
> author:         Simon Budig <simon.budig@kernelconcepts.de>
> alias:          i2c:edt-ft5x06
> alias:          of:N*T*Cedt,edt-ft5406*
> alias:          of:N*T*Cedt,edt-ft5306*
> alias:          of:N*T*Cedt,edt-ft5206*
> depends:        i2c-core
> intree:         Y
> vermagic:       3.17.0+ SMP preempt mod_unload 
> 
> As you can see we already have what I consider proper modaliases for the
> driver. Why don't they work? Is it because modprobe doe snot know how to
> handle OF-style modaliases or device's uevents are missing OF data in
> them?

no, it's because i2c call request_module for i2c:edt-ft5306. i2c core
never uses any of the of aliases.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Dmitry Torokhov]

On Tue, Nov 18, 2014 at 02:03:40PM -0600, Felipe Balbi wrote:
> On Tue, Nov 18, 2014 at 11:59:02AM -0800, Dmitry Torokhov wrote:
> > On Tue, Nov 18, 2014 at 01:27:42PM -0600, Felipe Balbi wrote:
> > > Because with OF we can pass more specific
> > > compatible flags (such as edt-ft5306) instead
> > > of generic edt-ft5x06, when i2c-core's
> > > of_i2c_register_devices() tries to request_module(),
> > > it'll request it with a non-existent specific module
> > > alias.
> > > 
> > > In order to have this driver autoprobing again, we
> > > just need to add missing MODULE_ALIAS() entries to
> > > edt-ft5x06 driver.
> > > 
> > > Thanks to Dmitry for noticing that it actually should
> > > autoprobe even with of_device_id.
> > > 
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Cc: <linux-input@vger.kernel.org>
> > > Cc: <stable@vger.kernel.org> # v3.15+
> > > Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > ---
> > >  drivers/input/touchscreen/edt-ft5x06.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > > index ee3434f..bcbf688 100644
> > > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > > @@ -1147,6 +1147,9 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
> > >  
> > >  module_i2c_driver(edt_ft5x06_ts_driver);
> > >  
> > > +MODULE_ALIAS("i2c:edt-ft5206");
> > > +MODULE_ALIAS("i2c:edt-ft5306");
> > > +MODULE_ALIAS("i2c:edt-ft5406");
> > >  MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
> > >  MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
> > >  MODULE_LICENSE("GPL");
> > > -- 
> > > 2.1.0.GIT
> > > 
> > 
> > Here is what I see if I run modinfo on the module:
> > 
> > dtor@dtor-ws:~/kernel/work$ modinfo
> > drivers/input/touchscreen/edt-ft5x06.ko
> > filename:
> > /home/dtor/kernel/work/drivers/input/touchscreen/edt-ft5x06.ko
> > license:        GPL
> > description:    EDT FT5x06 I2C Touchscreen Driver
> > author:         Simon Budig <simon.budig@kernelconcepts.de>
> > alias:          i2c:edt-ft5x06
> > alias:          of:N*T*Cedt,edt-ft5406*
> > alias:          of:N*T*Cedt,edt-ft5306*
> > alias:          of:N*T*Cedt,edt-ft5206*
> > depends:        i2c-core
> > intree:         Y
> > vermagic:       3.17.0+ SMP preempt mod_unload 
> > 
> > As you can see we already have what I consider proper modaliases for the
> > driver. Why don't they work? Is it because modprobe doe snot know how to
> > handle OF-style modaliases or device's uevents are missing OF data in
> > them?
> 
> no, it's because i2c call request_module for i2c:edt-ft5306. i2c core
> never uses any of the of aliases.

1. I think both i2c and spi are cheating in this regard: they should
rely on normal driver request paths through udev.

2. I2C should emit proper modalias for devices coming by the way of
devicetree. It already does special handling for ACPI, it needs to do
the same for OF. I think SPI does the right thing there.

Thanks.

-- 
Dmitry

Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3438 bytes --]

On Tue, Nov 18, 2014 at 12:14:21PM -0800, Dmitry Torokhov wrote:
> On Tue, Nov 18, 2014 at 02:03:40PM -0600, Felipe Balbi wrote:
> > On Tue, Nov 18, 2014 at 11:59:02AM -0800, Dmitry Torokhov wrote:
> > > On Tue, Nov 18, 2014 at 01:27:42PM -0600, Felipe Balbi wrote:
> > > > Because with OF we can pass more specific
> > > > compatible flags (such as edt-ft5306) instead
> > > > of generic edt-ft5x06, when i2c-core's
> > > > of_i2c_register_devices() tries to request_module(),
> > > > it'll request it with a non-existent specific module
> > > > alias.
> > > > 
> > > > In order to have this driver autoprobing again, we
> > > > just need to add missing MODULE_ALIAS() entries to
> > > > edt-ft5x06 driver.
> > > > 
> > > > Thanks to Dmitry for noticing that it actually should
> > > > autoprobe even with of_device_id.
> > > > 
> > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > Cc: <linux-input@vger.kernel.org>
> > > > Cc: <stable@vger.kernel.org> # v3.15+
> > > > Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
> > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > ---
> > > >  drivers/input/touchscreen/edt-ft5x06.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > > > index ee3434f..bcbf688 100644
> > > > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > > > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > > > @@ -1147,6 +1147,9 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
> > > >  
> > > >  module_i2c_driver(edt_ft5x06_ts_driver);
> > > >  
> > > > +MODULE_ALIAS("i2c:edt-ft5206");
> > > > +MODULE_ALIAS("i2c:edt-ft5306");
> > > > +MODULE_ALIAS("i2c:edt-ft5406");
> > > >  MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
> > > >  MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
> > > >  MODULE_LICENSE("GPL");
> > > > -- 
> > > > 2.1.0.GIT
> > > > 
> > > 
> > > Here is what I see if I run modinfo on the module:
> > > 
> > > dtor@dtor-ws:~/kernel/work$ modinfo
> > > drivers/input/touchscreen/edt-ft5x06.ko
> > > filename:
> > > /home/dtor/kernel/work/drivers/input/touchscreen/edt-ft5x06.ko
> > > license:        GPL
> > > description:    EDT FT5x06 I2C Touchscreen Driver
> > > author:         Simon Budig <simon.budig@kernelconcepts.de>
> > > alias:          i2c:edt-ft5x06
> > > alias:          of:N*T*Cedt,edt-ft5406*
> > > alias:          of:N*T*Cedt,edt-ft5306*
> > > alias:          of:N*T*Cedt,edt-ft5206*
> > > depends:        i2c-core
> > > intree:         Y
> > > vermagic:       3.17.0+ SMP preempt mod_unload 
> > > 
> > > As you can see we already have what I consider proper modaliases for the
> > > driver. Why don't they work? Is it because modprobe doe snot know how to
> > > handle OF-style modaliases or device's uevents are missing OF data in
> > > them?
> > 
> > no, it's because i2c call request_module for i2c:edt-ft5306. i2c core
> > never uses any of the of aliases.
> 
> 1. I think both i2c and spi are cheating in this regard: they should
> rely on normal driver request paths through udev.
> 
> 2. I2C should emit proper modalias for devices coming by the way of
> devicetree. It already does special handling for ACPI, it needs to do
> the same for OF. I think SPI does the right thing there.

I'll let Wolfram comment here for I2C.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3903 bytes --]

Hi Wolfram,

On Tue, Nov 18, 2014 at 02:19:07PM -0600, Felipe Balbi wrote:
> On Tue, Nov 18, 2014 at 12:14:21PM -0800, Dmitry Torokhov wrote:
> > On Tue, Nov 18, 2014 at 02:03:40PM -0600, Felipe Balbi wrote:
> > > On Tue, Nov 18, 2014 at 11:59:02AM -0800, Dmitry Torokhov wrote:
> > > > On Tue, Nov 18, 2014 at 01:27:42PM -0600, Felipe Balbi wrote:
> > > > > Because with OF we can pass more specific
> > > > > compatible flags (such as edt-ft5306) instead
> > > > > of generic edt-ft5x06, when i2c-core's
> > > > > of_i2c_register_devices() tries to request_module(),
> > > > > it'll request it with a non-existent specific module
> > > > > alias.
> > > > > 
> > > > > In order to have this driver autoprobing again, we
> > > > > just need to add missing MODULE_ALIAS() entries to
> > > > > edt-ft5x06 driver.
> > > > > 
> > > > > Thanks to Dmitry for noticing that it actually should
> > > > > autoprobe even with of_device_id.
> > > > > 
> > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > > Cc: <linux-input@vger.kernel.org>
> > > > > Cc: <stable@vger.kernel.org> # v3.15+
> > > > > Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
> > > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > > ---
> > > > >  drivers/input/touchscreen/edt-ft5x06.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > > > > index ee3434f..bcbf688 100644
> > > > > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > > > > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > > > > @@ -1147,6 +1147,9 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
> > > > >  
> > > > >  module_i2c_driver(edt_ft5x06_ts_driver);
> > > > >  
> > > > > +MODULE_ALIAS("i2c:edt-ft5206");
> > > > > +MODULE_ALIAS("i2c:edt-ft5306");
> > > > > +MODULE_ALIAS("i2c:edt-ft5406");
> > > > >  MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
> > > > >  MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
> > > > >  MODULE_LICENSE("GPL");
> > > > > -- 
> > > > > 2.1.0.GIT
> > > > > 
> > > > 
> > > > Here is what I see if I run modinfo on the module:
> > > > 
> > > > dtor@dtor-ws:~/kernel/work$ modinfo
> > > > drivers/input/touchscreen/edt-ft5x06.ko
> > > > filename:
> > > > /home/dtor/kernel/work/drivers/input/touchscreen/edt-ft5x06.ko
> > > > license:        GPL
> > > > description:    EDT FT5x06 I2C Touchscreen Driver
> > > > author:         Simon Budig <simon.budig@kernelconcepts.de>
> > > > alias:          i2c:edt-ft5x06
> > > > alias:          of:N*T*Cedt,edt-ft5406*
> > > > alias:          of:N*T*Cedt,edt-ft5306*
> > > > alias:          of:N*T*Cedt,edt-ft5206*
> > > > depends:        i2c-core
> > > > intree:         Y
> > > > vermagic:       3.17.0+ SMP preempt mod_unload 
> > > > 
> > > > As you can see we already have what I consider proper modaliases for the
> > > > driver. Why don't they work? Is it because modprobe doe snot know how to
> > > > handle OF-style modaliases or device's uevents are missing OF data in
> > > > them?
> > > 
> > > no, it's because i2c call request_module for i2c:edt-ft5306. i2c core
> > > never uses any of the of aliases.
> > 
> > 1. I think both i2c and spi are cheating in this regard: they should
> > rely on normal driver request paths through udev.
> > 
> > 2. I2C should emit proper modalias for devices coming by the way of
> > devicetree. It already does special handling for ACPI, it needs to do
> > the same for OF. I think SPI does the right thing there.
> 
> I'll let Wolfram comment here for I2C.

Can you comment on this ? As of today i2c-based drivers will probe with
DT-based boot if DT's compatible is set to the same id as in struct
i2c_device_id. No i2c driver with of_device_id can autoprobe using OF
ids.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]


> Can you comment on this ? As of today i2c-based drivers will probe with
> DT-based boot if DT's compatible is set to the same id as in struct
> i2c_device_id. No i2c driver with of_device_id can autoprobe using OF
> ids.

Long standing problem. Last one to tackle this was Lee Jones:

https://lkml.org/lkml/2014/8/28/283


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]