input: Add support for ChipOne icn8318 based touchscreens

PATCH Superseded Hans de Goede 2 participants 6 messages Mar 10, 2015 – Mar 24, 2015

Subsystems (4)

Device tree bindings HID Input drivers Chipone ICN8318 I2C touchscreen driver

lore

Share & tools

lore

Hans de Goede

author

2015-03-10 17:05 UTC #0 lore raw

PATCH v3 1/1 input: Add support for ChipOne icn8318 based touchscreens

The ChipOne icn8318 is an i2c capacitive touchscreen controller typically used in cheap android tablets, this commit adds a driver for it.

Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:
-Fix spurious events after a close + open of the evdev node. Changes in v3:
-Add a msleep after waking the device on open, to wait for it to be ready
for i2c communications, otherwise we get communication errors on device open

---
 .../bindings/input/touchscreen/chipone_icn8318.txt |  46 +++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 MAINTAINERS                                        |   7 +
 drivers/input/touchscreen/Kconfig                  |  13 +
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/chipone_icn8318.c        | 318 +++++++++++++++++++++
 6 files changed, 386 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/chipone_icn8318.txt
 create mode 100644 drivers/input/touchscreen/chipone_icn8318.c

Documentation/devicetree/bindings/input/touchscreen/chipone_icn8318.txt new file +46 -0

new file mode 100644
index 0000000..b405493
@@ -0,0 +1,46 @@
+* ChipOne icn8318 I2C touchscreen controller
+
+Required properties:
+ - compatible		  : "chipone,icn8318"
+ - reg			  : I2C slave address of the chip (0x40)
+ - interrupt-parent	  : a phandle pointing to the interrupt controller
+			    serving the interrupt for this chip
+ - interrupts		  : interrupt specification for the icn8318 interrupt
+ - wake-gpios		  : GPIO specification for the WAKE input
+ - touchscreen-size-x	  : horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y	  : vertical resolution of touchscreen (in pixels)
+
+Optional properties:
+ - pinctrl-names	  : should be "default"
+ - pinctrl-0:		  : a phandle pointing to the pin settings for the
+			    control gpios
+ - touchscreen-fuzz-x	  : horizontal noise value of the absolute input
+			    device (in pixels)
+ - touchscreen-fuzz-y	  : vertical noise value of the absolute input
+			    device (in pixels)
+ - touchscreen-inverted-x : X axis is inverted (boolean)
+ - touchscreen-inverted-y : Y axis is inverted (boolean)
+ - touchscreen-swap-x-y	  : X and Y axis are swapped (boolean)
+			    Swapping is done after inverting the axis
+
+Example:
+
+i2c at 00000000 {
+	/* ... */
+
+	chipone_icn8318 at 40 {
+		compatible = "chipone,icn8318";
+		reg = <0x40>;
+		interrupt-parent = <&pio>;
+		interrupts = <9 IRQ_TYPE_EDGE_FALLING>; /* EINT9 (PG9) */
+		pinctrl-names = "default";
+		pinctrl-0 = <&ts_wake_pin_p66>;
+		wake-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>; /* PB3 */
+		touchscreen-size-x = <800>;
+		touchscreen-size-y = <480>;
+		touchscreen-inverted-x;
+		touchscreen-swap-x-y;
+	};
+
+	/* ... */
+};

Documentation/devicetree/bindings/vendor-prefixes.txt +1 -0

index 389ca13..b5dea36 100644
@@ -35,6 +35,7 @@ capella	Capella Microsystems, Inc
 cavium	Cavium, Inc.
 cdns	Cadence Design Systems Inc.
 chipidea	Chipidea, Inc
+chipone		ChipOne
 chipspark	ChipSPARK
 chrp	Common Hardware Reference Platform
 chunghwa	Chunghwa Picture Tubes Ltd.

MAINTAINERS +7 -0

index ddc5a8c..d39304c 100644
@@ -2512,6 +2512,13 @@ L:	linux-usb at vger.kernel.org
 S:	Maintained
 F:	drivers/usb/chipidea/
 
+CHIPONE ICN8318 I2C TOUCHSCREEN DRIVER
+M:	Hans de Goede <[email protected]>
+L:	linux-input at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/touchscreen/chipone_icn8318.txt
+F:	drivers/input/touchscreen/chipone_icn8318.c
+
 CHROME HARDWARE PLATFORM SUPPORT
 M:	Olof Johansson <[email protected]>
 S:	Maintained

