Add device tree support to the cyttsp driver

PATCH Superseded Oreste Salerno 3 participants 11 messages Jan 10, 2016 – Jan 11, 2016

Subsystems (4)

Device tree bindings HID Input drivers Cyttsp touchscreen driver

lore

Share & tools

lore

Oreste Salerno

author

2016-01-10 17:36 UTC #0 lore raw

v3 cover Add device tree support to the cyttsp driver

This set of patches adds support to the cyttsp touchscreen driver to retrieve platform-specific details from the device tree. Additionally, they define a default init function that performs a hard reset of the chip on boot.

---
Changes in v3:
- Rework binding names and descriptions
Changes in v2:
- Fix err_cast.cocci warning in patch 2/3

Oreste Salerno (3):
Input: cyttsp - remove unused irq_gpio from platform_data
Input: cyttsp - add device tree bindings
Input: cyttsp - add default init function

 .../bindings/input/touchscreen/cyttsp.txt          |  82 ++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 146 ++++++++++++++++++++-
 include/linux/input/cyttsp.h                       |   4 +-
 3 files changed, 224 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

Signature · 6 lines

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Oreste Salerno

author

2016-01-10 17:36 UTC #1 lore raw

PATCH v3 1/3 Input: cyttsp - remove unused irq_gpio from platform_data

This field is not being used and the irq is anyway passed via the 'irq' parameter to cyttsp_probe.

Signed-off-by Oreste Salerno <[email protected]>

---
 include/linux/input/cyttsp.h | 1 -
 1 file changed, 1 deletion(-)

include/linux/input/cyttsp.h +0 -1

index 5af7c66..d7c2520 100644
@@ -51,7 +51,6 @@ struct cyttsp_platform_data {
 	int (*init)(void);
 	void (*exit)(void);
 	char *name;
-	s16 irq_gpio;
 	u8 *bl_keys;
 };

Signature · 1 line

1.9.1

Oreste Salerno

author

2016-01-10 17:36 UTC #2 lore raw

PATCH v3 2/3 Input: cyttsp - add device tree bindings

In reply to Oreste Salerno

Signed-off-by Oreste Salerno <[email protected]>

---
 .../bindings/input/touchscreen/cyttsp.txt          |  82 +++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 134 +++++++++++++++++++--
 include/linux/input/cyttsp.h                       |   3 +
 3 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +82 -0

