linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] Add device tree support to the cyttsp driver
@ 2016-01-21 19:21 Oreste Salerno
  2016-01-21 19:21 ` [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations Oreste Salerno
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 19:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-input, fery, dmitry.torokhov, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, oreste.salerno

Implement a number of updates for the cyttsp driver to be able to parse generic
ACPI/of/static properties and drop support for platform data (no platform data
user is present in the kernel tree).

---
Changes in v4:
  Major rework based on Dmitry's review:
  - Use device_property_read() functions instead of of_property()
  - Use touchscreen_parse_properties to parse common properties
  - Drop support for platform data
  - Used devres managed allocations
Changes in v3:
  - Rework binding names and descriptions
Changes in v2:
  - Fix err_cast.cocci warning in patch 2/3


Oreste Salerno (4):
  Input: cyttsp - use devres managed resource allocations
  Input: cyttsp - check return value of input_mt_init_slots
  Input: cyttsp - switch to using device properties
  Input: cyttsp - perform hard reset of the chip during probe

 .../bindings/input/touchscreen/cyttsp.txt          |  95 ++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 197 +++++++++++++--------
 drivers/input/touchscreen/cyttsp_core.h            |  10 +-
 drivers/input/touchscreen/cyttsp_i2c.c             |  10 --
 drivers/input/touchscreen/cyttsp_spi.c             |  10 --
 include/linux/input/cyttsp.h                       |  15 --
 6 files changed, 231 insertions(+), 106 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations
  2016-01-21 19:21 [PATCH v4 0/4] Add device tree support to the cyttsp driver Oreste Salerno
@ 2016-01-21 19:21 ` Oreste Salerno
       [not found]   ` <20e9f0188f07fc420912b3defa747303be3cf290.1453403916.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
  2016-01-21 19:21 ` [PATCH v4 2/4] Input: cyttsp - check return value of input_mt_init_slots Oreste Salerno
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 19:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-input, fery, dmitry.torokhov, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, oreste.salerno

Use devm_() functions for allocating memory, input device and IRQ.

Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
---
 drivers/input/touchscreen/cyttsp_core.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index 5b74e8b..3c2ee84 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -541,11 +541,11 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 		goto err_out;
 	}
 
-	ts = kzalloc(sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
-	input_dev = input_allocate_device();
+	ts = devm_kzalloc(dev, sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
+	input_dev = devm_input_allocate_device(dev);
 	if (!ts || !input_dev) {
 		error = -ENOMEM;
-		goto err_free_mem;
+		goto err_out;
 	}
 
 	ts->dev = dev;
@@ -562,7 +562,7 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 		if (error) {
 			dev_err(ts->dev, "platform init failed, err: %d\n",
 				error);
-			goto err_free_mem;
+			goto err_out;
 		}
 	}
 
@@ -586,9 +586,9 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 
 	input_mt_init_slots(input_dev, CY_MAX_ID, 0);
 
-	error = request_threaded_irq(ts->irq, NULL, cyttsp_irq,
-				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-				     pdata->name, ts);
+	error = devm_request_threaded_irq(dev, ts->irq, NULL, cyttsp_irq,
+					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					  pdata->name, ts);
 	if (error) {
 		dev_err(ts->dev, "failed to request IRQ %d, err: %d\n",
 			ts->irq, error);
@@ -599,25 +599,20 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 
 	error = cyttsp_power_on(ts);
 	if (error)
-		goto err_free_irq;
+		goto err_platform_exit;
 
 	error = input_register_device(input_dev);
 	if (error) {
 		dev_err(ts->dev, "failed to register input device: %d\n",
 			error);
-		goto err_free_irq;
+		goto err_platform_exit;
 	}
 
 	return ts;
 
-err_free_irq:
-	free_irq(ts->irq, ts);
 err_platform_exit:
 	if (pdata->exit)
 		pdata->exit();
-err_free_mem:
-	input_free_device(input_dev);
-	kfree(ts);
 err_out:
 	return ERR_PTR(error);
 }
@@ -625,11 +620,8 @@ EXPORT_SYMBOL_GPL(cyttsp_probe);
 
 void cyttsp_remove(struct cyttsp *ts)
 {
-	free_irq(ts->irq, ts);
-	input_unregister_device(ts->input);
 	if (ts->pdata->exit)
 		ts->pdata->exit();
-	kfree(ts);
 }
 EXPORT_SYMBOL_GPL(cyttsp_remove);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 2/4] Input: cyttsp - check return value of input_mt_init_slots
  2016-01-21 19:21 [PATCH v4 0/4] Add device tree support to the cyttsp driver Oreste Salerno
  2016-01-21 19:21 ` [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations Oreste Salerno
@ 2016-01-21 19:21 ` Oreste Salerno
  2016-01-21 19:21 ` [PATCH v4 3/4] Input: cyttsp - switch to using device properties Oreste Salerno
  2016-01-21 19:21 ` [PATCH v4 4/4] Input: cyttsp - perform hard reset of the chip during probe Oreste Salerno
  3 siblings, 0 replies; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 19:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-input, fery, dmitry.torokhov, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, oreste.salerno

Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
---
 drivers/input/touchscreen/cyttsp_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index 3c2ee84..10379bc 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -584,7 +584,11 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
 			     0, CY_MAXZ, 0, 0);
 
-	input_mt_init_slots(input_dev, CY_MAX_ID, 0);
+	error = input_mt_init_slots(input_dev, CY_MAX_ID, 0);
+	if (error) {
+		dev_err(dev, "Unable to init MT slots.\n");
+		goto err_platform_exit;
+	}
 
 	error = devm_request_threaded_irq(dev, ts->irq, NULL, cyttsp_irq,
 					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 3/4] Input: cyttsp - switch to using device properties
  2016-01-21 19:21 [PATCH v4 0/4] Add device tree support to the cyttsp driver Oreste Salerno
  2016-01-21 19:21 ` [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations Oreste Salerno
  2016-01-21 19:21 ` [PATCH v4 2/4] Input: cyttsp - check return value of input_mt_init_slots Oreste Salerno
