[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

[PATCH 07/10] arm: boot: dts: am437x-sk: switch over to new edt-ft5306 compatible

[Felipe Balbi]

now that we have the new vendor-less edt-ft5306
i2c_device_id, we can use it

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/boot/dts/am437x-sk-evm.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
index 859ff3d..26133cb 100644
--- a/arch/arm/boot/dts/am437x-sk-evm.dts
+++ b/arch/arm/boot/dts/am437x-sk-evm.dts
@@ -404,7 +404,7 @@
 
 	edt-ft5306@38 {
 		status = "okay";
-		compatible = "edt,edt-ft5306", "edt,edt-ft5x06";
+		compatible = "edt-ft5306", "edt-ft5x06";
 		pinctrl-names = "default";
 		pinctrl-0 = <&edt_ft5306_ts_pins>;
 
-- 
2.1.0.GIT

[PATCH 08/10] arm: boot: dts: imx28-tx28: switch over to new edt-ft5x06 compatible

[Felipe Balbi]

now that we have the new vendor-less edt-ft5x06
i2c_device_id, we can use it

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/boot/dts/imx28-tx28.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx28-tx28.dts b/arch/arm/boot/dts/imx28-tx28.dts
index a5b27c8..5c5c988 100644
--- a/arch/arm/boot/dts/imx28-tx28.dts
+++ b/arch/arm/boot/dts/imx28-tx28.dts
@@ -319,7 +319,7 @@
 	};
 
 	polytouch: edt-ft5x06@38 {
-		compatible = "edt,edt-ft5x06";
+		compatible = "edt-ft5x06";
 		reg = <0x38>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&tx28_edt_ft5x06_pins>;
-- 
2.1.0.GIT

[PATCH 09/10] arm: boot: dts: imx53-tx53-x03x: switch over to new edt-ft5x06 compatible

[Felipe Balbi]

now that we have the new vendor-less edt-ft5x06
i2c_device_id, we can use it

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/boot/dts/imx53-tx53-x03x.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx53-tx53-x03x.dts b/arch/arm/boot/dts/imx53-tx53-x03x.dts
index 3b73e81..27bf29d 100644
--- a/arch/arm/boot/dts/imx53-tx53-x03x.dts
+++ b/arch/arm/boot/dts/imx53-tx53-x03x.dts
@@ -211,7 +211,7 @@
 	};
 
 	polytouch: edt-ft5x06@38 {
-		compatible = "edt,edt-ft5x06";
+		compatible = "edt-ft5x06";
 		reg = <0x38>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_edt_ft5x06_1>;
-- 
2.1.0.GIT

[PATCH 10/10] arm: boot: dts: imx6qdl-tx6: switch over to new edt-ft5x06 compatible

[Felipe Balbi]

now that we have the new vendor-less edt-ft5x06
i2c_device_id, we can use it

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/boot/dts/imx6qdl-tx6.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-tx6.dtsi b/arch/arm/boot/dts/imx6qdl-tx6.dtsi
index f02b80b..5ecf3bf 100644
--- a/arch/arm/boot/dts/imx6qdl-tx6.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tx6.dtsi
@@ -267,7 +267,7 @@
 	};
 
 	polytouch: edt-ft5x06@38 {
-		compatible = "edt,edt-ft5x06";
+		compatible = "edt-ft5x06";
 		reg = <0x38>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_edt_ft5x06>;
-- 
2.1.0.GIT

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 --]