new file mode 100644
index 0000000..2dc5c65
@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible		: must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg			: Device I2C address or SPI chip select number
+- spi-max-frequency 	: Maximum SPI clocking speed of the device (for cyttsp-spi)
+- interrupt-parent	: the phandle for the gpio controller
+			  (see interrupt binding[0]).
+- interrupts		: (gpio) interrupt to which the chip is connected
+			  (see interrupt binding[0]).
+- reset-gpios		: the reset gpio the chip is connected to
+			  (see GPIO binding[1] for more details).
+- touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
+- bootloader-key	: the 8-byte bootloader key that is required to switch
+			  the chip from bootloader mode (default mode) to
+			  application mode.
+			  This property has to be specified as an array of 8
+			  '/bits/ 8' values.
+
+Optional properties:
+- active-distance	: the distance in pixels beyond which a touch must move
+			  before movement is detected and reported by the device.
+			  Valid values: 0-15.
+- active-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in active mode.
+			  Valid values: 0-255.
+- lowpower-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in low-power mode.
+			  Valid values: 0-2550
+- touch-timeout-ms	: minimum time in ms spent in the active power state while no
+			  touches are detected before entering low-power mode.
+			  Valid values: 0-2550
+
+[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[1]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+	&i2c1 {
+		/* ... */
+		cyttsp@a {
+			compatible = "cypress,cyttsp-i2c";
+			reg = <0xa>;
+			interrupt-parent = <&msm_gpio>;
+			interrupts = <13 0x2008>;
+			reset-gpios = <&msm_gpio 12 0x00>;
+
+			touchscreen-size-x = <800>;
+			touchscreen-size-y = <480>;
+			bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
+
+			active-distance = <8>;
+			active-interval-ms = <0>;
+			lowpower-interval-ms = <200>;
+			touch-timeout-ms = <100>;
+		};
+
+		/* ... */
+	};
+
+	&mcspi1 {
+		/* ... */
+		cyttsp@0 {
+			compatible = "cypress,cyttsp-spi";
+			spi-max-frequency = <6000000>;
+			reg = <0>;
+			interrupt-parent = <&msm_gpio>;
+			interrupts = <13 0x2008>;
+			reset-gpios = <&msm_gpio 12 0x00>;
+
+			touchscreen-size-x = <800>;
+			touchscreen-size-y = <480>;
+			bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
+
+			active-distance = <8>;
+			active-interval-ms = <0>;
+			lowpower-interval-ms = <200>;
+			touch-timeout-ms = <100>;
+		};
+
+		/* ... */
+	};

drivers/input/touchscreen/cyttsp_core.c +127 -7

171 lines not on this page · raw message

include/linux/input/cyttsp.h +3 -0

index d7c2520..92a9d52 100644
@@ -29,6 +29,8 @@
 #ifndef _CYTTSP_H_
 #define _CYTTSP_H_
 
+#include <linux/gpio/consumer.h>
+
 #define CY_SPI_NAME "cyttsp-spi"
 #define CY_I2C_NAME "cyttsp-i2c"
 /* Active Power state scanning/processing refresh interval */

@@ -51,6 +53,7 @@ struct cyttsp_platform_data {
 	int (*init)(void);
 	void (*exit)(void);
 	char *name;
+	struct gpio_desc *reset_gpio;
 	u8 *bl_keys;
 };

Signature · 6 lines

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rob Herring

maintainer

2016-01-11 03:01 UTC #3 lore raw

Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings

Quoted 28 lines · Oreste Salerno

Signed-off-by Oreste Salerno <[email protected]>

---
 .../bindings/input/touchscreen/cyttsp.txt          |  82 +++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 134 +++++++++++++++++++--
 include/linux/input/cyttsp.h                       |   3 +
 3 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +14 -0

new file mode 100644
index 0000000..2dc5c65
@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible		: must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg			: Device I2C address or SPI chip select number
+- spi-max-frequency 	: Maximum SPI clocking speed of the device (for cyttsp-spi)
+- interrupt-parent	: the phandle for the gpio controller
+			  (see interrupt binding[0]).
+- interrupts		: (gpio) interrupt to which the chip is connected
+			  (see interrupt binding[0]).
+- reset-gpios		: the reset gpio the chip is connected to
+			  (see GPIO binding[1] for more details).
+- touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y	: vertical resolution of touchscreen (in pixels)

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 24

Quoted 10 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +10 -0

+- bootloader-key	: the 8-byte bootloader key that is required to switch
+			  the chip from bootloader mode (default mode) to
+			  application mode.
+			  This property has to be specified as an array of 8
+			  '/bits/ 8' values.
+
+Optional properties:
+- active-distance	: the distance in pixels beyond which a touch must move
+			  before movement is detected and reported by the device.
+			  Valid values: 0-15.

Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the replacement for the deprecated moving-threshold?

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 34

Quoted 10 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +10 -0

+- active-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in active mode.
+			  Valid values: 0-255.
+- lowpower-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in low-power mode.
+			  Valid values: 0-2550
+- touch-timeout-ms	: minimum time in ms spent in the active power state while no
+			  touches are detected before entering low-power mode.
+			  Valid values: 0-2550
+

Otherwise, looks fine.

Oreste Salerno

author

2016-01-11 17:54 UTC #4 lore raw

Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings

Quoted 34 lines · Rob Herring

Quoted 33 lines · Oreste Salerno

Signed-off-by Oreste Salerno <[email protected]>

---
 .../bindings/input/touchscreen/cyttsp.txt          |  82 +++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 134 +++++++++++++++++++--
 include/linux/input/cyttsp.h                       |   3 +
 3 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +19 -0

new file mode 100644
index 0000000..2dc5c65
@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible		: must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg			: Device I2C address or SPI chip select number
+- spi-max-frequency 	: Maximum SPI clocking speed of the device (for cyttsp-spi)
+- interrupt-parent	: the phandle for the gpio controller
+			  (see interrupt binding[0]).
+- interrupts		: (gpio) interrupt to which the chip is connected
+			  (see interrupt binding[0]).
+- reset-gpios		: the reset gpio the chip is connected to
+			  (see GPIO binding[1] for more details).
+- touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
+- bootloader-key	: the 8-byte bootloader key that is required to switch
+			  the chip from bootloader mode (default mode) to
+			  application mode.
+			  This property has to be specified as an array of 8
+			  '/bits/ 8' values.

Quoted 9 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 24
Quoted 5 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +5 -0
+
+Optional properties:
+- active-distance	: the distance in pixels beyond which a touch must move
+			  before movement is detected and reported by the device.
+			  Valid values: 0-15.
Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the replacement for the deprecated moving-threshold?

It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties are meant to be used as parameters passed to input_set_abs_params(), which are used for a SW-based jitter filtering.
The active-distance value instead is used to set a HW register in the chip which effectively performs the filtering in the HW.
The driver is currently passing 0 as fuzz parameter to input_set_abs_params().

Quoted 4 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +4 -0

+- active-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in active mode.
+			  Valid values: 0-255.
+- lowpower-interval-ms	: the minimum period in ms between consecutive

Quoted 8 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 34
Quoted 6 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +6 -0
+			  scanning/processing cycles when the chip is in low-power mode.
+			  Valid values: 0-2550
+- touch-timeout-ms	: minimum time in ms spent in the active power state while no
+			  touches are detected before entering low-power mode.
+			  Valid values: 0-2550
+
Otherwise, looks fine.

I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see that he hasn't been actively reviewing patches for the cyttsp* drivers for a while. I wonder what would be the process in this case?

Dmitry Torokhov

maintainer

2016-01-11 18:45 UTC #5 lore raw

Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings

Quoted 45 lines · Oreste Salerno

Quoted 34 lines · Rob Herring

Quoted 33 lines · Oreste Salerno

Signed-off-by Oreste Salerno <[email protected]>

---
 .../bindings/input/touchscreen/cyttsp.txt          |  82 +++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 134 +++++++++++++++++++--
 include/linux/input/cyttsp.h                       |   3 +
 3 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +19 -0

new file mode 100644
index 0000000..2dc5c65
@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible		: must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg			: Device I2C address or SPI chip select number
+- spi-max-frequency 	: Maximum SPI clocking speed of the device (for cyttsp-spi)
+- interrupt-parent	: the phandle for the gpio controller
+			  (see interrupt binding[0]).
+- interrupts		: (gpio) interrupt to which the chip is connected
+			  (see interrupt binding[0]).
+- reset-gpios		: the reset gpio the chip is connected to
+			  (see GPIO binding[1] for more details).
+- touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
+- bootloader-key	: the 8-byte bootloader key that is required to switch
+			  the chip from bootloader mode (default mode) to
+			  application mode.
+			  This property has to be specified as an array of 8
+			  '/bits/ 8' values.

Quoted 9 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 24
Quoted 5 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +5 -0
+
+Optional properties:
+- active-distance	: the distance in pixels beyond which a touch must move
+			  before movement is detected and reported by the device.
+			  Valid values: 0-15.
Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the replacement for the deprecated moving-threshold?

Quoted 6 lines

It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties are meant to be used as parameters passed to input_set_abs_params(), which are used for a SW-based jitter filtering.
The active-distance value instead is used to set a HW register in the chip which effectively performs the filtering in the HW.
The driver is currently passing 0 as fuzz parameter to input_set_abs_params().

It doe snot have to and it will not if you use touchscreen_parse_properties(). But you are right, fuzz is reserved for software de-jittering, and it can be changed at any time from userspace at input device level, but will not be communicated to the hardware. I think hardware-based filtering is hardware-specific and I think having a separate property is fine for it. We can still standardize on name, units, etc though.

Quoted 14 lines

Quoted 13 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 34

Quoted 11 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +10 -0

+- active-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in active mode.
+			  Valid values: 0-255.
+- lowpower-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in low-power mode.
+			  Valid values: 0-2550
+- touch-timeout-ms	: minimum time in ms spent in the active power state while no
+			  touches are detected before entering low-power mode.
+			  Valid values: 0-2550
+

Otherwise, looks fine.

Quoted 3 lines

I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see that he hasn't been actively reviewing patches for the cyttsp* drivers for a while. I wonder what would be the process in this case?

Please address the items from my other email and I should be able to pick it up.

Thanks.

Signature · 1 line

Dmitry

Oreste Salerno

author

2016-01-11 19:01 UTC #6 lore raw

Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings

Quoted 53 lines · Dmitry Torokhov

Quoted 45 lines · Oreste Salerno
Quoted 34 lines · Rob Herring
Quoted 33 lines · Oreste Salerno
Signed-off-by Oreste Salerno <[email protected]>
---
 .../bindings/input/touchscreen/cyttsp.txt          |  82 +++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 134 +++++++++++++++++++--
 include/linux/input/cyttsp.h                       |   3 +
 3 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +19 -0
new file mode 100644
index 0000000..2dc5c65
@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible		: must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg			: Device I2C address or SPI chip select number
+- spi-max-frequency 	: Maximum SPI clocking speed of the device (for cyttsp-spi)
+- interrupt-parent	: the phandle for the gpio controller
+			  (see interrupt binding[0]).
+- interrupts		: (gpio) interrupt to which the chip is connected
+			  (see interrupt binding[0]).
+- reset-gpios		: the reset gpio the chip is connected to
+			  (see GPIO binding[1] for more details).
+- touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
+- bootloader-key	: the 8-byte bootloader key that is required to switch
+			  the chip from bootloader mode (default mode) to
+			  application mode.
+			  This property has to be specified as an array of 8
+			  '/bits/ 8' values.
Quoted 9 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 24
Quoted 5 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +5 -0
+
+Optional properties:
+- active-distance	: the distance in pixels beyond which a touch must move
+			  before movement is detected and reported by the device.
+			  Valid values: 0-15.
Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the replacement for the deprecated moving-threshold?
Quoted 6 lines
It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties are meant to be used as parameters passed to input_set_abs_params(), which are used for a SW-based jitter filtering.
The active-distance value instead is used to set a HW register in the chip which effectively performs the filtering in the HW.
The driver is currently passing 0 as fuzz parameter to input_set_abs_params().

Quoted 8 lines

It doe snot have to and it will not if you use touchscreen_parse_properties(). But you are right, fuzz is reserved for software de-jittering, and it can be changed at any time from userspace at input device level, but will not be communicated to the hardware. I think hardware-based filtering is hardware-specific and I think having a separate property is fine for it. We can still standardize on name, units, etc though.

Yes, I looked at touchscreen_parse_properties and I was planning to use it as part of a future improvement patch. But it makes sense to introduce it in this patch already.

Quoted 14 lines

Quoted 13 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 34

Quoted 11 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +10 -0

+- active-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in active mode.
+			  Valid values: 0-255.
+- lowpower-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in low-power mode.
+			  Valid values: 0-2550
+- touch-timeout-ms	: minimum time in ms spent in the active power state while no
+			  touches are detected before entering low-power mode.
+			  Valid values: 0-2550
+

Otherwise, looks fine.

Quoted 7 lines

Quoted 3 lines
I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see that he hasn't been actively reviewing patches for the cyttsp* drivers for a while. I wonder what would be the process in this case?
Please address the items from my other email and I should be able to pick it up.

OK, I'll try to rework the patch soon and post a new version. Thanks a lot for your review!

Quoted 4 lines

Thanks.
Signature · 1 line
Dmitry

Rob Herring

maintainer

2016-01-11 23:51 UTC #7 lore raw

Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings

In reply to Oreste Salerno

Quoted 70 lines · Oreste Salerno

Quoted 34 lines · Rob Herring
Quoted 33 lines · Oreste Salerno
Signed-off-by Oreste Salerno <[email protected]>
---
 .../bindings/input/touchscreen/cyttsp.txt          |  82 +++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 134 +++++++++++++++++++--
 include/linux/input/cyttsp.h                       |   3 +
 3 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +19 -0
new file mode 100644
index 0000000..2dc5c65
@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible               : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg                      : Device I2C address or SPI chip select number
+- spi-max-frequency        : Maximum SPI clocking speed of the device (for cyttsp-spi)
+- interrupt-parent : the phandle for the gpio controller
+                     (see interrupt binding[0]).
+- interrupts               : (gpio) interrupt to which the chip is connected
+                     (see interrupt binding[0]).
+- reset-gpios              : the reset gpio the chip is connected to
+                     (see GPIO binding[1] for more details).
+- touchscreen-size-x       : horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y       : vertical resolution of touchscreen (in pixels)
+- bootloader-key   : the 8-byte bootloader key that is required to switch
+                     the chip from bootloader mode (default mode) to
+                     application mode.
+                     This property has to be specified as an array of 8
+                     '/bits/ 8' values.
Quoted 9 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 24
Quoted 5 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +5 -0
+
+Optional properties:
+- active-distance  : the distance in pixels beyond which a touch must move
+                     before movement is detected and reported by the device.
+                     Valid values: 0-15.
Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the replacement for the deprecated moving-threshold?
It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties are meant to be used as parameters passed to input_set_abs_params(), which are used for a SW-based jitter filtering.
The active-distance value instead is used to set a HW register in the chip which effectively performs the filtering in the HW.
The driver is currently passing 0 as fuzz parameter to input_set_abs_params().
Quoted 11 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 34
Quoted 10 lines
Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +10 -0
+- active-interval-ms       : the minimum period in ms between consecutive
+                     scanning/processing cycles when the chip is in active mode.
+                     Valid values: 0-255.
+- lowpower-interval-ms     : the minimum period in ms between consecutive
+                     scanning/processing cycles when the chip is in low-power mode.
+                     Valid values: 0-2550
+- touch-timeout-ms : minimum time in ms spent in the active power state while no
+                     touches are detected before entering low-power mode.
+                     Valid values: 0-2550
+
Quoted 1 line
Otherwise, looks fine.
I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see that he hasn't been actively reviewing patches for the cyttsp* drivers for a while. I wonder what would be the process in this case?

Dmitry Torokhov

maintainer

2016-01-11 18:30 UTC #8 lore raw

Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings

In reply to Oreste Salerno

Hi Oreste,

Quoted 14 lines · Oreste Salerno

Signed-off-by Oreste Salerno <[email protected]>

---
 .../bindings/input/touchscreen/cyttsp.txt          |  82 +++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 134 +++++++++++++++++++--
 include/linux/input/cyttsp.h                       |   3 +
 3 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

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

new file mode 100644
index 0000000..2dc5c65

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 6

Quoted 6 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +6 -0

@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible		: must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg			: Device I2C address or SPI chip select number
+- spi-max-frequency 	: Maximum SPI clocking speed of the device (for cyttsp-spi)

Stray space before tab after spi-max-frequency.

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt line 14

Quoted 8 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +8 -0

+- interrupt-parent	: the phandle for the gpio controller
+			  (see interrupt binding[0]).
+- interrupts		: (gpio) interrupt to which the chip is connected
+			  (see interrupt binding[0]).
+- reset-gpios		: the reset gpio the chip is connected to
+			  (see GPIO binding[1] for more details).
+- touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y	: vertical resolution of touchscreen (in pixels)

Let's direct users to common touchscreen properties.

Quoted 122 lines

Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt new file +68 -0

+- bootloader-key	: the 8-byte bootloader key that is required to switch
+			  the chip from bootloader mode (default mode) to
+			  application mode.
+			  This property has to be specified as an array of 8
+			  '/bits/ 8' values.
+
+Optional properties:
+- active-distance	: the distance in pixels beyond which a touch must move
+			  before movement is detected and reported by the device.
+			  Valid values: 0-15.
+- active-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in active mode.
+			  Valid values: 0-255.
+- lowpower-interval-ms	: the minimum period in ms between consecutive
+			  scanning/processing cycles when the chip is in low-power mode.
+			  Valid values: 0-2550
+- touch-timeout-ms	: minimum time in ms spent in the active power state while no
+			  touches are detected before entering low-power mode.
+			  Valid values: 0-2550
+
+[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[1]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+	&i2c1 {
+		/* ... */
+		cyttsp@a {
+			compatible = "cypress,cyttsp-i2c";
+			reg = <0xa>;
+			interrupt-parent = <&msm_gpio>;
+			interrupts = <13 0x2008>;
+			reset-gpios = <&msm_gpio 12 0x00>;
+
+			touchscreen-size-x = <800>;
+			touchscreen-size-y = <480>;
+			bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
+
+			active-distance = <8>;
+			active-interval-ms = <0>;
+			lowpower-interval-ms = <200>;
+			touch-timeout-ms = <100>;
+		};
+
+		/* ... */
+	};
+
+	&mcspi1 {
+		/* ... */
+		cyttsp@0 {
+			compatible = "cypress,cyttsp-spi";
+			spi-max-frequency = <6000000>;
+			reg = <0>;
+			interrupt-parent = <&msm_gpio>;
+			interrupts = <13 0x2008>;
+			reset-gpios = <&msm_gpio 12 0x00>;
+
+			touchscreen-size-x = <800>;
+			touchscreen-size-y = <480>;
+			bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
+
+			active-distance = <8>;
+			active-interval-ms = <0>;
+			lowpower-interval-ms = <200>;
+			touch-timeout-ms = <100>;
+		};
+
+		/* ... */
+	};

drivers/input/touchscreen/cyttsp_core.c +32 -0

index 5b74e8b..5dc6bf6 100644
@@ -33,6 +33,8 @@
 #include <linux/gpio.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/gpio/consumer.h>
 
 #include "cyttsp_core.h"

@@ -57,6 +59,7 @@
 #define CY_DELAY_DFLT			20 /* ms */
 #define CY_DELAY_MAX			500
 #define CY_ACT_DIST_DFLT		0xF8
+#define CY_ACT_DIST_MASK		0x0F
 #define CY_HNDSHK_BIT			0x80
 /* device mode bits */
 #define CY_OPERATE_MODE			0x00

@@ -528,18 +531,136 @@ static void cyttsp_close(struct input_dev *dev)
 		cyttsp_disable(ts);
 }
 
+#ifdef CONFIG_OF
+static const struct cyttsp_platform_data *cyttsp_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct cyttsp_platform_data *pdata;
+	u32 dt_value;
+	int ret;
+	static const char err_msg[] =
+		"property not provided in the device tree";
+
+	if (!np)
+		return ERR_PTR(-ENOENT);
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
+	if (!pdata->bl_keys)
+		return ERR_PTR(-ENOMEM);
+
+	/* Set some default values */
+	pdata->act_dist = CY_ACT_DIST_DFLT;
+	pdata->act_intrvl = CY_ACT_INTRVL_DFLT;
+	pdata->tch_tmout = CY_TCH_TMOUT_DFLT;
+	pdata->lp_intrvl = CY_LP_INTRVL_DFLT;
+	pdata->name = "cyttsp";
+
+	ret = of_property_read_u32(np, "touchscreen-size-x", &pdata->maxx);

drivers/input/touchscreen/cyttsp_core.c static void cyttsp_close(struct input_dev *dev) · line 572

Quoted 10 lines

drivers/input/touchscreen/cyttsp_core.c +10 -0

+	if (ret) {
+		dev_err(dev, "touchscreen-size-x %s\n", err_msg);
+		return ERR_PTR(ret);
+	}
+
+	ret = of_property_read_u32(np, "touchscreen-size-y", &pdata->maxy);
+	if (ret) {
+		dev_err(dev, "touchscreen-size-y %s\n", err_msg);
+		return ERR_PTR(ret);
+	}

I'd rather we called touchscreen_parse_properties() in common code when setting up the input device. It should work for variety of platform providers (OF, ACPI, etc).

drivers/input/touchscreen/cyttsp_core.c static void cyttsp_close(struct input_dev *dev) · line 579

Quoted 7 lines

drivers/input/touchscreen/cyttsp_core.c +7 -0

+
+	pdata->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(pdata->reset_gpio)) {
+		ret = PTR_ERR(pdata->reset_gpio);
+		dev_err(dev, "error acquiring reset gpio: %d\n", ret);
+		return ERR_PTR(ret);
+	}