@ 2016-01-21 19:21 ` Oreste Salerno
       [not found]   ` <03a328d762d8ed654095d9a6edf648bc1c7fa262.1453403916.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
  2016-01-21 19:21 ` [PATCH v4 4/4] Input: cyttsp - perform hard reset of the chip during probe Oreste Salerno
  3 siblings, 1 reply; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 19:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-input, fery, dmitry.torokhov, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, oreste.salerno

Drop support for platform data passed via a C-structure and switch to
device properties instead, which should make the driver compatible
with all platforms: OF, ACPI and static boards. Static boards should
use property sets to communicate device parameters to the driver.

Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
---
 .../bindings/input/touchscreen/cyttsp.txt          |  95 ++++++++++++
 drivers/input/touchscreen/cyttsp_core.c            | 167 ++++++++++++++-------
 drivers/input/touchscreen/cyttsp_core.h            |  10 +-
 drivers/input/touchscreen/cyttsp_i2c.c             |  10 --
 drivers/input/touchscreen/cyttsp_spi.c             |  10 --
 include/linux/input/cyttsp.h                       |  15 --
 6 files changed, 213 insertions(+), 94 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
new file mode 100644
index 0000000..b0fccae
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
@@ -0,0 +1,95 @@
+* 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:
+ - 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)
+ - 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
+ - use-handshake	: enable register-based handshake (boolean). This should
+			  only be used if the chip is configured to use 'blocking
+			  communication with timeout' (in this case the device
+			  generates an interrupt at the end of every
+			  scanning/processing cycle).
+
+[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 = <&gpio0>;
+			interrupts = <28 0>;
+			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
+
+			touchscreen-size-x = <800>;
+			touchscreen-size-y = <480>;
+			touchscreen-fuzz-x = <4>;
+			touchscreen-fuzz-y = <7>;
+
+			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 = <&gpio0>;
+			interrupts = <28 0>;
+			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
+
+			touchscreen-size-x = <800>;
+			touchscreen-size-y = <480>;
+			touchscreen-fuzz-x = <4>;
+			touchscreen-fuzz-y = <7>;
+
+			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>;
+		};
+
+		/* ... */
+	};
diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index 10379bc..92b459d 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -30,9 +30,12 @@
 #include <linux/delay.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
 #include <linux/gpio.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/property.h>
+#include <linux/gpio/consumer.h>
 
 #include "cyttsp_core.h"
 
@@ -57,6 +60,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
@@ -120,7 +124,7 @@ static int ttsp_send_command(struct cyttsp *ts, u8 cmd)
 
 static int cyttsp_handshake(struct cyttsp *ts)
 {
-	if (ts->pdata->use_hndshk)
+	if (ts->use_hndshk)
 		return ttsp_send_command(ts,
 				ts->xy_data.hst_mode ^ CY_HNDSHK_BIT);
 
@@ -142,9 +146,9 @@ static int cyttsp_exit_bl_mode(struct cyttsp *ts)
 	u8 bl_cmd[sizeof(bl_command)];
 
 	memcpy(bl_cmd, bl_command, sizeof(bl_command));
-	if (ts->pdata->bl_keys)
+	if (ts->bl_keys)
 		memcpy(&bl_cmd[sizeof(bl_command) - CY_NUM_BL_KEYS],
-			ts->pdata->bl_keys, CY_NUM_BL_KEYS);
+			ts->bl_keys, CY_NUM_BL_KEYS);
 
 	error = ttsp_write_block_data(ts, CY_REG_BASE,
 				      sizeof(bl_cmd), bl_cmd);
@@ -217,14 +221,14 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
 {
 	int retval = 0;
 
-	if (ts->pdata->act_intrvl != CY_ACT_INTRVL_DFLT ||
-	    ts->pdata->tch_tmout != CY_TCH_TMOUT_DFLT ||
-	    ts->pdata->lp_intrvl != CY_LP_INTRVL_DFLT) {
+	if (ts->act_intrvl != CY_ACT_INTRVL_DFLT ||
+	    ts->tch_tmout != CY_TCH_TMOUT_DFLT ||
+	    ts->lp_intrvl != CY_LP_INTRVL_DFLT) {
 
 		u8 intrvl_ray[] = {
-			ts->pdata->act_intrvl,
-			ts->pdata->tch_tmout,
-			ts->pdata->lp_intrvl
+			ts->act_intrvl,
+			ts->tch_tmout,
+			ts->lp_intrvl
 		};
 
 		/* set intrvl registers */
@@ -263,7 +267,7 @@ out:
 
 static int cyttsp_act_dist_setup(struct cyttsp *ts)
 {
-	u8 act_dist_setup = ts->pdata->act_dist;
+	u8 act_dist_setup = ts->act_dist;
 
 	/* Init gesture; active distance setup */
 	return ttsp_write_block_data(ts, CY_REG_ACT_DIST,
@@ -528,45 +532,107 @@ static void cyttsp_close(struct input_dev *dev)
 		cyttsp_disable(ts);
 }
 
+static int cyttsp_parse_properties(struct cyttsp *ts)
+{
+	struct device *dev = ts->dev;
+	u32 dt_value;
+	int ret;
+
+	ts->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
+	if (!ts->bl_keys)
+		return -ENOMEM;
+
+	/* Set some default values */
+	ts->use_hndshk = false;
+	ts->act_dist = CY_ACT_DIST_DFLT;
+	ts->act_intrvl = CY_ACT_INTRVL_DFLT;
+	ts->tch_tmout = CY_TCH_TMOUT_DFLT;
+	ts->lp_intrvl = CY_LP_INTRVL_DFLT;
+
+	ret = device_property_read_u8_array(dev, "bootloader-key",
+					    ts->bl_keys, CY_NUM_BL_KEYS);
+	if (ret) {
+		dev_err(dev,
+			"bootloader-key property could not be retrieved\n");
+		return ret;
+	}
+
+	ts->use_hndshk = device_property_present(dev, "use-handshake");
+
+	if (!device_property_read_u32(dev, "active-distance", &dt_value)) {
+		if (dt_value > 15) {
+			dev_err(dev, "active-distance (%u) must be [0-15]\n",
+				dt_value);
+			return -EINVAL;
+		}
+		ts->act_dist &= ~CY_ACT_DIST_MASK;
+		ts->act_dist |= dt_value;
+	}
+
+	if (!device_property_read_u32(dev, "active-interval-ms", &dt_value)) {
+		if (dt_value > 255) {
+			dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
+				dt_value);
+			return -EINVAL;
+		}
+		ts->act_intrvl = dt_value;
+	}
+
+	if (!device_property_read_u32(dev, "lowpower-interval-ms", &dt_value)) {
+		if (dt_value > 2550) {
+			dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
+				dt_value);
+			return -EINVAL;
+		}
+		/* Register value is expressed in 0.01s / bit */
+		ts->lp_intrvl = dt_value / 10;
+	}
+
+	if (!device_property_read_u32(dev, "touch-timeout-ms", &dt_value)) {
+		if (dt_value > 2550) {
+			dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
+				dt_value);
+			return -EINVAL;
+		}
+		/* Register value is expressed in 0.01s / bit */
+		ts->tch_tmout = dt_value/10;
+	}
+
+	return 0;
+}
+
 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);
 	struct cyttsp *ts;
 	struct input_dev *input_dev;
 	int error;
 
-	if (!pdata || !pdata->name || irq <= 0) {
-		error = -EINVAL;
-		goto err_out;
-	}
-
 	ts = devm_kzalloc(dev, sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
 	input_dev = devm_input_allocate_device(dev);
-	if (!ts || !input_dev) {
-		error = -ENOMEM;
-		goto err_out;
-	}
+	if (!ts || !input_dev)
+		return ERR_PTR(-ENOMEM);
 
 	ts->dev = dev;
 	ts->input = input_dev;
-	ts->pdata = dev_get_platdata(dev);
 	ts->bus_ops = bus_ops;
 	ts->irq = irq;
 
+	ts->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ts->reset_gpio)) {
+		error = PTR_ERR(ts->reset_gpio);
+		dev_err(dev, "Failed to request reset gpio, error %d\n", error);
+		return ERR_PTR(error);
+	}
+
+	error = cyttsp_parse_properties(ts);
+	if (error)
+		return ERR_PTR(error);
+
 	init_completion(&ts->bl_ready);
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
 
-	if (pdata->init) {
-		error = pdata->init();
-		if (error) {
-			dev_err(ts->dev, "platform init failed, err: %d\n",
-				error);
-			goto err_out;
-		}
-	}
-
-	input_dev->name = pdata->name;
+	input_dev->name = "cyttsp";
 	input_dev->phys = ts->phys;
 	input_dev->id.bustype = bus_ops->bustype;
 	input_dev->dev.parent = ts->dev;
@@ -576,59 +642,46 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 
 	input_set_drvdata(input_dev, ts);
 
-	__set_bit(EV_ABS, input_dev->evbit);
-	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
-			     0, pdata->maxx, 0, 0);
-	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
-			     0, pdata->maxy, 0, 0);
-	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
-			     0, CY_MAXZ, 0, 0);
+	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
+	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
+	touchscreen_parse_properties(input_dev, true);
+	if (!input_abs_get_max(input_dev, ABS_MT_POSITION_X) ||
+	    !input_abs_get_max(input_dev, ABS_MT_POSITION_Y)) {
+		dev_err(dev, "Touchscreen size is not specified\n");
+		return ERR_PTR(-EINVAL);
+	}
 
 	error = input_mt_init_slots(input_dev, CY_MAX_ID, 0);
 	if (error) {
 		dev_err(dev, "Unable to init MT slots.\n");
-		goto err_platform_exit;
+		return ERR_PTR(error);
 	}
 
 	error = devm_request_threaded_irq(dev, ts->irq, NULL, cyttsp_irq,
 					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					  pdata->name, ts);
+					  "cyttsp", ts);
 	if (error) {
 		dev_err(ts->dev, "failed to request IRQ %d, err: %d\n",
 			ts->irq, error);
-		goto err_platform_exit;
+		return ERR_PTR(error);
 	}
 
 	disable_irq(ts->irq);
 
 	error = cyttsp_power_on(ts);
 	if (error)
-		goto err_platform_exit;
+		return ERR_PTR(error);
 
 	error = input_register_device(input_dev);
 	if (error) {
 		dev_err(ts->dev, "failed to register input device: %d\n",
 			error);
-		goto err_platform_exit;
+		return ERR_PTR(error);
 	}
-
 	return ts;
-
-err_platform_exit:
-	if (pdata->exit)
-		pdata->exit();
-err_out:
-	return ERR_PTR(error);
 }
 EXPORT_SYMBOL_GPL(cyttsp_probe);
 
-void cyttsp_remove(struct cyttsp *ts)
-{
-	if (ts->pdata->exit)
-		ts->pdata->exit();
-}
-EXPORT_SYMBOL_GPL(cyttsp_remove);
-
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Cypress TrueTouch(R) Standard touchscreen driver core");
 MODULE_AUTHOR("Cypress");
diff --git a/drivers/input/touchscreen/cyttsp_core.h b/drivers/input/touchscreen/cyttsp_core.h
index 0707411..7835e2b 100644
--- a/drivers/input/touchscreen/cyttsp_core.h
+++ b/drivers/input/touchscreen/cyttsp_core.h
@@ -129,7 +129,6 @@ struct cyttsp {
 	int irq;
 	struct input_dev *input;
 	char phys[32];
-	const struct cyttsp_platform_data *pdata;
 	const struct cyttsp_bus_ops *bus_ops;
 	struct cyttsp_bootloader_data bl_data;
 	struct cyttsp_sysinfo_data sysinfo_data;
@@ -138,12 +137,19 @@ struct cyttsp {
 	enum cyttsp_state state;
 	bool suspended;
 
+	struct gpio_desc *reset_gpio;
+	bool use_hndshk;
+	u8 act_dist;
+	u8 act_intrvl;
+	u8 tch_tmout;
+	u8 lp_intrvl;
+	u8 *bl_keys;
+
 	u8 xfer_buf[] ____cacheline_aligned;
 };
 
 struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 			    struct device *dev, int irq, size_t xfer_buf_size);
-void cyttsp_remove(struct cyttsp *ts);
 
 int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
 		u8 length, const void *values);
diff --git a/drivers/input/touchscreen/cyttsp_i2c.c b/drivers/input/touchscreen/cyttsp_i2c.c
index eee51b3..1edfdba 100644
--- a/drivers/input/touchscreen/cyttsp_i2c.c
+++ b/drivers/input/touchscreen/cyttsp_i2c.c
@@ -56,15 +56,6 @@ static int cyttsp_i2c_probe(struct i2c_client *client,
 	return 0;
 }
 
-static int cyttsp_i2c_remove(struct i2c_client *client)
-{
-	struct cyttsp *ts = i2c_get_clientdata(client);
-
-	cyttsp_remove(ts);
-
-	return 0;
-}
-
 static const struct i2c_device_id cyttsp_i2c_id[] = {
 	{ CY_I2C_NAME, 0 },
 	{ }
@@ -77,7 +68,6 @@ static struct i2c_driver cyttsp_i2c_driver = {
 		.pm	= &cyttsp_pm_ops,
 	},
 	.probe		= cyttsp_i2c_probe,
-	.remove		= cyttsp_i2c_remove,
 	.id_table	= cyttsp_i2c_id,
 };
 
diff --git a/drivers/input/touchscreen/cyttsp_spi.c b/drivers/input/touchscreen/cyttsp_spi.c
index bbeeb24..3c9d18b 100644
--- a/drivers/input/touchscreen/cyttsp_spi.c
+++ b/drivers/input/touchscreen/cyttsp_spi.c
@@ -170,22 +170,12 @@ static int cyttsp_spi_probe(struct spi_device *spi)
 	return 0;
 }
 
-static int cyttsp_spi_remove(struct spi_device *spi)
-{
-	struct cyttsp *ts = spi_get_drvdata(spi);
-
-	cyttsp_remove(ts);
-
-	return 0;
-}
-
 static struct spi_driver cyttsp_spi_driver = {
 	.driver = {
 		.name	= CY_SPI_NAME,
 		.pm	= &cyttsp_pm_ops,
 	},
 	.probe  = cyttsp_spi_probe,
-	.remove = cyttsp_spi_remove,
 };
 
 module_spi_driver(cyttsp_spi_driver);
diff --git a/include/linux/input/cyttsp.h b/include/linux/input/cyttsp.h
index 5af7c66..586c8c9 100644
--- a/include/linux/input/cyttsp.h
+++ b/include/linux/input/cyttsp.h
@@ -40,19 +40,4 @@
 /* Active distance in pixels for a gesture to be reported */
 #define CY_ACT_DIST_DFLT 0xF8 /* pixels */
 
-struct cyttsp_platform_data {
-	u32 maxx;
-	u32 maxy;
-	bool use_hndshk;
-	u8 act_dist;	/* Active distance */
-	u8 act_intrvl;  /* Active refresh interval; ms */
-	u8 tch_tmout;   /* Active touch timeout; ms */
-	u8 lp_intrvl;   /* Low power refresh interval; ms */
-	int (*init)(void);
-	void (*exit)(void);
-	char *name;
-	s16 irq_gpio;
-	u8 *bl_keys;
-};
-
 #endif /* _CYTTSP_H_ */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 4/4] Input: cyttsp - perform hard reset of the chip during probe
  2016-01-21 19:21 [PATCH v4 0/4] Add device tree support to the cyttsp driver Oreste Salerno
                   ` (2 preceding siblings ...)
  2016-01-21 19:21 ` [PATCH v4 3/4] Input: cyttsp - switch to using device properties Oreste Salerno
@ 2016-01-21 19:21 ` Oreste Salerno
  3 siblings, 0 replies; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 19:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-input, fery, dmitry.torokhov, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, oreste.salerno

