[PATCH 0/3] Add device tree support for Goodix GT9xx series touchscreen controller

[PATCH 0/3] Add device tree support for Goodix GT9xx series touchscreen controller

[Aleksei Mamlin]

Provide device tree support and binding information.

Aleksei Mamlin (3):
  input: goodix: Add device tree support
  dt-bindings: Add vendor-prefix for Goodix
  input: goodix: Add bindings documentation

 .../bindings/input/touchscreen/gt9xx.txt           | 23 ++++++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 drivers/input/touchscreen/goodix.c                 | 10 ++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/gt9xx.txt

-- 
2.0.5

[PATCH 1/3] input: goodix: Add device tree support

[Aleksei Mamlin]

Signed-off-by: Aleksei Mamlin <mamlinav-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/input/touchscreen/goodix.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index ca19668..603d2ae 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -23,6 +23,7 @@
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <asm/unaligned.h>
 
 struct goodix_ts_data {
@@ -378,6 +379,14 @@ static const struct acpi_device_id goodix_acpi_match[] = {
 };
 MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
 
+#ifdef CONFIG_OF
+static struct of_device_id goodix_of_match[] = {
+	{ .compatible = "goodix,gt9xx" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, goodix_of_match);
+#endif
+
 static struct i2c_driver goodix_ts_driver = {
 	.probe = goodix_ts_probe,
 	.id_table = goodix_ts_id,
@@ -385,6 +394,7 @@ static struct i2c_driver goodix_ts_driver = {
 		.name = "Goodix-TS",
 		.owner = THIS_MODULE,
 		.acpi_match_table = goodix_acpi_match,
+		.of_match_table = of_match_ptr(goodix_of_match),
 	},
 };
 module_i2c_driver(goodix_ts_driver);
-- 
2.0.5

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

[PATCH 2/3] dt-bindings: Add vendor-prefix for Goodix

[Aleksei Mamlin]

Signed-off-by: Aleksei Mamlin <mamlinav@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b1df0ad..72136f1 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -64,6 +64,7 @@ gef	GE Fanuc Intelligent Platforms Embedded Systems, Inc.
 geniatech	Geniatech, Inc.
 globalscale	Globalscale Technologies, Inc.
 gmt	Global Mixed-mode Technology, Inc.
+goodix	Shenzhen Huiding Technology Co. Ltd.
 google	Google, Inc.
 gumstix	Gumstix, Inc.
 gw	Gateworks Corporation
-- 
2.0.5

[PATCH 3/3] input: goodix: Add bindings documentation

[Aleksei Mamlin]

Signed-off-by: Aleksei Mamlin <mamlinav@gmail.com>
---
 .../bindings/input/touchscreen/gt9xx.txt           | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/gt9xx.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/gt9xx.txt b/Documentation/devicetree/bindings/input/touchscreen/gt9xx.txt
new file mode 100644
index 0000000..96c17f0
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/gt9xx.txt
@@ -0,0 +1,23 @@
+Device tree bindings for Goodix GT9xx series touchscreen controller
+
+Required properties:
+
+ - compatible		: Should be "goodix,gt9xx"
+ - reg			: I2C address of the chip
+ - interrupt-parent 	: Interrupt controller to which the chip is connected
+ - interrupts		: Interrupt to which the chip is connected
+
+Example:
+
+	i2c@00000000 {
+		/* ... */
+
+		gt9xx@5d {
+			compatible = "goodix,gt9xx";
+			reg = <0x5d>;
+			interrupt-parent = <&gpio>;
+			interrupts = <0 2>;
+		};
+
+		/* ... */
+	};
-- 
2.0.5

Re: [PATCH 0/3] Add device tree support for Goodix GT9xx series touchscreen controller

[Bastien Nocera]

On Sat, 2015-01-17 at 17:34 +0300, Aleksei Mamlin wrote:
> Provide device tree support and binding information.

What hardware is this enabling support for? The commit messages lack
that information.

> Aleksei Mamlin (3):
>   input: goodix: Add device tree support
>   dt-bindings: Add vendor-prefix for Goodix
>   input: goodix: Add bindings documentation
> 
>  .../bindings/input/touchscreen/gt9xx.txt           | 23 ++++++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  drivers/input/touchscreen/goodix.c                 | 10 ++++++++++
>  3 files changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/gt9xx.txt
> 

Re: [PATCH 1/3] input: goodix: Add device tree support

[Bastien Nocera]

On Sat, 2015-01-17 at 17:36 +0300, Aleksei Mamlin wrote:
> Signed-off-by: Aleksei Mamlin <mamlinav-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/input/touchscreen/goodix.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index ca19668..603d2ae 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -23,6 +23,7 @@
>  #include <linux/irq.h>
>  #include <linux/interrupt.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
>  #include <asm/unaligned.h>
>  
>  struct goodix_ts_data {
> @@ -378,6 +379,14 @@ static const struct acpi_device_id goodix_acpi_match[] = {
>  };
>  MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
>  
> +#ifdef CONFIG_OF
> +static struct of_device_id goodix_of_match[] = {
> +	{ .compatible = "goodix,gt9xx" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, goodix_of_match);
> +#endif
> +
>  static struct i2c_driver goodix_ts_driver = {
>  	.probe = goodix_ts_probe,
>  	.id_table = goodix_ts_id,
> @@ -385,6 +394,7 @@ static struct i2c_driver goodix_ts_driver = {
>  		.name = "Goodix-TS",
>  		.owner = THIS_MODULE,
>  		.acpi_match_table = goodix_acpi_match,
> +		.of_match_table = of_match_ptr(goodix_of_match),

Pretty sure that wouldn't compile if CONFIG_OF is disabled.

>  	},
>  };
>  module_i2c_driver(goodix_ts_driver);


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

Re: [PATCH 1/3] input: goodix: Add device tree support

[Алексей Мамлин]

Look at include/linux/of.h

#ifdef CONFIG_OF
 ...
#define of_match_ptr(_ptr)	(_ptr)
 ...
#else /* CONFIG_OF */
 ...
#define of_match_ptr(_ptr)	NULL
 ...
#endif /* CONFIG_OF */

So it would be compiled. Similarly in other touchscreen drivers.
But Kconfig should be fixed to enable CONFIG_OF. Will be fixed in v2.

2015-01-17 23:38 GMT+03:00, Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>:
> On Sat, 2015-01-17 at 17:36 +0300, Aleksei Mamlin wrote:
>> Signed-off-by: Aleksei Mamlin <mamlinav-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/input/touchscreen/goodix.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/goodix.c
>> b/drivers/input/touchscreen/goodix.c
>> index ca19668..603d2ae 100644
>> --- a/drivers/input/touchscreen/goodix.c
>> +++ b/drivers/input/touchscreen/goodix.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/irq.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/slab.h>
>> +#include <linux/of.h>
>>  #include <asm/unaligned.h>
>>
>>  struct goodix_ts_data {
>> @@ -378,6 +379,14 @@ static const struct acpi_device_id
>> goodix_acpi_match[] = {
>>  };
>>  MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
>>
>> +#ifdef CONFIG_OF
>> +static struct of_device_id goodix_of_match[] = {
>> +	{ .compatible = "goodix,gt9xx" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, goodix_of_match);
>> +#endif
>> +
>>  static struct i2c_driver goodix_ts_driver = {
>>  	.probe = goodix_ts_probe,
>>  	.id_table = goodix_ts_id,
>> @@ -385,6 +394,7 @@ static struct i2c_driver goodix_ts_driver = {
>>  		.name = "Goodix-TS",
>>  		.owner = THIS_MODULE,
>>  		.acpi_match_table = goodix_acpi_match,
>> +		.of_match_table = of_match_ptr(goodix_of_match),
>
> Pretty sure that wouldn't compile if CONFIG_OF is disabled.
>
>>  	},
>>  };
>>  module_i2c_driver(goodix_ts_driver);
>
>
>


-- 
Thanks and regards,
Aleksei Mamlin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] input: goodix: Add device tree support

[Antonio Ospite]

On Sun, 18 Jan 2015 16:25:35 +0400
Алексей Мамлин <mamlinav@gmail.com> wrote:

> Look at include/linux/of.h
> 
> #ifdef CONFIG_OF
>  ...
> #define of_match_ptr(_ptr)	(_ptr)
>  ...
> #else /* CONFIG_OF */
>  ...
> #define of_match_ptr(_ptr)	NULL
>  ...
> #endif /* CONFIG_OF */
> 
> So it would be compiled. Similarly in other touchscreen drivers.
> But Kconfig should be fixed to enable CONFIG_OF. Will be fixed in v2.
> 

I was thinking, instead of depending on (ACPI || OF), can we make ACPI
optional as well and remove the dependency on it?
Does it make sense to allow selecting a driver even with no
enabled mechanism to pass platform data to it?

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
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 1/3] input: goodix: Add device tree support

[Aleksei Mamlin]

Do you mean that I should use #ifdef CONFIG_ACPI and ACPI_PTR?

#ifdef CONFIG_ACPI
static const struct acpi_device_id goodix_acpi_match[] = {
	{ "GDIX1001", 0 },
	{ }
};
MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
#endif

#ifdef CONFIG_OF
static struct of_device_id goodix_of_match[] = {
	{ .compatible = "goodix,gt9xx" },
	{ }
};
MODULE_DEVICE_TABLE(of, goodix_of_match);
#endif

static struct i2c_driver goodix_ts_driver = {
	...
		.acpi_match_table = ACPI_PTR(goodix_acpi_match),
		.of_match_table = of_match_ptr(goodix_of_match),
}

2015-01-18 17:22 GMT+03:00, Antonio Ospite <ao2@ao2.it>:
> On Sun, 18 Jan 2015 16:25:35 +0400
> Алексей Мамлин <mamlinav@gmail.com> wrote:
>
>> Look at include/linux/of.h
>>
>> #ifdef CONFIG_OF
>>  ...
>> #define of_match_ptr(_ptr)	(_ptr)
>>  ...
>> #else /* CONFIG_OF */
>>  ...
>> #define of_match_ptr(_ptr)	NULL
>>  ...
>> #endif /* CONFIG_OF */
>>
>> So it would be compiled. Similarly in other touchscreen drivers.
>> But Kconfig should be fixed to enable CONFIG_OF. Will be fixed in v2.
>>
>
> I was thinking, instead of depending on (ACPI || OF), can we make ACPI
> optional as well and remove the dependency on it?
> Does it make sense to allow selecting a driver even with no
> enabled mechanism to pass platform data to it?
>
> Thanks,
>    Antonio
>
> --
> Antonio Ospite
> http://ao2.it
>
> A: Because it messes up the order in which people normally read text.
>    See http://en.wikipedia.org/wiki/Posting_style
> Q: Why is top-posting such a bad thing?
>


-- 
Thanks and regards,
Aleksei Mamlin
--
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 3/3] input: goodix: Add bindings documentation

[Arnd Bergmann]

On Saturday 17 January 2015 17:36:50 Aleksei Mamlin wrote:
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/gt9xx.txt
> @@ -0,0 +1,23 @@
> +Device tree bindings for Goodix GT9xx series touchscreen controller
> +
> +Required properties:
> +
> + - compatible          : Should be "goodix,gt9xx"
> + - reg                 : I2C address of the chip
> + - interrupt-parent    : Interrupt controller to which the chip is connected
> + - interrupts          : Interrupt to which the chip is connected
> +
> 

You should avoid the use of wildcards in compatible strings. Better list
all the known part numbers, or (if they are 100% compatible with one another)
pick the oldest one we support and use that as the compatible string.

	Arnd