Please call it directly in probe(). Again, it should work fine not only on OF, but on all other platforms, so

Quoted 6 lines

drivers/input/touchscreen/cyttsp_core.c +6 -0

+
+	ret = of_property_read_u8_array(np, "bootloader-key",
+			pdata->bl_keys, CY_NUM_BL_KEYS);
+	if (ret) {
+		dev_err(dev, "bootloader-key %s\n", err_msg);
+		return ERR_PTR(ret);

drivers/input/touchscreen/cyttsp_core.c static void cyttsp_close(struct input_dev *dev) · line 595

Quoted 10 lines

drivers/input/touchscreen/cyttsp_core.c +10 -0

+	}
+
+	if (!of_property_read_u32(np, "active-distance", &dt_value)) {
+		if (dt_value > 15) {
+			dev_err(dev, "active-distance (%u) must be [0-15]\n",
+				dt_value);
+			return ERR_PTR(-EINVAL);
+		}
+		pdata->act_dist &= ~CY_ACT_DIST_MASK;
+		pdata->act_dist |= (u8)dt_value;

I do not think you need to cast.

drivers/input/touchscreen/cyttsp_core.c static void cyttsp_close(struct input_dev *dev) · line 604

Quoted 9 lines

drivers/input/touchscreen/cyttsp_core.c +9 -0

+	}
+
+	if (!of_property_read_u32(np, "active-interval-ms", &dt_value)) {
+		if (dt_value > 255) {
+			dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
+				dt_value);
+			return ERR_PTR(-EINVAL);
+		}
+		pdata->act_intrvl = (u8)dt_value;