Since we removed support for calling an init() callback from
the platform data, introduce a function which initializes
the chip by performing a hard reset, using the reset gpio
defined in the device properties.

Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
---
 drivers/input/touchscreen/cyttsp_core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index 92b459d..19499a6 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -240,6 +240,14 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
 	return retval;
 }
 
+static void cyttsp_hard_reset(struct cyttsp *ts)
+{
+	gpiod_set_value_cansleep(ts->reset_gpio, 1);
+	msleep(CY_DELAY_DFLT);
+	gpiod_set_value_cansleep(ts->reset_gpio, 0);
+	msleep(CY_DELAY_DFLT);
+}
+
 static int cyttsp_soft_reset(struct cyttsp *ts)
 {
 	unsigned long timeout;
@@ -668,6 +676,8 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
 
 	disable_irq(ts->irq);
 
+	cyttsp_hard_reset(ts);
+
 	error = cyttsp_power_on(ts);
 	if (error)
 		return ERR_PTR(error);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 3/4] Input: cyttsp - switch to using device properties
       [not found]   ` <03a328d762d8ed654095d9a6edf648bc1c7fa262.1453403916.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
@ 2016-01-21 19:25     ` Oreste Salerno
  2016-01-21 19:35       ` Dmitry Torokhov
  0 siblings, 1 reply; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 19:25 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA, fery-+wT8y+m8/X5BDgjK7y7TUQ,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, Jan 21, 2016 at 08:21:15PM +0100, Oreste Salerno wrote:
> Drop support for platform data passed via a C-structure and switch to
> device properties instead, which should make the driver compatible
> with all platforms: OF, ACPI and static boards. Static boards should
> use property sets to communicate device parameters to the driver.
> 
> Signed-off-by: Oreste Salerno <oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
> ---
>  .../bindings/input/touchscreen/cyttsp.txt          |  95 ++++++++++++
>  drivers/input/touchscreen/cyttsp_core.c            | 167 ++++++++++++++-------
>  drivers/input/touchscreen/cyttsp_core.h            |  10 +-
>  drivers/input/touchscreen/cyttsp_i2c.c             |  10 --
>  drivers/input/touchscreen/cyttsp_spi.c             |  10 --
>  include/linux/input/cyttsp.h                       |  15 --
>  6 files changed, 213 insertions(+), 94 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> new file mode 100644
> index 0000000..b0fccae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> @@ -0,0 +1,95 @@
> +* 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)

I decided to explicitly specify the generic touchscreen properties here (instead of
referring to touchscreen.txt) because not all properties listed in touchscreen.txt
are actually parsed by the driver.

> + - 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:
> + - 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)
> + - 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
> + - use-handshake	: enable register-based handshake (boolean). This should
> +			  only be used if the chip is configured to use 'blocking
> +			  communication with timeout' (in this case the device
> +			  generates an interrupt at the end of every
> +			  scanning/processing cycle).
> +
> +[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 = <&gpio0>;
> +			interrupts = <28 0>;
> +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> +
> +			touchscreen-size-x = <800>;
> +			touchscreen-size-y = <480>;
> +			touchscreen-fuzz-x = <4>;
> +			touchscreen-fuzz-y = <7>;
> +
> +			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 = <&gpio0>;
> +			interrupts = <28 0>;
> +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> +
> +			touchscreen-size-x = <800>;
> +			touchscreen-size-y = <480>;
> +			touchscreen-fuzz-x = <4>;
> +			touchscreen-fuzz-y = <7>;
> +
> +			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>;
> +		};
> +
> +		/* ... */
> +	};
> diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> index 10379bc..92b459d 100644
> --- a/drivers/input/touchscreen/cyttsp_core.c
> +++ b/drivers/input/touchscreen/cyttsp_core.c
> @@ -30,9 +30,12 @@
>  #include <linux/delay.h>
>  #include <linux/input.h>
>  #include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
>  #include <linux/gpio.h>
>  #include <linux/interrupt.h>
>  #include <linux/slab.h>
> +#include <linux/property.h>
> +#include <linux/gpio/consumer.h>
>  
>  #include "cyttsp_core.h"
>  
> @@ -57,6 +60,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
> @@ -120,7 +124,7 @@ static int ttsp_send_command(struct cyttsp *ts, u8 cmd)
>  
>  static int cyttsp_handshake(struct cyttsp *ts)
>  {
> -	if (ts->pdata->use_hndshk)
> +	if (ts->use_hndshk)
>  		return ttsp_send_command(ts,
>  				ts->xy_data.hst_mode ^ CY_HNDSHK_BIT);
>  
> @@ -142,9 +146,9 @@ static int cyttsp_exit_bl_mode(struct cyttsp *ts)
>  	u8 bl_cmd[sizeof(bl_command)];
>  
>  	memcpy(bl_cmd, bl_command, sizeof(bl_command));
> -	if (ts->pdata->bl_keys)
> +	if (ts->bl_keys)
>  		memcpy(&bl_cmd[sizeof(bl_command) - CY_NUM_BL_KEYS],
> -			ts->pdata->bl_keys, CY_NUM_BL_KEYS);
> +			ts->bl_keys, CY_NUM_BL_KEYS);
>  
>  	error = ttsp_write_block_data(ts, CY_REG_BASE,
>  				      sizeof(bl_cmd), bl_cmd);
> @@ -217,14 +221,14 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
>  {
>  	int retval = 0;
>  
> -	if (ts->pdata->act_intrvl != CY_ACT_INTRVL_DFLT ||
> -	    ts->pdata->tch_tmout != CY_TCH_TMOUT_DFLT ||
> -	    ts->pdata->lp_intrvl != CY_LP_INTRVL_DFLT) {
> +	if (ts->act_intrvl != CY_ACT_INTRVL_DFLT ||
> +	    ts->tch_tmout != CY_TCH_TMOUT_DFLT ||
> +	    ts->lp_intrvl != CY_LP_INTRVL_DFLT) {
>  
>  		u8 intrvl_ray[] = {
> -			ts->pdata->act_intrvl,
> -			ts->pdata->tch_tmout,
> -			ts->pdata->lp_intrvl
> +			ts->act_intrvl,
> +			ts->tch_tmout,
> +			ts->lp_intrvl
>  		};
>  
>  		/* set intrvl registers */
> @@ -263,7 +267,7 @@ out:
>  
>  static int cyttsp_act_dist_setup(struct cyttsp *ts)
>  {
> -	u8 act_dist_setup = ts->pdata->act_dist;
> +	u8 act_dist_setup = ts->act_dist;
>  
>  	/* Init gesture; active distance setup */
>  	return ttsp_write_block_data(ts, CY_REG_ACT_DIST,
> @@ -528,45 +532,107 @@ static void cyttsp_close(struct input_dev *dev)
>  		cyttsp_disable(ts);
>  }
>  
> +static int cyttsp_parse_properties(struct cyttsp *ts)
> +{
> +	struct device *dev = ts->dev;
> +	u32 dt_value;
> +	int ret;
> +
> +	ts->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
> +	if (!ts->bl_keys)
> +		return -ENOMEM;
> +
> +	/* Set some default values */
> +	ts->use_hndshk = false;
> +	ts->act_dist = CY_ACT_DIST_DFLT;
> +	ts->act_intrvl = CY_ACT_INTRVL_DFLT;
> +	ts->tch_tmout = CY_TCH_TMOUT_DFLT;
> +	ts->lp_intrvl = CY_LP_INTRVL_DFLT;
> +
> +	ret = device_property_read_u8_array(dev, "bootloader-key",
> +					    ts->bl_keys, CY_NUM_BL_KEYS);
> +	if (ret) {
> +		dev_err(dev,
> +			"bootloader-key property could not be retrieved\n");
> +		return ret;
> +	}
> +
> +	ts->use_hndshk = device_property_present(dev, "use-handshake");
> +
> +	if (!device_property_read_u32(dev, "active-distance", &dt_value)) {
> +		if (dt_value > 15) {
> +			dev_err(dev, "active-distance (%u) must be [0-15]\n",
> +				dt_value);
> +			return -EINVAL;
> +		}
> +		ts->act_dist &= ~CY_ACT_DIST_MASK;
> +		ts->act_dist |= dt_value;
> +	}
> +
> +	if (!device_property_read_u32(dev, "active-interval-ms", &dt_value)) {
> +		if (dt_value > 255) {
> +			dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
> +				dt_value);
> +			return -EINVAL;
> +		}
> +		ts->act_intrvl = dt_value;
> +	}
> +
> +	if (!device_property_read_u32(dev, "lowpower-interval-ms", &dt_value)) {
> +		if (dt_value > 2550) {
> +			dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
> +				dt_value);
> +			return -EINVAL;
> +		}
> +		/* Register value is expressed in 0.01s / bit */
> +		ts->lp_intrvl = dt_value / 10;
> +	}
> +
> +	if (!device_property_read_u32(dev, "touch-timeout-ms", &dt_value)) {
> +		if (dt_value > 2550) {
> +			dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
> +				dt_value);
> +			return -EINVAL;
> +		}
> +		/* Register value is expressed in 0.01s / bit */
> +		ts->tch_tmout = dt_value/10;
> +	}
> +
> +	return 0;
> +}
> +
>  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);
>  	struct cyttsp *ts;
>  	struct input_dev *input_dev;
>  	int error;
>  
> -	if (!pdata || !pdata->name || irq <= 0) {
> -		error = -EINVAL;
> -		goto err_out;
> -	}
> -
>  	ts = devm_kzalloc(dev, sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
>  	input_dev = devm_input_allocate_device(dev);
> -	if (!ts || !input_dev) {
> -		error = -ENOMEM;
> -		goto err_out;
> -	}
> +	if (!ts || !input_dev)
> +		return ERR_PTR(-ENOMEM);
>  
>  	ts->dev = dev;
>  	ts->input = input_dev;
> -	ts->pdata = dev_get_platdata(dev);
>  	ts->bus_ops = bus_ops;
>  	ts->irq = irq;
>  
> +	ts->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ts->reset_gpio)) {
> +		error = PTR_ERR(ts->reset_gpio);
> +		dev_err(dev, "Failed to request reset gpio, error %d\n", error);
> +		return ERR_PTR(error);
> +	}
> +
> +	error = cyttsp_parse_properties(ts);
> +	if (error)
> +		return ERR_PTR(error);
> +
>  	init_completion(&ts->bl_ready);
>  	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
>  
> -	if (pdata->init) {
> -		error = pdata->init();
> -		if (error) {
> -			dev_err(ts->dev, "platform init failed, err: %d\n",
> -				error);
> -			goto err_out;
> -		}
> -	}
> -
> -	input_dev->name = pdata->name;
> +	input_dev->name = "cyttsp";
>  	input_dev->phys = ts->phys;
>  	input_dev->id.bustype = bus_ops->bustype;
>  	input_dev->dev.parent = ts->dev;
> @@ -576,59 +642,46 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
>  
>  	input_set_drvdata(input_dev, ts);
>  
> -	__set_bit(EV_ABS, input_dev->evbit);
> -	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> -			     0, pdata->maxx, 0, 0);
> -	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> -			     0, pdata->maxy, 0, 0);
> -	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
> -			     0, CY_MAXZ, 0, 0);
> +	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
> +	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
> +	touchscreen_parse_properties(input_dev, true);
> +	if (!input_abs_get_max(input_dev, ABS_MT_POSITION_X) ||
> +	    !input_abs_get_max(input_dev, ABS_MT_POSITION_Y)) {
> +		dev_err(dev, "Touchscreen size is not specified\n");
> +		return ERR_PTR(-EINVAL);
> +	}
>  
>  	error = input_mt_init_slots(input_dev, CY_MAX_ID, 0);
>  	if (error) {
>  		dev_err(dev, "Unable to init MT slots.\n");
> -		goto err_platform_exit;
> +		return ERR_PTR(error);
>  	}
>  
>  	error = devm_request_threaded_irq(dev, ts->irq, NULL, cyttsp_irq,
>  					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -					  pdata->name, ts);
> +					  "cyttsp", ts);
>  	if (error) {
>  		dev_err(ts->dev, "failed to request IRQ %d, err: %d\n",
>  			ts->irq, error);
> -		goto err_platform_exit;
> +		return ERR_PTR(error);
>  	}
>  
>  	disable_irq(ts->irq);
>  
>  	error = cyttsp_power_on(ts);
>  	if (error)
> -		goto err_platform_exit;
> +		return ERR_PTR(error);
>  
>  	error = input_register_device(input_dev);
>  	if (error) {
>  		dev_err(ts->dev, "failed to register input device: %d\n",
>  			error);
> -		goto err_platform_exit;
> +		return ERR_PTR(error);
>  	}
> -
>  	return ts;
> -
> -err_platform_exit:
> -	if (pdata->exit)
> -		pdata->exit();
> -err_out:
> -	return ERR_PTR(error);
>  }
>  EXPORT_SYMBOL_GPL(cyttsp_probe);
>  
> -void cyttsp_remove(struct cyttsp *ts)
> -{
> -	if (ts->pdata->exit)
> -		ts->pdata->exit();
> -}
> -EXPORT_SYMBOL_GPL(cyttsp_remove);
> -
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("Cypress TrueTouch(R) Standard touchscreen driver core");
>  MODULE_AUTHOR("Cypress");
> diff --git a/drivers/input/touchscreen/cyttsp_core.h b/drivers/input/touchscreen/cyttsp_core.h
> index 0707411..7835e2b 100644
> --- a/drivers/input/touchscreen/cyttsp_core.h
> +++ b/drivers/input/touchscreen/cyttsp_core.h
> @@ -129,7 +129,6 @@ struct cyttsp {
>  	int irq;
>  	struct input_dev *input;
>  	char phys[32];
> -	const struct cyttsp_platform_data *pdata;
>  	const struct cyttsp_bus_ops *bus_ops;
>  	struct cyttsp_bootloader_data bl_data;
>  	struct cyttsp_sysinfo_data sysinfo_data;
> @@ -138,12 +137,19 @@ struct cyttsp {
>  	enum cyttsp_state state;
>  	bool suspended;
>  
> +	struct gpio_desc *reset_gpio;
> +	bool use_hndshk;
> +	u8 act_dist;
> +	u8 act_intrvl;
> +	u8 tch_tmout;
> +	u8 lp_intrvl;
> +	u8 *bl_keys;
> +
>  	u8 xfer_buf[] ____cacheline_aligned;
>  };
>  
>  struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
>  			    struct device *dev, int irq, size_t xfer_buf_size);
> -void cyttsp_remove(struct cyttsp *ts);
>  
>  int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
>  		u8 length, const void *values);
> diff --git a/drivers/input/touchscreen/cyttsp_i2c.c b/drivers/input/touchscreen/cyttsp_i2c.c
> index eee51b3..1edfdba 100644
> --- a/drivers/input/touchscreen/cyttsp_i2c.c
> +++ b/drivers/input/touchscreen/cyttsp_i2c.c
> @@ -56,15 +56,6 @@ static int cyttsp_i2c_probe(struct i2c_client *client,
>  	return 0;
>  }
>  
> -static int cyttsp_i2c_remove(struct i2c_client *client)
> -{
> -	struct cyttsp *ts = i2c_get_clientdata(client);
> -
> -	cyttsp_remove(ts);
> -
> -	return 0;
> -}
> -
>  static const struct i2c_device_id cyttsp_i2c_id[] = {
>  	{ CY_I2C_NAME, 0 },
>  	{ }
> @@ -77,7 +68,6 @@ static struct i2c_driver cyttsp_i2c_driver = {
>  		.pm	= &cyttsp_pm_ops,
>  	},
>  	.probe		= cyttsp_i2c_probe,
> -	.remove		= cyttsp_i2c_remove,
>  	.id_table	= cyttsp_i2c_id,
>  };
>  
> diff --git a/drivers/input/touchscreen/cyttsp_spi.c b/drivers/input/touchscreen/cyttsp_spi.c
> index bbeeb24..3c9d18b 100644
> --- a/drivers/input/touchscreen/cyttsp_spi.c
> +++ b/drivers/input/touchscreen/cyttsp_spi.c
> @@ -170,22 +170,12 @@ static int cyttsp_spi_probe(struct spi_device *spi)
>  	return 0;
>  }
>  
> -static int cyttsp_spi_remove(struct spi_device *spi)
> -{
> -	struct cyttsp *ts = spi_get_drvdata(spi);
> -
> -	cyttsp_remove(ts);
> -
> -	return 0;
> -}
> -
>  static struct spi_driver cyttsp_spi_driver = {
>  	.driver = {
>  		.name	= CY_SPI_NAME,
>  		.pm	= &cyttsp_pm_ops,
>  	},
>  	.probe  = cyttsp_spi_probe,
> -	.remove = cyttsp_spi_remove,
>  };
>  
>  module_spi_driver(cyttsp_spi_driver);
> diff --git a/include/linux/input/cyttsp.h b/include/linux/input/cyttsp.h
> index 5af7c66..586c8c9 100644
> --- a/include/linux/input/cyttsp.h
> +++ b/include/linux/input/cyttsp.h
> @@ -40,19 +40,4 @@
>  /* Active distance in pixels for a gesture to be reported */
>  #define CY_ACT_DIST_DFLT 0xF8 /* pixels */
>  
> -struct cyttsp_platform_data {
> -	u32 maxx;
> -	u32 maxy;
> -	bool use_hndshk;
> -	u8 act_dist;	/* Active distance */
> -	u8 act_intrvl;  /* Active refresh interval; ms */
> -	u8 tch_tmout;   /* Active touch timeout; ms */
> -	u8 lp_intrvl;   /* Low power refresh interval; ms */
> -	int (*init)(void);
> -	void (*exit)(void);
> -	char *name;
> -	s16 irq_gpio;
> -	u8 *bl_keys;
> -};
> -
>  #endif /* _CYTTSP_H_ */
> -- 
> 1.9.1
> 
--
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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations
       [not found]   ` <20e9f0188f07fc420912b3defa747303be3cf290.1453403916.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