drivers/input/touchscreen/Kconfig +13 -0

index 5891752..19ca23e 100644
@@ -140,6 +140,19 @@ config TOUCHSCREEN_BU21013
 	  To compile this driver as a module, choose M here: the
 	  module will be called bu21013_ts.
 
+config TOUCHSCREEN_CHIPONE_ICN8318
+	tristate "chipone icn8318 touchscreen controller"
+	depends on GPIOLIB
+	depends on I2C
+	depends on OF
+	help
+	  Say Y here if you have a ChipOne icn8318 based I2C touchscreen.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called chipone_icn8318.
+
 config TOUCHSCREEN_CY8CTMG110
 	tristate "cy8ctmg110 touchscreen"
 	depends on I2C

drivers/input/touchscreen/Makefile +1 -0

index 0242fea..9517ae4 100644
@@ -17,6 +17,7 @@ obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C)	+= ar1021_i2c.o
 obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT)	+= atmel_mxt_ts.o
 obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR)	+= auo-pixcir-ts.o
 obj-$(CONFIG_TOUCHSCREEN_BU21013)	+= bu21013_ts.o
+obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318)	+= chipone_icn8318.o
 obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110)	+= cy8ctmg110_ts.o
 obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE)	+= cyttsp_core.o
 obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C)	+= cyttsp_i2c.o cyttsp_i2c_common.o

drivers/input/touchscreen/chipone_icn8318.c new file +318 -0

318 lines not on this page · raw message

Signature · 1 line

2.3.1

Dmitry Torokhov

maintainer

2015-03-22 04:03 UTC #1 lore raw

PATCH v3 1/1 input: Add support for ChipOne icn8318 based touchscreens

Hi Hans,

drivers/input/touchscreen/chipone_icn8318.c line 272

Quoted 2 lines · Hans de Goede

drivers/input/touchscreen/chipone_icn8318.c new file +2 -0
+	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,

Shouldn't we let DT data tell us what trigger to use? I.e. just leave IRQF_ONESHOT here?

No need to resubmit, just let me know if you agree/disagree...

Thanks.

Signature · 1 line

Dmitry

Hans de Goede

author

2015-03-22 11:00 UTC #2 lore raw

PATCH v3 1/1 input: Add support for ChipOne icn8318 based touchscreens

Hi,

Quoted 8 lines · Dmitry Torokhov

Hi Hans,
drivers/input/touchscreen/chipone_icn8318.c line 272
Quoted 2 lines · Hans de Goede
drivers/input/touchscreen/chipone_icn8318.c new file +2 -0
+	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
Shouldn't we let DT data tell us what trigger to use? I.e. just leave IRQF_ONESHOT here?

That is an interesting question, that new data is available is signalled by the irq pin of the chip going low is a property of the chip, not the board layout, so I believe it is best to leave this as is.

Also note that if we want to get this from devicetree, that simply leaving out the flag is not enough, we must specifically get the data from devicetree and pass it into request_irq AFAICT. So the above would change to:

	irqflags = irqd_get_trigger_type(irq_get_irq_data(client->irq)) | IRQF_ONESHOT,
	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq, irqflags,

Regards,

Hans

Dmitry Torokhov

maintainer

2015-03-22 22:42 UTC #3 lore raw

PATCH v3 1/1 input: Add support for ChipOne icn8318 based touchscreens

Quoted 12 lines · Hans de Goede

Hi,
Quoted 8 lines · Dmitry Torokhov
Hi Hans,
drivers/input/touchscreen/chipone_icn8318.c line 272
Quoted 2 lines · Hans de Goede
drivers/input/touchscreen/chipone_icn8318.c new file +2 -0
+	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
Shouldn't we let DT data tell us what trigger to use? I.e. just leave IRQF_ONESHOT here?

Quoted 3 lines

That is an interesting question, that new data is available is signalled by the irq pin of the chip going low is a property of the chip, not the board layout, so I believe it is best to leave this as is.

My concern is that even if pin behavior is property of chip maybe on some boards we want to use level-triggered interrupts instead of edge? And if we indeed want to hard-code the trigger then shouldn't the binding document use onecell mapping (so that users do not attempt to configure triggers from DT)?

Quoted 7 lines