I do not think you need to cast.

drivers/input/touchscreen/cyttsp_core.c static void cyttsp_close(struct input_dev *dev) · line 614

Quoted 10 lines

drivers/input/touchscreen/cyttsp_core.c +10 -0

+	}
+
+	if (!of_property_read_u32(np, "lowpower-interval-ms", &dt_value)) {
+		if (dt_value > 2550) {
+			dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
+				dt_value);
+			return ERR_PTR(-EINVAL);
+		}
+		/* Register value is expressed in 0.01s / bit */
+		pdata->lp_intrvl = (u8)(dt_value/10);

Spaces around operations please. Do we need explicit cast here?

drivers/input/touchscreen/cyttsp_core.c static void cyttsp_close(struct input_dev *dev) · line 624

Quoted 10 lines

drivers/input/touchscreen/cyttsp_core.c +10 -0

+	}
+
+	if (!of_property_read_u32(np, "touch-timeout-ms", &dt_value)) {
+		if (dt_value > 2550) {
+			dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
+				dt_value);
+			return ERR_PTR(-EINVAL);
+		}
+		/* Register value is expressed in 0.01s / bit */
+		pdata->tch_tmout = (u8)(dt_value/10);

Spaces around operations please. Do we need explicit cast here?

Quoted 78 lines