@ 2016-01-21 19:29     ` Dmitry Torokhov
  2016-01-21 20:22       ` Oreste Salerno
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2016-01-21 19:29 UTC (permalink / raw)
  To: Oreste Salerno
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, fery-+wT8y+m8/X5BDgjK7y7TUQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Oreste,

On Thu, Jan 21, 2016 at 08:21:13PM +0100, Oreste Salerno wrote:
> Use devm_() functions for allocating memory, input device and IRQ.
> 

...

> @@ -625,11 +620,8 @@ EXPORT_SYMBOL_GPL(cyttsp_probe);
>  
>  void cyttsp_remove(struct cyttsp *ts)
>  {
> -	free_irq(ts->irq, ts);
> -	input_unregister_device(ts->input);
>  	if (ts->pdata->exit)
>  		ts->pdata->exit();
> -	kfree(ts);

This is not quite correct as it changes the operations order... Given
that it is going to be removed in the next patch I guess it is OK, but
ideally you can add a custom devm action to shut off the chip.

Thanks.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 3/4] Input: cyttsp - switch to using device properties
  2016-01-21 19:25     ` Oreste Salerno
@ 2016-01-21 19:35       ` Dmitry Torokhov
  2016-01-21 20:44         ` Oreste Salerno
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2016-01-21 19:35 UTC (permalink / raw)
  To: Oreste Salerno
  Cc: linux-kernel, linux-input, fery, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree

On Thu, Jan 21, 2016 at 08:25:53PM +0100, Oreste Salerno wrote:
> On Thu, Jan 21, 2016 at 08:21:15PM +0100, Oreste Salerno wrote:
> > Drop support for platform data passed via a C-structure and switch to
> > device properties instead, which should make the driver compatible
> > with all platforms: OF, ACPI and static boards. Static boards should
> > use property sets to communicate device parameters to the driver.
> > 
> > Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
> > ---
> >  .../bindings/input/touchscreen/cyttsp.txt          |  95 ++++++++++++
> >  drivers/input/touchscreen/cyttsp_core.c            | 167 ++++++++++++++-------
> >  drivers/input/touchscreen/cyttsp_core.h            |  10 +-
> >  drivers/input/touchscreen/cyttsp_i2c.c             |  10 --
> >  drivers/input/touchscreen/cyttsp_spi.c             |  10 --
> >  include/linux/input/cyttsp.h                       |  15 --
> >  6 files changed, 213 insertions(+), 94 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > new file mode 100644
> > index 0000000..b0fccae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > @@ -0,0 +1,95 @@
> > +* 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).

Why do we have to have reset gpio available? If platform firmware powers up and
resets the controller and keeps it's power do they have to expose reset
gpio?

Also, maybe you also need to wire up power supply support (maybe a
follwup patch)?


> > + - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
> > + - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)

Do these have to be mandatory? Will not the driver work without them?

> 
> I decided to explicitly specify the generic touchscreen properties here (instead of
> referring to touchscreen.txt) because not all properties listed in touchscreen.txt
> are actually parsed by the driver.

Fair enough.

Thanks.

> 
> > + - 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:
> > + - 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)
> > + - 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
> > + - use-handshake	: enable register-based handshake (boolean). This should
> > +			  only be used if the chip is configured to use 'blocking
> > +			  communication with timeout' (in this case the device
> > +			  generates an interrupt at the end of every
> > +			  scanning/processing cycle).
> > +
> > +[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 = <&gpio0>;
> > +			interrupts = <28 0>;
> > +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> > +
> > +			touchscreen-size-x = <800>;
> > +			touchscreen-size-y = <480>;
> > +			touchscreen-fuzz-x = <4>;
> > +			touchscreen-fuzz-y = <7>;
> > +
> > +			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 = <&gpio0>;
> > +			interrupts = <28 0>;
> > +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> > +
> > +			touchscreen-size-x = <800>;
> > +			touchscreen-size-y = <480>;
> > +			touchscreen-fuzz-x = <4>;
> > +			touchscreen-fuzz-y = <7>;
> > +
> > +			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>;
> > +		};
> > +
> > +		/* ... */
> > +	};
> > diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> > index 10379bc..92b459d 100644
> > --- a/drivers/input/touchscreen/cyttsp_core.c
> > +++ b/drivers/input/touchscreen/cyttsp_core.c
> > @@ -30,9 +30,12 @@
> >  #include <linux/delay.h>
> >  #include <linux/input.h>
> >  #include <linux/input/mt.h>
> > +#include <linux/input/touchscreen.h>
> >  #include <linux/gpio.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/slab.h>
> > +#include <linux/property.h>
> > +#include <linux/gpio/consumer.h>
> >  
> >  #include "cyttsp_core.h"
> >  
> > @@ -57,6 +60,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
> > @@ -120,7 +124,7 @@ static int ttsp_send_command(struct cyttsp *ts, u8 cmd)
> >  
> >  static int cyttsp_handshake(struct cyttsp *ts)
> >  {
> > -	if (ts->pdata->use_hndshk)
> > +	if (ts->use_hndshk)
> >  		return ttsp_send_command(ts,
> >  				ts->xy_data.hst_mode ^ CY_HNDSHK_BIT);
> >  
> > @@ -142,9 +146,9 @@ static int cyttsp_exit_bl_mode(struct cyttsp *ts)
> >  	u8 bl_cmd[sizeof(bl_command)];
> >  
> >  	memcpy(bl_cmd, bl_command, sizeof(bl_command));
> > -	if (ts->pdata->bl_keys)
> > +	if (ts->bl_keys)
> >  		memcpy(&bl_cmd[sizeof(bl_command) - CY_NUM_BL_KEYS],
> > -			ts->pdata->bl_keys, CY_NUM_BL_KEYS);
> > +			ts->bl_keys, CY_NUM_BL_KEYS);
> >  
> >  	error = ttsp_write_block_data(ts, CY_REG_BASE,
> >  				      sizeof(bl_cmd), bl_cmd);
> > @@ -217,14 +221,14 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
> >  {
> >  	int retval = 0;
> >  
> > -	if (ts->pdata->act_intrvl != CY_ACT_INTRVL_DFLT ||
> > -	    ts->pdata->tch_tmout != CY_TCH_TMOUT_DFLT ||
> > -	    ts->pdata->lp_intrvl != CY_LP_INTRVL_DFLT) {
> > +	if (ts->act_intrvl != CY_ACT_INTRVL_DFLT ||
> > +	    ts->tch_tmout != CY_TCH_TMOUT_DFLT ||
> > +	    ts->lp_intrvl != CY_LP_INTRVL_DFLT) {
> >  
> >  		u8 intrvl_ray[] = {
> > -			ts->pdata->act_intrvl,
> > -			ts->pdata->tch_tmout,
> > -			ts->pdata->lp_intrvl
> > +			ts->act_intrvl,
> > +			ts->tch_tmout,
> > +			ts->lp_intrvl
> >  		};
> >  
> >  		/* set intrvl registers */
> > @@ -263,7 +267,7 @@ out:
> >  
> >  static int cyttsp_act_dist_setup(struct cyttsp *ts)
> >  {
> > -	u8 act_dist_setup = ts->pdata->act_dist;
> > +	u8 act_dist_setup = ts->act_dist;
> >  
> >  	/* Init gesture; active distance setup */
> >  	return ttsp_write_block_data(ts, CY_REG_ACT_DIST,
> > @@ -528,45 +532,107 @@ static void cyttsp_close(struct input_dev *dev)
> >  		cyttsp_disable(ts);
> >  }
> >  
> > +static int cyttsp_parse_properties(struct cyttsp *ts)
> > +{
> > +	struct device *dev = ts->dev;
> > +	u32 dt_value;
> > +	int ret;
> > +
> > +	ts->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
> > +	if (!ts->bl_keys)
> > +		return -ENOMEM;
> > +
> > +	/* Set some default values */
> > +	ts->use_hndshk = false;
> > +	ts->act_dist = CY_ACT_DIST_DFLT;
> > +	ts->act_intrvl = CY_ACT_INTRVL_DFLT;
> > +	ts->tch_tmout = CY_TCH_TMOUT_DFLT;
> > +	ts->lp_intrvl = CY_LP_INTRVL_DFLT;
> > +
> > +	ret = device_property_read_u8_array(dev, "bootloader-key",
> > +					    ts->bl_keys, CY_NUM_BL_KEYS);
> > +	if (ret) {
> > +		dev_err(dev,
> > +			"bootloader-key property could not be retrieved\n");
> > +		return ret;
> > +	}
> > +
> > +	ts->use_hndshk = device_property_present(dev, "use-handshake");
> > +
> > +	if (!device_property_read_u32(dev, "active-distance", &dt_value)) {
> > +		if (dt_value > 15) {
> > +			dev_err(dev, "active-distance (%u) must be [0-15]\n",
> > +				dt_value);
> > +			return -EINVAL;
> > +		}
> > +		ts->act_dist &= ~CY_ACT_DIST_MASK;
> > +		ts->act_dist |= dt_value;
> > +	}
> > +
> > +	if (!device_property_read_u32(dev, "active-interval-ms", &dt_value)) {
> > +		if (dt_value > 255) {
> > +			dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
> > +				dt_value);
> > +			return -EINVAL;
> > +		}
> > +		ts->act_intrvl = dt_value;
> > +	}
> > +
> > +	if (!device_property_read_u32(dev, "lowpower-interval-ms", &dt_value)) {
> > +		if (dt_value > 2550) {
> > +			dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
> > +				dt_value);
> > +			return -EINVAL;
> > +		}
> > +		/* Register value is expressed in 0.01s / bit */
> > +		ts->lp_intrvl = dt_value / 10;
> > +	}
> > +
> > +	if (!device_property_read_u32(dev, "touch-timeout-ms", &dt_value)) {
> > +		if (dt_value > 2550) {
> > +			dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
> > +				dt_value);
> > +			return -EINVAL;
> > +		}
> > +		/* Register value is expressed in 0.01s / bit */
> > +		ts->tch_tmout = dt_value/10;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  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);
> >  	struct cyttsp *ts;
> >  	struct input_dev *input_dev;
> >  	int error;
> >  
> > -	if (!pdata || !pdata->name || irq <= 0) {
> > -		error = -EINVAL;
> > -		goto err_out;
> > -	}
> > -
> >  	ts = devm_kzalloc(dev, sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
> >  	input_dev = devm_input_allocate_device(dev);
> > -	if (!ts || !input_dev) {
> > -		error = -ENOMEM;
> > -		goto err_out;
> > -	}
> > +	if (!ts || !input_dev)
> > +		return ERR_PTR(-ENOMEM);
> >  
> >  	ts->dev = dev;
> >  	ts->input = input_dev;
> > -	ts->pdata = dev_get_platdata(dev);
> >  	ts->bus_ops = bus_ops;
> >  	ts->irq = irq;
> >  
> > +	ts->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > +	if (IS_ERR(ts->reset_gpio)) {
> > +		error = PTR_ERR(ts->reset_gpio);
> > +		dev_err(dev, "Failed to request reset gpio, error %d\n", error);
> > +		return ERR_PTR(error);
> > +	}
> > +
> > +	error = cyttsp_parse_properties(ts);
> > +	if (error)
> > +		return ERR_PTR(error);
> > +
> >  	init_completion(&ts->bl_ready);
> >  	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
> >  
> > -	if (pdata->init) {
> > -		error = pdata->init();
> > -		if (error) {
> > -			dev_err(ts->dev, "platform init failed, err: %d\n",
> > -				error);
> > -			goto err_out;
> > -		}
> > -	}
> > -
> > -	input_dev->name = pdata->name;
> > +	input_dev->name = "cyttsp";

Is there a friendlier name we could use? It is exported in
/proc/bus/input/devices and usually is more descriptive.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations
  2016-01-21 19:29     ` Dmitry Torokhov