Also note that if we want to get this from devicetree, that simply leaving out the flag is not enough, we must specifically get the data from devicetree and pass it into request_irq AFAICT. So the above would change to:
	irqflags = irqd_get_trigger_type(irq_get_irq_data(client->irq)) | IRQF_ONESHOT,
	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq, irqflags,

No, of_irq_get() that i2c core calls before probing driver should already set the trigger type for us. There is no need for the individual drivers to do that.

Thanks.

Signature · 1 line

Dmitry

Hans de Goede

author

2015-03-24 17:45 UTC #4 lore raw

PATCH v3 1/1 input: Add support for ChipOne icn8318 based touchscreens

Hi,

Quoted 17 lines · Dmitry Torokhov

Quoted 12 lines · Hans de Goede
Hi,
Quoted 8 lines · Dmitry Torokhov
Hi Hans,
drivers/input/touchscreen/chipone_icn8318.c line 272
Quoted 2 lines · Hans de Goede
drivers/input/touchscreen/chipone_icn8318.c new file +2 -0
+	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
Shouldn't we let DT data tell us what trigger to use? I.e. just leave IRQF_ONESHOT here?
Quoted 3 lines
That is an interesting question, that new data is available is signalled by the irq pin of the chip going low is a property of the chip, not the board layout, so I believe it is best to leave this as is.

Quoted 5 lines

My concern is that even if pin behavior is property of chip maybe on some boards we want to use level-triggered interrupts instead of edge? And if we indeed want to hard-code the trigger then shouldn't the binding document use onecell mapping (so that users do not attempt to configure triggers from DT)?

The number of irq cells is specified by the interrupt controller driver rather then by the device binding.

Quoted 6 lines

Quoted 6 lines
Also note that if we want to get this from devicetree, that simply leaving out the flag is not enough, we must specifically get the data from devicetree and pass it into request_irq AFAICT. So the above would change to:

Quoted 6 lines

Quoted 2 lines
	irqflags = irqd_get_trigger_type(irq_get_irq_data(client->irq)) | IRQF_ONESHOT,
	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq, irqflags,
No, of_irq_get() that i2c core calls before probing driver should already set the trigger type for us. There is no need for the individual drivers to do that.

Ah I see, ok I've just tested removing the IRQF_TRIGGER_FALLING flag and indeed things still work fine, so feel free to merge this patch with that flagged dropped.

Regards,

Hans

Dmitry Torokhov

maintainer

2015-03-24 18:33 UTC #5 lore raw

PATCH v3 1/1 input: Add support for ChipOne icn8318 based touchscreens

Quoted 38 lines · Hans de Goede

Hi,
Quoted 17 lines · Dmitry Torokhov
Quoted 12 lines · Hans de Goede
Hi,
Quoted 8 lines · Dmitry Torokhov
Hi Hans,
drivers/input/touchscreen/chipone_icn8318.c line 272
Quoted 2 lines · Hans de Goede
drivers/input/touchscreen/chipone_icn8318.c new file +2 -0
+	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
Shouldn't we let DT data tell us what trigger to use? I.e. just leave IRQF_ONESHOT here?
Quoted 3 lines
That is an interesting question, that new data is available is signalled by the irq pin of the chip going low is a property of the chip, not the board layout, so I believe it is best to leave this as is.
Quoted 5 lines
My concern is that even if pin behavior is property of chip maybe on some boards we want to use level-triggered interrupts instead of edge? And if we indeed want to hard-code the trigger then shouldn't the binding document use onecell mapping (so that users do not attempt to configure triggers from DT)?
The number of irq cells is specified by the interrupt controller driver rather then by the device binding.
Quoted 9 lines
Quoted 8 lines
Also note that if we want to get this from devicetree, that simply leaving out the flag is not enough, we must specifically get the data from devicetree and pass it into request_irq AFAICT. So the above would change to:
	irqflags = irqd_get_trigger_type(irq_get_irq_data(client->irq)) | IRQF_ONESHOT,
	error = devm_request_threaded_irq(dev, client->irq, NULL, icn8318_irq, irqflags,

Quoted 6 lines

Quoted 3 lines
No, of_irq_get() that i2c core calls before probing driver should already set the trigger type for us. There is no need for the individual drivers to do that.
Ah I see, ok I've just tested removing the IRQF_TRIGGER_FALLING flag and indeed things still work fine, so feel free to merge this patch with that flagged dropped.

OK, thanks, queued for 4.1.

Thanks.

Signature · 1 line

Dmitry