drivers/input/touchscreen/cyttsp_core.c +33 -7

+	}
+
+	return pdata;
+}
+#else
+static const struct cyttsp_platform_data *cyttsp_parse_dt(struct device *dev)
+{
+	return ERR_PTR(-ENOENT);
+}
+#endif
+
+static const struct cyttsp_platform_data *
+cyttsp_get_platform_data(struct device *dev)
+{
+	const struct cyttsp_platform_data *pdata;
+
+	pdata = dev_get_platdata(dev);
+	if (pdata)
+		return pdata;
+
+	pdata = cyttsp_parse_dt(dev);
+	if (!IS_ERR(pdata) || PTR_ERR(pdata) != -ENOENT)
+		return pdata;
+
+	dev_err(dev, "No platform data specified\n");
+	return ERR_PTR(-EINVAL);
+}
+
 struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 			    struct device *dev, int irq, size_t xfer_buf_size)
 {
-	const struct cyttsp_platform_data *pdata = dev_get_platdata(dev);
+	const struct cyttsp_platform_data *pdata;
 	struct cyttsp *ts;
 	struct input_dev *input_dev;
 	int error;
 
-	if (!pdata || !pdata->name || irq <= 0) {
-		error = -EINVAL;
-		goto err_out;
-	}
+	pdata = cyttsp_get_platform_data(dev);
+	if (IS_ERR(pdata))
+		return ERR_CAST(pdata);
 
 	ts = kzalloc(sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
 	input_dev = input_allocate_device();

@@ -550,7 +671,7 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 
 	ts->dev = dev;
 	ts->input = input_dev;
-	ts->pdata = dev_get_platdata(dev);
+	ts->pdata = pdata;
 	ts->bus_ops = bus_ops;
 	ts->irq = irq;

@@ -618,7 +739,6 @@ err_platform_exit:
 err_free_mem:
 	input_free_device(input_dev);
 	kfree(ts);
-err_out:
 	return ERR_PTR(error);
 }
 EXPORT_SYMBOL_GPL(cyttsp_probe);

include/linux/input/cyttsp.h +2 -0

index d7c2520..92a9d52 100644
@@ -29,6 +29,8 @@
 #ifndef _CYTTSP_H_
 #define _CYTTSP_H_
 
+#include <linux/gpio/consumer.h>
+
 #define CY_SPI_NAME "cyttsp-spi"
 #define CY_I2C_NAME "cyttsp-i2c"
 /* Active Power state scanning/processing refresh interval */

include/linux/input/cyttsp.h struct cyttsp_platform_data { · line 56

Quoted 4 lines

include/linux/input/cyttsp.h +1 -0

@@ -51,6 +53,7 @@ struct cyttsp_platform_data {
 	int (*init)(void);
 	void (*exit)(void);
 	char *name;
+	struct gpio_desc *reset_gpio;

No, we shoudl not be passing gpio descriptor in platform data. For non-of non-ACPI system gpiod_* framework allows boards to specify lookup tables so they can use that. Just drop it form here.

Quoted 6 lines

include/linux/input/cyttsp.h +0 -0
 	u8 *bl_keys;
 };
 
Signature · 1 line
1.9.1

Thanks.

Signature · 5 lines

Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Oreste Salerno

author

2016-01-10 17:36 UTC #9 lore raw

PATCH v3 3/3 Input: cyttsp - add default init function

In reply to Oreste Salerno

When using device trees, it's not possible to assign an init() function in the platform data.
In case the init function has not been assigned, attempt to perform a hard reset of the chip, using reset gpio defined in the platform data / device tree.

Signed-off-by Oreste Salerno <[email protected]>

---
 drivers/input/touchscreen/cyttsp_core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/input/touchscreen/cyttsp_core.c +12 -0

index 5dc6bf6..baf0fd5 100644
@@ -239,6 +239,16 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
 	return retval;
 }
 
+static void cyttsp_hard_reset(struct cyttsp *ts)
+{
+	if (ts->pdata->reset_gpio) {
+		gpiod_set_value_cansleep(ts->pdata->reset_gpio, 1);
+		msleep(CY_DELAY_DFLT);
+		gpiod_set_value_cansleep(ts->pdata->reset_gpio, 0);
+		msleep(CY_DELAY_DFLT);
+	}
+}
+
 static int cyttsp_soft_reset(struct cyttsp *ts)
 {
 	unsigned long timeout;

@@ -685,6 +695,8 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 				error);
 			goto err_free_mem;
 		}