@ 2016-01-21 20:22       ` Oreste Salerno
  2016-01-21 21:32         ` Dmitry Torokhov
  0 siblings, 1 reply; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 20:22 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-input, fery, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree

On Thu, Jan 21, 2016 at 11:29:24AM -0800, Dmitry Torokhov wrote:
> Hi Oreste,
> 
> On Thu, Jan 21, 2016 at 08:21:13PM +0100, Oreste Salerno wrote:
> > Use devm_() functions for allocating memory, input device and IRQ.
> > 
> 
> ...
> 
> > @@ -625,11 +620,8 @@ EXPORT_SYMBOL_GPL(cyttsp_probe);
> >  
> >  void cyttsp_remove(struct cyttsp *ts)
> >  {
> > -	free_irq(ts->irq, ts);
> > -	input_unregister_device(ts->input);
> >  	if (ts->pdata->exit)
> >  		ts->pdata->exit();
> > -	kfree(ts);
> 
> This is not quite correct as it changes the operations order... Given
> that it is going to be removed in the next patch I guess it is OK, but
> ideally you can add a custom devm action to shut off the chip.
>

You mean a devm action to call pdata->exit() ? OK, I could do that.

> Thanks.
> 
> -- 
> Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 3/4] Input: cyttsp - switch to using device properties
  2016-01-21 19:35       ` Dmitry Torokhov
@ 2016-01-21 20:44         ` Oreste Salerno
  2016-01-21 21:33           ` Dmitry Torokhov
  0 siblings, 1 reply; 12+ messages in thread
From: Oreste Salerno @ 2016-01-21 20:44 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-input, fery, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree

On Thu, Jan 21, 2016 at 11:35:46AM -0800, Dmitry Torokhov wrote:
> On Thu, Jan 21, 2016 at 08:25:53PM +0100, Oreste Salerno wrote:
> > On Thu, Jan 21, 2016 at 08:21:15PM +0100, Oreste Salerno wrote:
> > > Drop support for platform data passed via a C-structure and switch to
> > > device properties instead, which should make the driver compatible
> > > with all platforms: OF, ACPI and static boards. Static boards should
> > > use property sets to communicate device parameters to the driver.
> > > 
> > > Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
> > > ---
> > >  .../bindings/input/touchscreen/cyttsp.txt          |  95 ++++++++++++
> > >  drivers/input/touchscreen/cyttsp_core.c            | 167 ++++++++++++++-------
> > >  drivers/input/touchscreen/cyttsp_core.h            |  10 +-
> > >  drivers/input/touchscreen/cyttsp_i2c.c             |  10 --
> > >  drivers/input/touchscreen/cyttsp_spi.c             |  10 --
> > >  include/linux/input/cyttsp.h                       |  15 --
> > >  6 files changed, 213 insertions(+), 94 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > new file mode 100644
> > > index 0000000..b0fccae
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > @@ -0,0 +1,95 @@
> > > +* 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).
> 
> Why do we have to have reset gpio available? If platform firmware powers up and
> resets the controller and keeps it's power do they have to expose reset
> gpio?

No, if this is handled in some other way I guess it won't be necessary.
I can make it optional.
 
> Also, maybe you also need to wire up power supply support (maybe a
> follwup patch)?

Yes, I can do that in a followup patch if that's ok. 

> 
> > > + - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
> > > + - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
> 
> Do these have to be mandatory? Will not the driver work without them?
>

To be honest I had not tried it yet. I've just tried and it works ok.

> > 
> > I decided to explicitly specify the generic touchscreen properties here (instead of
> > referring to touchscreen.txt) because not all properties listed in touchscreen.txt
> > are actually parsed by the driver.
> 
> Fair enough.
> 
> Thanks.
> 
> > 
> > > + - 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:
> > > + - 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)
> > > + - 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
> > > + - use-handshake	: enable register-based handshake (boolean). This should
> > > +			  only be used if the chip is configured to use 'blocking
> > > +			  communication with timeout' (in this case the device
> > > +			  generates an interrupt at the end of every
> > > +			  scanning/processing cycle).
> > > +
> > > +[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 = <&gpio0>;
> > > +			interrupts = <28 0>;
> > > +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> > > +
> > > +			touchscreen-size-x = <800>;
> > > +			touchscreen-size-y = <480>;
> > > +			touchscreen-fuzz-x = <4>;
> > > +			touchscreen-fuzz-y = <7>;
> > > +
> > > +			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 = <&gpio0>;
> > > +			interrupts = <28 0>;
> > > +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> > > +
> > > +			touchscreen-size-x = <800>;
> > > +			touchscreen-size-y = <480>;
> > > +			touchscreen-fuzz-x = <4>;
> > > +			touchscreen-fuzz-y = <7>;
> > > +
> > > +			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>;
> > > +		};
> > > +
> > > +		/* ... */
> > > +	};
> > > diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> > > index 10379bc..92b459d 100644
> > > --- a/drivers/input/touchscreen/cyttsp_core.c
> > > +++ b/drivers/input/touchscreen/cyttsp_core.c
> > > @@ -30,9 +30,12 @@
> > >  #include <linux/delay.h>
> > >  #include <linux/input.h>
> > >  #include <linux/input/mt.h>
> > > +#include <linux/input/touchscreen.h>
> > >  #include <linux/gpio.h>
> > >  #include <linux/interrupt.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/property.h>
> > > +#include <linux/gpio/consumer.h>
> > >  
> > >  #include "cyttsp_core.h"
> > >  
> > > @@ -57,6 +60,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
> > > @@ -120,7 +124,7 @@ static int ttsp_send_command(struct cyttsp *ts, u8 cmd)
> > >  
> > >  static int cyttsp_handshake(struct cyttsp *ts)
> > >  {
> > > -	if (ts->pdata->use_hndshk)
> > > +	if (ts->use_hndshk)
> > >  		return ttsp_send_command(ts,
> > >  				ts->xy_data.hst_mode ^ CY_HNDSHK_BIT);
> > >  
> > > @@ -142,9 +146,9 @@ static int cyttsp_exit_bl_mode(struct cyttsp *ts)
> > >  	u8 bl_cmd[sizeof(bl_command)];
> > >  
> > >  	memcpy(bl_cmd, bl_command, sizeof(bl_command));
> > > -	if (ts->pdata->bl_keys)
> > > +	if (ts->bl_keys)
> > >  		memcpy(&bl_cmd[sizeof(bl_command) - CY_NUM_BL_KEYS],
> > > -			ts->pdata->bl_keys, CY_NUM_BL_KEYS);
> > > +			ts->bl_keys, CY_NUM_BL_KEYS);
> > >  
> > >  	error = ttsp_write_block_data(ts, CY_REG_BASE,
> > >  				      sizeof(bl_cmd), bl_cmd);
> > > @@ -217,14 +221,14 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
> > >  {
> > >  	int retval = 0;
> > >  
> > > -	if (ts->pdata->act_intrvl != CY_ACT_INTRVL_DFLT ||
> > > -	    ts->pdata->tch_tmout != CY_TCH_TMOUT_DFLT ||
> > > -	    ts->pdata->lp_intrvl != CY_LP_INTRVL_DFLT) {
> > > +	if (ts->act_intrvl != CY_ACT_INTRVL_DFLT ||
> > > +	    ts->tch_tmout != CY_TCH_TMOUT_DFLT ||
> > > +	    ts->lp_intrvl != CY_LP_INTRVL_DFLT) {
> > >  
> > >  		u8 intrvl_ray[] = {
> > > -			ts->pdata->act_intrvl,
> > > -			ts->pdata->tch_tmout,
> > > -			ts->pdata->lp_intrvl
> > > +			ts->act_intrvl,
> > > +			ts->tch_tmout,
> > > +			ts->lp_intrvl
> > >  		};
> > >  
> > >  		/* set intrvl registers */
> > > @@ -263,7 +267,7 @@ out:
> > >  
> > >  static int cyttsp_act_dist_setup(struct cyttsp *ts)
> > >  {
> > > -	u8 act_dist_setup = ts->pdata->act_dist;
> > > +	u8 act_dist_setup = ts->act_dist;
> > >  
> > >  	/* Init gesture; active distance setup */
> > >  	return ttsp_write_block_data(ts, CY_REG_ACT_DIST,
> > > @@ -528,45 +532,107 @@ static void cyttsp_close(struct input_dev *dev)
> > >  		cyttsp_disable(ts);
> > >  }
> > >  
> > > +static int cyttsp_parse_properties(struct cyttsp *ts)
> > > +{
> > > +	struct device *dev = ts->dev;
> > > +	u32 dt_value;
> > > +	int ret;
> > > +
> > > +	ts->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
> > > +	if (!ts->bl_keys)
> > > +		return -ENOMEM;
> > > +
> > > +	/* Set some default values */
> > > +	ts->use_hndshk = false;
> > > +	ts->act_dist = CY_ACT_DIST_DFLT;
> > > +	ts->act_intrvl = CY_ACT_INTRVL_DFLT;
> > > +	ts->tch_tmout = CY_TCH_TMOUT_DFLT;
> > > +	ts->lp_intrvl = CY_LP_INTRVL_DFLT;
> > > +
> > > +	ret = device_property_read_u8_array(dev, "bootloader-key",
> > > +					    ts->bl_keys, CY_NUM_BL_KEYS);
> > > +	if (ret) {
> > > +		dev_err(dev,
> > > +			"bootloader-key property could not be retrieved\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ts->use_hndshk = device_property_present(dev, "use-handshake");
> > > +
> > > +	if (!device_property_read_u32(dev, "active-distance", &dt_value)) {
> > > +		if (dt_value > 15) {
> > > +			dev_err(dev, "active-distance (%u) must be [0-15]\n",
> > > +				dt_value);
> > > +			return -EINVAL;
> > > +		}
> > > +		ts->act_dist &= ~CY_ACT_DIST_MASK;
> > > +		ts->act_dist |= dt_value;
> > > +	}
> > > +
> > > +	if (!device_property_read_u32(dev, "active-interval-ms", &dt_value)) {
> > > +		if (dt_value > 255) {
> > > +			dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
> > > +				dt_value);
> > > +			return -EINVAL;
> > > +		}
> > > +		ts->act_intrvl = dt_value;
> > > +	}
> > > +
> > > +	if (!device_property_read_u32(dev, "lowpower-interval-ms", &dt_value)) {
> > > +		if (dt_value > 2550) {
> > > +			dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
> > > +				dt_value);
> > > +			return -EINVAL;
> > > +		}
> > > +		/* Register value is expressed in 0.01s / bit */
> > > +		ts->lp_intrvl = dt_value / 10;
> > > +	}
> > > +
> > > +	if (!device_property_read_u32(dev, "touch-timeout-ms", &dt_value)) {
> > > +		if (dt_value > 2550) {
> > > +			dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
> > > +				dt_value);
> > > +			return -EINVAL;
> > > +		}
> > > +		/* Register value is expressed in 0.01s / bit */
> > > +		ts->tch_tmout = dt_value/10;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  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);
> > >  	struct cyttsp *ts;
> > >  	struct input_dev *input_dev;
> > >  	int error;
> > >  
> > > -	if (!pdata || !pdata->name || irq <= 0) {
> > > -		error = -EINVAL;
> > > -		goto err_out;
> > > -	}
> > > -
> > >  	ts = devm_kzalloc(dev, sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
> > >  	input_dev = devm_input_allocate_device(dev);
> > > -	if (!ts || !input_dev) {
> > > -		error = -ENOMEM;
> > > -		goto err_out;
> > > -	}
> > > +	if (!ts || !input_dev)
> > > +		return ERR_PTR(-ENOMEM);
> > >  
> > >  	ts->dev = dev;
> > >  	ts->input = input_dev;
> > > -	ts->pdata = dev_get_platdata(dev);
> > >  	ts->bus_ops = bus_ops;
> > >  	ts->irq = irq;
> > >  
> > > +	ts->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > +	if (IS_ERR(ts->reset_gpio)) {
> > > +		error = PTR_ERR(ts->reset_gpio);
> > > +		dev_err(dev, "Failed to request reset gpio, error %d\n", error);
> > > +		return ERR_PTR(error);
> > > +	}
> > > +
> > > +	error = cyttsp_parse_properties(ts);
> > > +	if (error)
> > > +		return ERR_PTR(error);
> > > +
> > >  	init_completion(&ts->bl_ready);
> > >  	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
> > >  
> > > -	if (pdata->init) {
> > > -		error = pdata->init();
> > > -		if (error) {
> > > -			dev_err(ts->dev, "platform init failed, err: %d\n",
> > > -				error);
> > > -			goto err_out;
> > > -		}
> > > -	}
> > > -
> > > -	input_dev->name = pdata->name;
> > > +	input_dev->name = "cyttsp";
> 
> Is there a friendlier name we could use? It is exported in
> /proc/bus/input/devices and usually is more descriptive.
>

Something like "Cypress TTSP touchscreen"? Same naming used in the Kconfig.

> Thanks.
> 
> -- 
> Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations
  2016-01-21 20:22       ` Oreste Salerno
@ 2016-01-21 21:32         ` Dmitry Torokhov
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2016-01-21 21:32 UTC (permalink / raw)
  To: Oreste Salerno
  Cc: linux-kernel, linux-input, fery, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree

On Thu, Jan 21, 2016 at 09:22:05PM +0100, Oreste Salerno wrote:
> On Thu, Jan 21, 2016 at 11:29:24AM -0800, Dmitry Torokhov wrote:
> > Hi Oreste,
> > 
> > On Thu, Jan 21, 2016 at 08:21:13PM +0100, Oreste Salerno wrote:
> > > Use devm_() functions for allocating memory, input device and IRQ.
> > > 
> > 
> > ...
> > 
> > > @@ -625,11 +620,8 @@ EXPORT_SYMBOL_GPL(cyttsp_probe);
> > >  
> > >  void cyttsp_remove(struct cyttsp *ts)
> > >  {
> > > -	free_irq(ts->irq, ts);
> > > -	input_unregister_device(ts->input);
> > >  	if (ts->pdata->exit)
> > >  		ts->pdata->exit();
> > > -	kfree(ts);
> > 
> > This is not quite correct as it changes the operations order... Given
> > that it is going to be removed in the next patch I guess it is OK, but
> > ideally you can add a custom devm action to shut off the chip.
> >
> 
> You mean a devm action to call pdata->exit() ? OK, I could do that.

Yes.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 3/4] Input: cyttsp - switch to using device properties
  2016-01-21 20:44         ` Oreste Salerno