+	} else {
+		cyttsp_hard_reset(ts);
 	}
 
 	input_dev->name = pdata->name;

Signature · 1 line

1.9.1

Dmitry Torokhov

maintainer

2016-01-11 20:02 UTC #10 lore raw

Re: [PATCH v3 3/3] Input: cyttsp - add default init function

Quoted 5 lines · Oreste Salerno

When using device trees, it's not possible to assign an init() function in the platform data.
In case the init function has not been assigned, attempt to perform a hard reset of the chip, using reset gpio defined in the platform data / device tree.

Hmm, I do not see any users of cyttsp platform data in mainline, so let's kill init/exit and just do GPIO toggle for everyone. We might need to add regulator support as well, but that's when someone will need it.

Thanks.

Quoted 39 lines

Signed-off-by Oreste Salerno <[email protected]>

---
 drivers/input/touchscreen/cyttsp_core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/input/touchscreen/cyttsp_core.c +12 -0

index 5dc6bf6..baf0fd5 100644
@@ -239,6 +239,16 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
 	return retval;
 }
 
+static void cyttsp_hard_reset(struct cyttsp *ts)
+{
+	if (ts->pdata->reset_gpio) {
+		gpiod_set_value_cansleep(ts->pdata->reset_gpio, 1);
+		msleep(CY_DELAY_DFLT);
+		gpiod_set_value_cansleep(ts->pdata->reset_gpio, 0);
+		msleep(CY_DELAY_DFLT);
+	}
+}
+
 static int cyttsp_soft_reset(struct cyttsp *ts)
 {
 	unsigned long timeout;

@@ -685,6 +695,8 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 				error);
 			goto err_free_mem;
 		}
+	} else {
+		cyttsp_hard_reset(ts);
 	}
 
 	input_dev->name = pdata->name;

Signature · 1 line

1.9.1

Signature · 5 lines

Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html