@ 2016-01-21 21:33           ` Dmitry Torokhov
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2016-01-21 21:33 UTC (permalink / raw)
  To: Oreste Salerno
  Cc: linux-kernel, linux-input, fery, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree

On Thu, Jan 21, 2016 at 09:44:24PM +0100, Oreste Salerno wrote:
> On Thu, Jan 21, 2016 at 11:35:46AM -0800, Dmitry Torokhov wrote:
> > On Thu, Jan 21, 2016 at 08:25:53PM +0100, Oreste Salerno wrote:
> > > On Thu, Jan 21, 2016 at 08:21:15PM +0100, Oreste Salerno wrote:
> > > > Drop support for platform data passed via a C-structure and switch to
> > > > device properties instead, which should make the driver compatible
> > > > with all platforms: OF, ACPI and static boards. Static boards should
> > > > use property sets to communicate device parameters to the driver.
> > > > 
> > > > Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
> > > > ---
> > > >  .../bindings/input/touchscreen/cyttsp.txt          |  95 ++++++++++++
> > > >  drivers/input/touchscreen/cyttsp_core.c            | 167 ++++++++++++++-------
> > > >  drivers/input/touchscreen/cyttsp_core.h            |  10 +-
> > > >  drivers/input/touchscreen/cyttsp_i2c.c             |  10 --
> > > >  drivers/input/touchscreen/cyttsp_spi.c             |  10 --
> > > >  include/linux/input/cyttsp.h                       |  15 --
> > > >  6 files changed, 213 insertions(+), 94 deletions(-)
> > > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > > new file mode 100644
> > > > index 0000000..b0fccae
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > > @@ -0,0 +1,95 @@
> > > > +* 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).
> > 
> > Why do we have to have reset gpio available? If platform firmware powers up and
> > resets the controller and keeps it's power do they have to expose reset
> > gpio?
> 
> No, if this is handled in some other way I guess it won't be necessary.
> I can make it optional.

Please.

>  
> > Also, maybe you also need to wire up power supply support (maybe a
> > follwup patch)?
> 
> Yes, I can do that in a followup patch if that's ok. 

Sure.

> 
> > 
> > > > + - touchscreen-size-x	: horizontal resolution of touchscreen (in pixels)
> > > > + - touchscreen-size-y	: vertical resolution of touchscreen (in pixels)
> > 
> > Do these have to be mandatory? Will not the driver work without them?
> >
> 
> To be honest I had not tried it yet. I've just tried and it works ok.
> 
> > > 
> > > I decided to explicitly specify the generic touchscreen properties here (instead of
> > > referring to touchscreen.txt) because not all properties listed in touchscreen.txt
> > > are actually parsed by the driver.
> > 
> > Fair enough.
> > 
> > Thanks.
> > 
> > > 
> > > > + - 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:
> > > > + - 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)
> > > > + - 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
> > > > + - use-handshake	: enable register-based handshake (boolean). This should
> > > > +			  only be used if the chip is configured to use 'blocking
> > > > +			  communication with timeout' (in this case the device
> > > > +			  generates an interrupt at the end of every
> > > > +			  scanning/processing cycle).
> > > > +
> > > > +[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 = <&gpio0>;
> > > > +			interrupts = <28 0>;
> > > > +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> > > > +
> > > > +			touchscreen-size-x = <800>;
> > > > +			touchscreen-size-y = <480>;
> > > > +			touchscreen-fuzz-x = <4>;
> > > > +			touchscreen-fuzz-y = <7>;
> > > > +
> > > > +			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 = <&gpio0>;
> > > > +			interrupts = <28 0>;
> > > > +			reset-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> > > > +
> > > > +			touchscreen-size-x = <800>;
> > > > +			touchscreen-size-y = <480>;
> > > > +			touchscreen-fuzz-x = <4>;
> > > > +			touchscreen-fuzz-y = <7>;
> > > > +
> > > > +			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>;
> > > > +		};
> > > > +
> > > > +		/* ... */
> > > > +	};
> > > > diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> > > > index 10379bc..92b459d 100644
> > > > --- a/drivers/input/touchscreen/cyttsp_core.c
> > > > +++ b/drivers/input/touchscreen/cyttsp_core.c
> > > > @@ -30,9 +30,12 @@
> > > >  #include <linux/delay.h>
> > > >  #include <linux/input.h>
> > > >  #include <linux/input/mt.h>
> > > > +#include <linux/input/touchscreen.h>
> > > >  #include <linux/gpio.h>
> > > >  #include <linux/interrupt.h>
> > > >  #include <linux/slab.h>
> > > > +#include <linux/property.h>
> > > > +#include <linux/gpio/consumer.h>
> > > >  
> > > >  #include "cyttsp_core.h"
> > > >  
> > > > @@ -57,6 +60,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
> > > > @@ -120,7 +124,7 @@ static int ttsp_send_command(struct cyttsp *ts, u8 cmd)
> > > >  
> > > >  static int cyttsp_handshake(struct cyttsp *ts)
> > > >  {
> > > > -	if (ts->pdata->use_hndshk)
> > > > +	if (ts->use_hndshk)
> > > >  		return ttsp_send_command(ts,
> > > >  				ts->xy_data.hst_mode ^ CY_HNDSHK_BIT);
> > > >  
> > > > @@ -142,9 +146,9 @@ static int cyttsp_exit_bl_mode(struct cyttsp *ts)
> > > >  	u8 bl_cmd[sizeof(bl_command)];
> > > >  
> > > >  	memcpy(bl_cmd, bl_command, sizeof(bl_command));
> > > > -	if (ts->pdata->bl_keys)
> > > > +	if (ts->bl_keys)
> > > >  		memcpy(&bl_cmd[sizeof(bl_command) - CY_NUM_BL_KEYS],
> > > > -			ts->pdata->bl_keys, CY_NUM_BL_KEYS);
> > > > +			ts->bl_keys, CY_NUM_BL_KEYS);
> > > >  
> > > >  	error = ttsp_write_block_data(ts, CY_REG_BASE,
> > > >  				      sizeof(bl_cmd), bl_cmd);
> > > > @@ -217,14 +221,14 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
> > > >  {
> > > >  	int retval = 0;
> > > >  
> > > > -	if (ts->pdata->act_intrvl != CY_ACT_INTRVL_DFLT ||
> > > > -	    ts->pdata->tch_tmout != CY_TCH_TMOUT_DFLT ||
> > > > -	    ts->pdata->lp_intrvl != CY_LP_INTRVL_DFLT) {
> > > > +	if (ts->act_intrvl != CY_ACT_INTRVL_DFLT ||
> > > > +	    ts->tch_tmout != CY_TCH_TMOUT_DFLT ||
> > > > +	    ts->lp_intrvl != CY_LP_INTRVL_DFLT) {
> > > >  
> > > >  		u8 intrvl_ray[] = {
> > > > -			ts->pdata->act_intrvl,
> > > > -			ts->pdata->tch_tmout,
> > > > -			ts->pdata->lp_intrvl
> > > > +			ts->act_intrvl,
> > > > +			ts->tch_tmout,
> > > > +			ts->lp_intrvl
> > > >  		};
> > > >  
> > > >  		/* set intrvl registers */
> > > > @@ -263,7 +267,7 @@ out:
> > > >  
> > > >  static int cyttsp_act_dist_setup(struct cyttsp *ts)
> > > >  {
> > > > -	u8 act_dist_setup = ts->pdata->act_dist;
> > > > +	u8 act_dist_setup = ts->act_dist;
> > > >  
> > > >  	/* Init gesture; active distance setup */
> > > >  	return ttsp_write_block_data(ts, CY_REG_ACT_DIST,
> > > > @@ -528,45 +532,107 @@ static void cyttsp_close(struct input_dev *dev)
> > > >  		cyttsp_disable(ts);
> > > >  }
> > > >  
> > > > +static int cyttsp_parse_properties(struct cyttsp *ts)
> > > > +{
> > > > +	struct device *dev = ts->dev;
> > > > +	u32 dt_value;
> > > > +	int ret;
> > > > +
> > > > +	ts->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
> > > > +	if (!ts->bl_keys)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	/* Set some default values */
> > > > +	ts->use_hndshk = false;
> > > > +	ts->act_dist = CY_ACT_DIST_DFLT;
> > > > +	ts->act_intrvl = CY_ACT_INTRVL_DFLT;
> > > > +	ts->tch_tmout = CY_TCH_TMOUT_DFLT;
> > > > +	ts->lp_intrvl = CY_LP_INTRVL_DFLT;
> > > > +
> > > > +	ret = device_property_read_u8_array(dev, "bootloader-key",
> > > > +					    ts->bl_keys, CY_NUM_BL_KEYS);
> > > > +	if (ret) {
> > > > +		dev_err(dev,
> > > > +			"bootloader-key property could not be retrieved\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	ts->use_hndshk = device_property_present(dev, "use-handshake");
> > > > +
> > > > +	if (!device_property_read_u32(dev, "active-distance", &dt_value)) {
> > > > +		if (dt_value > 15) {
> > > > +			dev_err(dev, "active-distance (%u) must be [0-15]\n",
> > > > +				dt_value);
> > > > +			return -EINVAL;
> > > > +		}
> > > > +		ts->act_dist &= ~CY_ACT_DIST_MASK;
> > > > +		ts->act_dist |= dt_value;
> > > > +	}
> > > > +
> > > > +	if (!device_property_read_u32(dev, "active-interval-ms", &dt_value)) {
> > > > +		if (dt_value > 255) {
> > > > +			dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
> > > > +				dt_value);
> > > > +			return -EINVAL;
> > > > +		}
> > > > +		ts->act_intrvl = dt_value;
> > > > +	}
> > > > +
> > > > +	if (!device_property_read_u32(dev, "lowpower-interval-ms", &dt_value)) {
> > > > +		if (dt_value > 2550) {
> > > > +			dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
> > > > +				dt_value);
> > > > +			return -EINVAL;
> > > > +		}
> > > > +		/* Register value is expressed in 0.01s / bit */
> > > > +		ts->lp_intrvl = dt_value / 10;
> > > > +	}
> > > > +
> > > > +	if (!device_property_read_u32(dev, "touch-timeout-ms", &dt_value)) {
> > > > +		if (dt_value > 2550) {
> > > > +			dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
> > > > +				dt_value);
> > > > +			return -EINVAL;
> > > > +		}
> > > > +		/* Register value is expressed in 0.01s / bit */
> > > > +		ts->tch_tmout = dt_value/10;
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  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);
> > > >  	struct cyttsp *ts;
> > > >  	struct input_dev *input_dev;
> > > >  	int error;
> > > >  
> > > > -	if (!pdata || !pdata->name || irq <= 0) {
> > > > -		error = -EINVAL;
> > > > -		goto err_out;
> > > > -	}
> > > > -
> > > >  	ts = devm_kzalloc(dev, sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
> > > >  	input_dev = devm_input_allocate_device(dev);
> > > > -	if (!ts || !input_dev) {
> > > > -		error = -ENOMEM;
> > > > -		goto err_out;
> > > > -	}
> > > > +	if (!ts || !input_dev)
> > > > +		return ERR_PTR(-ENOMEM);
> > > >  
> > > >  	ts->dev = dev;
> > > >  	ts->input = input_dev;
> > > > -	ts->pdata = dev_get_platdata(dev);
> > > >  	ts->bus_ops = bus_ops;
> > > >  	ts->irq = irq;
> > > >  
> > > > +	ts->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > > +	if (IS_ERR(ts->reset_gpio)) {
> > > > +		error = PTR_ERR(ts->reset_gpio);
> > > > +		dev_err(dev, "Failed to request reset gpio, error %d\n", error);
> > > > +		return ERR_PTR(error);
> > > > +	}
> > > > +
> > > > +	error = cyttsp_parse_properties(ts);
> > > > +	if (error)
> > > > +		return ERR_PTR(error);
> > > > +
> > > >  	init_completion(&ts->bl_ready);
> > > >  	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
> > > >  
> > > > -	if (pdata->init) {
> > > > -		error = pdata->init();
> > > > -		if (error) {
> > > > -			dev_err(ts->dev, "platform init failed, err: %d\n",
> > > > -				error);
> > > > -			goto err_out;
> > > > -		}
> > > > -	}
> > > > -
> > > > -	input_dev->name = pdata->name;
> > > > +	input_dev->name = "cyttsp";
> > 
> > Is there a friendlier name we could use? It is exported in
> > /proc/bus/input/devices and usually is more descriptive.
> >
> 
> Something like "Cypress TTSP touchscreen"? Same naming used in the Kconfig.

Sounds good to me.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-01-21 21:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-21 19:21 [PATCH v4 0/4] Add device tree support to the cyttsp driver Oreste Salerno
2016-01-21 19:21 ` [PATCH v4 1/4] Input: cyttsp - use devres managed resource allocations Oreste Salerno
     [not found]   ` <20e9f0188f07fc420912b3defa747303be3cf290.1453403916.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
2016-01-21 19:29     ` Dmitry Torokhov
2016-01-21 20:22       ` Oreste Salerno
2016-01-21 21:32         ` Dmitry Torokhov
2016-01-21 19:21 ` [PATCH v4 2/4] Input: cyttsp - check return value of input_mt_init_slots Oreste Salerno
2016-01-21 19:21 ` [PATCH v4 3/4] Input: cyttsp - switch to using device properties Oreste Salerno
     [not found]   ` <03a328d762d8ed654095d9a6edf648bc1c7fa262.1453403916.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
2016-01-21 19:25     ` Oreste Salerno
2016-01-21 19:35       ` Dmitry Torokhov
2016-01-21 20:44         ` Oreste Salerno
2016-01-21 21:33           ` Dmitry Torokhov
2016-01-21 19:21 ` [PATCH v4 4/4] Input: cyttsp - perform hard reset of the chip during probe Oreste Salerno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).