All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/3] Input: edt-ft5x06: Add DT support
@ 2014-01-16  8:02 Lothar Waßmann
  2014-01-16  8:02 ` [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate Lothar Waßmann
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Lothar Waßmann @ 2014-01-16  8:02 UTC (permalink / raw
  To: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov,
	Thierry Reding, Grant Likely, Jonathan Cameron, Shawn Guo,
	Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan,
	Sachin Kamat, devicetree, linux-doc, linux-kernel

Changes wrt. v1:
addressed the comments from Jingoo Han and Mark Rutland
- added another patch to convert the driver to use devm_* functions
- removed sysfs reference from bindings documentation
- changed '_' to '-' in property name
- added 'edt,' prefix to properties names
- added sanity check for parameters read from DT
- cleaned up the gpio handling code


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

* [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate
  2014-01-16  8:02 [PATCHv2 0/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
@ 2014-01-16  8:02 ` Lothar Waßmann
  2014-01-17  0:28     ` Dmitry Torokhov
  2014-01-16  8:02 ` [PATCHv2 2/3] DT: Add vendor prefix for Emerging Display Technologies Lothar Waßmann
  2014-01-16  8:02 ` [PATCHv2 3/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
  2 siblings, 1 reply; 10+ messages in thread
From: Lothar Waßmann @ 2014-01-16  8:02 UTC (permalink / raw
  To: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov,
	Thierry Reding, Grant Likely, Jonathan Cameron, Shawn Guo,
	Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan,
	Sachin Kamat, devicetree, linux-doc, linux-kernel
  Cc: Lothar Waßmann

Simplify the error path and remove() function by using devm_*
functions for requesting gpios and irq and allocating the input
device.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/input/touchscreen/edt-ft5x06.c |   62 ++++++++++---------------------
 1 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index af0d68b..acb6b9f 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -623,8 +623,9 @@ static int edt_ft5x06_ts_reset(struct i2c_client *client,
 
 	if (gpio_is_valid(reset_pin)) {
 		/* this pulls reset down, enabling the low active reset */
-		error = gpio_request_one(reset_pin, GPIOF_OUT_INIT_LOW,
-					 "edt-ft5x06 reset");
+		error = devm_gpio_request_one(&client->dev, reset_pin,
+					GPIOF_OUT_INIT_LOW,
+					"edt-ft5x06 reset");
 		if (error) {
 			dev_err(&client->dev,
 				"Failed to request GPIO %d as reset pin, error %d\n",
@@ -723,7 +724,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		return error;
 
 	if (gpio_is_valid(pdata->irq_pin)) {
-		error = gpio_request_one(pdata->irq_pin,
+		error = devm_gpio_request_one(&client->dev, pdata->irq_pin,
 					 GPIOF_IN, "edt-ft5x06 irq");
 		if (error) {
 			dev_err(&client->dev,
@@ -734,11 +735,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 	}
 
 	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
-	input = input_allocate_device();
-	if (!tsdata || !input) {
-		dev_err(&client->dev, "failed to allocate driver data.\n");
-		error = -ENOMEM;
-		goto err_free_mem;
+	input = devm_input_allocate_device(&client->dev);
+	if (!input) {
+		dev_err(&client->dev, "failed to allocate input device.\n");
+		return -ENOMEM;
 	}
 
 	mutex_init(&tsdata->mutex);
@@ -749,7 +749,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 	error = edt_ft5x06_ts_identify(client, tsdata->name, fw_version);
 	if (error) {
 		dev_err(&client->dev, "touchscreen probe failed\n");
-		goto err_free_mem;
+		return error;
 	}
 
 	edt_ft5x06_ts_get_defaults(tsdata, pdata);
@@ -776,27 +776,30 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 	error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
 	if (error) {
 		dev_err(&client->dev, "Unable to init MT slots.\n");
-		goto err_free_mem;
+		return error;
 	}
 
 	input_set_drvdata(input, tsdata);
 	i2c_set_clientdata(client, tsdata);
 
-	error = request_threaded_irq(client->irq, NULL, edt_ft5x06_ts_isr,
-				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-				     client->name, tsdata);
+	error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+					edt_ft5x06_ts_isr,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					client->name, tsdata);
 	if (error) {
 		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
-		goto err_free_mem;
+		return error;
 	}
 
 	error = sysfs_create_group(&client->dev.kobj, &edt_ft5x06_attr_group);
 	if (error)
-		goto err_free_irq;
+		return error;
 
 	error = input_register_device(input);
-	if (error)
-		goto err_remove_attrs;
+	if (error) {
+		sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
+		return error;
+	}
 
 	edt_ft5x06_ts_prepare_debugfs(tsdata, dev_driver_string(&client->dev));
 	device_init_wakeup(&client->dev, 1);
@@ -806,40 +809,15 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		pdata->irq_pin, pdata->reset_pin);
 
 	return 0;
-
-err_remove_attrs:
-	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
-err_free_irq:
-	free_irq(client->irq, tsdata);
-err_free_mem:
-	input_free_device(input);
-	kfree(tsdata);
-
-	if (gpio_is_valid(pdata->irq_pin))
-		gpio_free(pdata->irq_pin);
-
-	return error;
 }
 
 static int edt_ft5x06_ts_remove(struct i2c_client *client)
 {
-	const struct edt_ft5x06_platform_data *pdata =
-						dev_get_platdata(&client->dev);
 	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
 
 	edt_ft5x06_ts_teardown_debugfs(tsdata);
 	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
 
-	free_irq(client->irq, tsdata);
-	input_unregister_device(tsdata->input);
-
-	if (gpio_is_valid(pdata->irq_pin))
-		gpio_free(pdata->irq_pin);
-	if (gpio_is_valid(pdata->reset_pin))
-		gpio_free(pdata->reset_pin);
-
-	kfree(tsdata);
-
 	return 0;
 }
 
-- 
1.7.2.5


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

* [PATCHv2 2/3] DT: Add vendor prefix for Emerging Display Technologies
  2014-01-16  8:02 [PATCHv2 0/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
  2014-01-16  8:02 ` [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate Lothar Waßmann
@ 2014-01-16  8:02 ` Lothar Waßmann
  2014-01-16  8:02 ` [PATCHv2 3/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
  2 siblings, 0 replies; 10+ messages in thread
From: Lothar Waßmann @ 2014-01-16  8:02 UTC (permalink / raw
  To: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov,
	Thierry Reding, Grant Likely, Jonathan Cameron, Shawn Guo,
	Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan,
	Sachin Kamat, devicetree, linux-doc, linux-kernel
  Cc: Lothar Waßmann


Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b458760..49774c3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -29,6 +29,7 @@ dallas	Maxim Integrated Products (formerly Dallas Semiconductor)
 davicom	DAVICOM Semiconductor, Inc.
 denx	Denx Software Engineering
 dmo	Data Modul AG
+edt	Emerging Display Technologies
 emmicro	EM Microelectronic
 epson	Seiko Epson Corp.
 est	ESTeem Wireless Modems
-- 
1.7.2.5


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

* [PATCHv2 3/3] Input: edt-ft5x06: Add DT support
  2014-01-16  8:02 [PATCHv2 0/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
  2014-01-16  8:02 ` [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate Lothar Waßmann
  2014-01-16  8:02 ` [PATCHv2 2/3] DT: Add vendor prefix for Emerging Display Technologies Lothar Waßmann
@ 2014-01-16  8:02 ` Lothar Waßmann
  2014-01-17  0:26     ` Dmitry Torokhov
  2 siblings, 1 reply; 10+ messages in thread
From: Lothar Waßmann @ 2014-01-16  8:02 UTC (permalink / raw
  To: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov,
	Thierry Reding, Grant Likely, Jonathan Cameron, Shawn Guo,
	Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan,
	Sachin Kamat, devicetree, linux-doc, linux-kernel
  Cc: Lothar Waßmann


Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |   29 +++++
 drivers/input/touchscreen/edt-ft5x06.c             |  121 +++++++++++++++++---
 2 files changed, 132 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
new file mode 100644
index 0000000..8d94cdc
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -0,0 +1,29 @@
+* EDT FT5x06 Multiple Touch Controller
+
+Required properties:
+- compatible: must be "edt,ft5x06"
+- reg: i2c slave address
+- interrupt-parent: the phandle for the interrupt controller
+- interrupts: touch controller interrupt
+
+Optional properties:
+- reset-gpios: the gpio pin to be used for resetting the controller
+- wake-gpios:  the gpio pin to be used for waking up the controller
+
+  The following properties provide default values for the
+  corresponding parameters (see Documentation/input/edt-ft5x06.txt)
+- edt,threshold: allows setting the "click"-threshold in the range from 20 to 80.
+- edt,gain: sensitivity (0..31) (lower value -> higher sensitivity)
+- edt,offset: edge compensation (0..31)
+- edt,report-rate: report rate (3..14)
+
+Example:
+
+	edt_ft5x06@38 {
+		compatible = "edt,ft5x06";
+		reg = <0x38>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <5 0>;
+		reset-gpios = <&gpio2 6 1>;
+		wake-gpios = <&gpio4 9 0>;
+	};
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index acb6b9f..0467591 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -33,6 +33,7 @@
 #include <linux/debugfs.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 #include <linux/input/mt.h>
 #include <linux/input/edt-ft5x06.h>
 
@@ -65,6 +66,10 @@ struct edt_ft5x06_ts_data {
 	u16 num_x;
 	u16 num_y;
 
+	int reset_pin;
+	int irq_pin;
+	int wake_pin;
+
 #if defined(CONFIG_DEBUG_FS)
 	struct dentry *debug_dir;
 	u8 *raw_buffer;
@@ -617,25 +622,38 @@ edt_ft5x06_ts_teardown_debugfs(struct edt_ft5x06_ts_data *tsdata)
 
 
 static int edt_ft5x06_ts_reset(struct i2c_client *client,
-					 int reset_pin)
+			struct edt_ft5x06_ts_data *tsdata)
 {
 	int error;
 
-	if (gpio_is_valid(reset_pin)) {
+	if (gpio_is_valid(tsdata->wake_pin)) {
+		error = devm_gpio_request_one(&client->dev, tsdata->wake_pin,
+					GPIOF_OUT_INIT_LOW, "edt-ft5x06 wake");
+		if (error) {
+			dev_err(&client->dev,
+				"Failed to request GPIO %d as wake pin, error %d\n",
+				tsdata->wake_pin, error);
+			return error;
+		}
+
+		mdelay(5);
+		gpio_set_value(tsdata->wake_pin, 1);
+	}
+	if (gpio_is_valid(tsdata->reset_pin)) {
 		/* this pulls reset down, enabling the low active reset */
-		error = devm_gpio_request_one(&client->dev, reset_pin,
+		error = devm_gpio_request_one(&client->dev, tsdata->reset_pin,
 					GPIOF_OUT_INIT_LOW,
 					"edt-ft5x06 reset");
 		if (error) {
 			dev_err(&client->dev,
 				"Failed to request GPIO %d as reset pin, error %d\n",
-				reset_pin, error);
+				tsdata->reset_pin, error);
 			return error;
 		}
 
-		mdelay(50);
-		gpio_set_value(reset_pin, 1);
-		mdelay(100);
+		mdelay(5);
+		gpio_set_value(tsdata->reset_pin, 1);
+		mdelay(300);
 	}
 
 	return 0;
@@ -675,6 +693,29 @@ static int edt_ft5x06_ts_identify(struct i2c_client *client,
 	    pdata->name <= edt_ft5x06_attr_##name.limit_high)		\
 		edt_ft5x06_register_write(tsdata, reg, pdata->name)
 
+#define EDT_GET_PROP(name, var, reg) {					\
+	u32 val;							\
+	if (of_property_read_u32(np, name, &val) == 0) {		\
+		if (val >= edt_ft5x06_attr_##var.limit_low &&		\
+			val <= edt_ft5x06_attr_##var.limit_high)	\
+			edt_ft5x06_register_write(tsdata, reg, val);	\
+		else							\
+			pr_err("edt_ft5x06: property %s (%u) is out of range: %u..%u", \
+				name, val,  edt_ft5x06_attr_##var.limit_low, \
+				edt_ft5x06_attr_##var.limit_high);	\
+	}								\
+}
+
+static void edt_ft5x06_ts_get_dt_defaults(struct device_node *np,
+					struct edt_ft5x06_ts_data *tsdata)
+{
+	/* pick up defaults from the DT data */
+	EDT_GET_PROP("edt,threshold", threshold, WORK_REGISTER_THRESHOLD);
+	EDT_GET_PROP("edt,gain", gain, WORK_REGISTER_GAIN);
+	EDT_GET_PROP("edt,offset", offset, WORK_REGISTER_OFFSET);
+	EDT_GET_PROP("edt,report-rate", report_rate, WORK_REGISTER_REPORT_RATE);
+}
+
 static void
 edt_ft5x06_ts_get_defaults(struct edt_ft5x06_ts_data *tsdata,
 			   const struct edt_ft5x06_platform_data *pdata)
@@ -702,6 +743,33 @@ edt_ft5x06_ts_get_parameters(struct edt_ft5x06_ts_data *tsdata)
 	tsdata->num_y = edt_ft5x06_register_read(tsdata, WORK_REGISTER_NUM_Y);
 }
 
+#ifdef CONFIG_OF
+static int edt_ft5x06_i2c_ts_probe_dt(struct device *dev,
+				struct edt_ft5x06_ts_data *tsdata)
+{
+	struct device_node *np = dev->of_node;
+
+	if (!np)
+		return -ENODEV;
+
+	/*
+	 * irq_pin is not needed for DT setup.
+	 * irq is associated via 'interrupts' property in DT
+	 */
+	tsdata->irq_pin = -EINVAL;
+	tsdata->reset_pin = of_get_named_gpio(np, "reset-gpios", 0);
+	tsdata->wake_pin = of_get_named_gpio(np, "wake-gpios", 0);
+
+	return 0;
+}
+#else
+static inline int edt_ft5x06_i2c_ts_probe_dt(struct device *dev,
+					struct edt_ft5x06_i2c_ts_data *tsdata)
+{
+	return -ENODEV;
+}
+#endif
+
 static int edt_ft5x06_ts_probe(struct i2c_client *client,
 					 const struct i2c_device_id *id)
 {
@@ -714,27 +782,40 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 
 	dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
 
+	tsdata = devm_kzalloc(&client->dev, sizeof(*tsdata), GFP_KERNEL);
+	if (!tsdata) {
+		dev_err(&client->dev, "failed to allocate driver data.\n");
+		return -ENOMEM;
+	}
+
 	if (!pdata) {
-		dev_err(&client->dev, "no platform data?\n");
-		return -EINVAL;
+		error = edt_ft5x06_i2c_ts_probe_dt(&client->dev, tsdata);
+		if (error) {
+			dev_err(&client->dev,
+				"DT probe failed and no platform data present\n");
+			return error;
+		}
+	} else {
+		tsdata->reset_pin = pdata->reset_pin;
+		tsdata->irq_pin = pdata->irq_pin;
+		tsdata->wake_pin = -EINVAL;
 	}
 
-	error = edt_ft5x06_ts_reset(client, pdata->reset_pin);
+	error = edt_ft5x06_ts_reset(client, tsdata);
 	if (error)
 		return error;
 
-	if (gpio_is_valid(pdata->irq_pin)) {
-		error = devm_gpio_request_one(&client->dev, pdata->irq_pin,
-					 GPIOF_IN, "edt-ft5x06 irq");
+	if (gpio_is_valid(tsdata->irq_pin)) {
+		error = devm_gpio_request_one(&client->dev, tsdata->irq_pin,
+					GPIOF_IN, "edt-ft5x06 irq");
 		if (error) {
 			dev_err(&client->dev,
 				"Failed to request GPIO %d, error %d\n",
-				pdata->irq_pin, error);
+				tsdata->irq_pin, error);
 			return error;
 		}
 	}
 
-	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
 	input = devm_input_allocate_device(&client->dev);
 	if (!input) {
 		dev_err(&client->dev, "failed to allocate input device.\n");
@@ -752,7 +833,11 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		return error;
 	}
 
-	edt_ft5x06_ts_get_defaults(tsdata, pdata);
+	if (!pdata)
+		edt_ft5x06_ts_get_dt_defaults(client->dev.of_node, tsdata);
+	else
+		edt_ft5x06_ts_get_defaults(tsdata, pdata);
+
 	edt_ft5x06_ts_get_parameters(tsdata);
 
 	dev_dbg(&client->dev,
@@ -805,8 +890,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 	device_init_wakeup(&client->dev, 1);
 
 	dev_dbg(&client->dev,
-		"EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
-		pdata->irq_pin, pdata->reset_pin);
+		"EDT FT5x06 initialized: IRQ %d, WAKE pin %d, Reset pin %d.\n",
+		client->irq, tsdata->wake_pin, tsdata->reset_pin);
 
 	return 0;
 }
-- 
1.7.2.5


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

* Re: [PATCHv2 3/3] Input: edt-ft5x06: Add DT support
  2014-01-16  8:02 ` [PATCHv2 3/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
@ 2014-01-17  0:26     ` Dmitry Torokhov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2014-01-17  0:26 UTC (permalink / raw
  To: Lothar Waßmann
  Cc: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Thierry Reding,
	Grant Likely, Jonathan Cameron, Shawn Guo, Silvio F,
	Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat,
	devicetree, linux-doc, linux-kernel

Hi Lothar,

On Thu, Jan 16, 2014 at 09:02:18AM +0100, Lothar Waßmann wrote:
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  .../bindings/input/touchscreen/edt-ft5x06.txt      |   29 +++++
>  drivers/input/touchscreen/edt-ft5x06.c             |  121 +++++++++++++++++---
>  2 files changed, 132 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> new file mode 100644
> index 0000000..8d94cdc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> @@ -0,0 +1,29 @@
> +* EDT FT5x06 Multiple Touch Controller
> +
> +Required properties:
> +- compatible: must be "edt,ft5x06"
> +- reg: i2c slave address
> +- interrupt-parent: the phandle for the interrupt controller
> +- interrupts: touch controller interrupt
> +
> +Optional properties:
> +- reset-gpios: the gpio pin to be used for resetting the controller
> +- wake-gpios:  the gpio pin to be used for waking up the controller
> +
> +  The following properties provide default values for the
> +  corresponding parameters (see Documentation/input/edt-ft5x06.txt)
> +- edt,threshold: allows setting the "click"-threshold in the range from 20 to 80.
> +- edt,gain: sensitivity (0..31) (lower value -> higher sensitivity)
> +- edt,offset: edge compensation (0..31)
> +- edt,report-rate: report rate (3..14)

I wonder if we really need to have it in device tree? Can users needing
top tweak the settings do it via udev rules?

> +
> +Example:
> +
> +	edt_ft5x06@38 {
> +		compatible = "edt,ft5x06";
> +		reg = <0x38>;
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <5 0>;
> +		reset-gpios = <&gpio2 6 1>;
> +		wake-gpios = <&gpio4 9 0>;
> +	};
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index acb6b9f..0467591 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -33,6 +33,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/slab.h>
>  #include <linux/gpio.h>
> +#include <linux/of_gpio.h>
>  #include <linux/input/mt.h>
>  #include <linux/input/edt-ft5x06.h>
>  
> @@ -65,6 +66,10 @@ struct edt_ft5x06_ts_data {
>  	u16 num_x;
>  	u16 num_y;
>  
> +	int reset_pin;
> +	int irq_pin;
> +	int wake_pin;
> +
>  #if defined(CONFIG_DEBUG_FS)
>  	struct dentry *debug_dir;
>  	u8 *raw_buffer;
> @@ -617,25 +622,38 @@ edt_ft5x06_ts_teardown_debugfs(struct edt_ft5x06_ts_data *tsdata)
>  
>  
>  static int edt_ft5x06_ts_reset(struct i2c_client *client,
> -					 int reset_pin)
> +			struct edt_ft5x06_ts_data *tsdata)
>  {
>  	int error;
>  
> -	if (gpio_is_valid(reset_pin)) {
> +	if (gpio_is_valid(tsdata->wake_pin)) {
> +		error = devm_gpio_request_one(&client->dev, tsdata->wake_pin,
> +					GPIOF_OUT_INIT_LOW, "edt-ft5x06 wake");
> +		if (error) {
> +			dev_err(&client->dev,
> +				"Failed to request GPIO %d as wake pin, error %d\n",
> +				tsdata->wake_pin, error);
> +			return error;
> +		}
> +
> +		mdelay(5);
> +		gpio_set_value(tsdata->wake_pin, 1);
> +	}
> +	if (gpio_is_valid(tsdata->reset_pin)) {
>  		/* this pulls reset down, enabling the low active reset */
> -		error = devm_gpio_request_one(&client->dev, reset_pin,
> +		error = devm_gpio_request_one(&client->dev, tsdata->reset_pin,
>  					GPIOF_OUT_INIT_LOW,
>  					"edt-ft5x06 reset");
>  		if (error) {
>  			dev_err(&client->dev,
>  				"Failed to request GPIO %d as reset pin, error %d\n",
> -				reset_pin, error);
> +				tsdata->reset_pin, error);
>  			return error;
>  		}
>  
> -		mdelay(50);
> -		gpio_set_value(reset_pin, 1);
> -		mdelay(100);
> +		mdelay(5);
> +		gpio_set_value(tsdata->reset_pin, 1);
> +		mdelay(300);

Hmm, this change seems unrelated to DT support.

Thanks.

-- 
Dmitry

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

* Re: [PATCHv2 3/3] Input: edt-ft5x06: Add DT support
@ 2014-01-17  0:26     ` Dmitry Torokhov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2014-01-17  0:26 UTC (permalink / raw
  To: Lothar Waßmann
  Cc: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Thierry Reding,
	Grant Likely, Jonathan Cameron, Shawn Guo, Silvio F,
	Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat,
	devicetree, linux-doc, linux-kernel

Hi Lothar,

On Thu, Jan 16, 2014 at 09:02:18AM +0100, Lothar Waßmann wrote:
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  .../bindings/input/touchscreen/edt-ft5x06.txt      |   29 +++++
>  drivers/input/touchscreen/edt-ft5x06.c             |  121 +++++++++++++++++---
>  2 files changed, 132 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> new file mode 100644
> index 0000000..8d94cdc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> @@ -0,0 +1,29 @@
> +* EDT FT5x06 Multiple Touch Controller
> +
> +Required properties:
> +- compatible: must be "edt,ft5x06"
> +- reg: i2c slave address
> +- interrupt-parent: the phandle for the interrupt controller
> +- interrupts: touch controller interrupt
> +
> +Optional properties:
> +- reset-gpios: the gpio pin to be used for resetting the controller
> +- wake-gpios:  the gpio pin to be used for waking up the controller
> +
> +  The following properties provide default values for the
> +  corresponding parameters (see Documentation/input/edt-ft5x06.txt)
> +- edt,threshold: allows setting the "click"-threshold in the range from 20 to 80.
> +- edt,gain: sensitivity (0..31) (lower value -> higher sensitivity)
> +- edt,offset: edge compensation (0..31)
> +- edt,report-rate: report rate (3..14)

I wonder if we really need to have it in device tree? Can users needing
top tweak the settings do it via udev rules?

> +
> +Example:
> +
> +	edt_ft5x06@38 {
> +		compatible = "edt,ft5x06";
> +		reg = <0x38>;
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <5 0>;
> +		reset-gpios = <&gpio2 6 1>;
> +		wake-gpios = <&gpio4 9 0>;
> +	};
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index acb6b9f..0467591 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -33,6 +33,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/slab.h>
>  #include <linux/gpio.h>
> +#include <linux/of_gpio.h>
>  #include <linux/input/mt.h>
>  #include <linux/input/edt-ft5x06.h>
>  
> @@ -65,6 +66,10 @@ struct edt_ft5x06_ts_data {
>  	u16 num_x;
>  	u16 num_y;
>  
> +	int reset_pin;
> +	int irq_pin;
> +	int wake_pin;
> +
>  #if defined(CONFIG_DEBUG_FS)
>  	struct dentry *debug_dir;
>  	u8 *raw_buffer;
> @@ -617,25 +622,38 @@ edt_ft5x06_ts_teardown_debugfs(struct edt_ft5x06_ts_data *tsdata)
>  
>  
>  static int edt_ft5x06_ts_reset(struct i2c_client *client,
> -					 int reset_pin)
> +			struct edt_ft5x06_ts_data *tsdata)
>  {
>  	int error;
>  
> -	if (gpio_is_valid(reset_pin)) {
> +	if (gpio_is_valid(tsdata->wake_pin)) {
> +		error = devm_gpio_request_one(&client->dev, tsdata->wake_pin,
> +					GPIOF_OUT_INIT_LOW, "edt-ft5x06 wake");
> +		if (error) {
> +			dev_err(&client->dev,
> +				"Failed to request GPIO %d as wake pin, error %d\n",
> +				tsdata->wake_pin, error);
> +			return error;
> +		}
> +
> +		mdelay(5);
> +		gpio_set_value(tsdata->wake_pin, 1);
> +	}
> +	if (gpio_is_valid(tsdata->reset_pin)) {
>  		/* this pulls reset down, enabling the low active reset */
> -		error = devm_gpio_request_one(&client->dev, reset_pin,
> +		error = devm_gpio_request_one(&client->dev, tsdata->reset_pin,
>  					GPIOF_OUT_INIT_LOW,
>  					"edt-ft5x06 reset");
>  		if (error) {
>  			dev_err(&client->dev,
>  				"Failed to request GPIO %d as reset pin, error %d\n",
> -				reset_pin, error);
> +				tsdata->reset_pin, error);
>  			return error;
>  		}
>  
> -		mdelay(50);
> -		gpio_set_value(reset_pin, 1);
> -		mdelay(100);
> +		mdelay(5);
> +		gpio_set_value(tsdata->reset_pin, 1);
> +		mdelay(300);

Hmm, this change seems unrelated to DT support.

Thanks.

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

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

* Re: [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate
  2014-01-16  8:02 ` [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate Lothar Waßmann
@ 2014-01-17  0:28     ` Dmitry Torokhov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2014-01-17  0:28 UTC (permalink / raw
  To: Lothar Waßmann
  Cc: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Thierry Reding,
	Grant Likely, Jonathan Cameron, Shawn Guo, Silvio F,
	Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat,
	devicetree, linux-doc, linux-kernel

On Thu, Jan 16, 2014 at 09:02:16AM +0100, Lothar Waßmann wrote:
> Simplify the error path and remove() function by using devm_*
> functions for requesting gpios and irq and allocating the input
> device.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>

Applied, thank you.

> ---
>  drivers/input/touchscreen/edt-ft5x06.c |   62 ++++++++++---------------------
>  1 files changed, 20 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index af0d68b..acb6b9f 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -623,8 +623,9 @@ static int edt_ft5x06_ts_reset(struct i2c_client *client,
>  
>  	if (gpio_is_valid(reset_pin)) {
>  		/* this pulls reset down, enabling the low active reset */
> -		error = gpio_request_one(reset_pin, GPIOF_OUT_INIT_LOW,
> -					 "edt-ft5x06 reset");
> +		error = devm_gpio_request_one(&client->dev, reset_pin,
> +					GPIOF_OUT_INIT_LOW,
> +					"edt-ft5x06 reset");
>  		if (error) {
>  			dev_err(&client->dev,
>  				"Failed to request GPIO %d as reset pin, error %d\n",
> @@ -723,7 +724,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  		return error;
>  
>  	if (gpio_is_valid(pdata->irq_pin)) {
> -		error = gpio_request_one(pdata->irq_pin,
> +		error = devm_gpio_request_one(&client->dev, pdata->irq_pin,
>  					 GPIOF_IN, "edt-ft5x06 irq");
>  		if (error) {
>  			dev_err(&client->dev,
> @@ -734,11 +735,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  	}
>  
>  	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
> -	input = input_allocate_device();
> -	if (!tsdata || !input) {
> -		dev_err(&client->dev, "failed to allocate driver data.\n");
> -		error = -ENOMEM;
> -		goto err_free_mem;
> +	input = devm_input_allocate_device(&client->dev);
> +	if (!input) {
> +		dev_err(&client->dev, "failed to allocate input device.\n");
> +		return -ENOMEM;
>  	}
>  
>  	mutex_init(&tsdata->mutex);
> @@ -749,7 +749,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  	error = edt_ft5x06_ts_identify(client, tsdata->name, fw_version);
>  	if (error) {
>  		dev_err(&client->dev, "touchscreen probe failed\n");
> -		goto err_free_mem;
> +		return error;
>  	}
>  
>  	edt_ft5x06_ts_get_defaults(tsdata, pdata);
> @@ -776,27 +776,30 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  	error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
>  	if (error) {
>  		dev_err(&client->dev, "Unable to init MT slots.\n");
> -		goto err_free_mem;
> +		return error;
>  	}
>  
>  	input_set_drvdata(input, tsdata);
>  	i2c_set_clientdata(client, tsdata);
>  
> -	error = request_threaded_irq(client->irq, NULL, edt_ft5x06_ts_isr,
> -				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -				     client->name, tsdata);
> +	error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> +					edt_ft5x06_ts_isr,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					client->name, tsdata);
>  	if (error) {
>  		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
> -		goto err_free_mem;
> +		return error;
>  	}
>  
>  	error = sysfs_create_group(&client->dev.kobj, &edt_ft5x06_attr_group);
>  	if (error)
> -		goto err_free_irq;
> +		return error;
>  
>  	error = input_register_device(input);
> -	if (error)
> -		goto err_remove_attrs;
> +	if (error) {
> +		sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
> +		return error;
> +	}
>  
>  	edt_ft5x06_ts_prepare_debugfs(tsdata, dev_driver_string(&client->dev));
>  	device_init_wakeup(&client->dev, 1);
> @@ -806,40 +809,15 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  		pdata->irq_pin, pdata->reset_pin);
>  
>  	return 0;
> -
> -err_remove_attrs:
> -	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
> -err_free_irq:
> -	free_irq(client->irq, tsdata);
> -err_free_mem:
> -	input_free_device(input);
> -	kfree(tsdata);
> -
> -	if (gpio_is_valid(pdata->irq_pin))
> -		gpio_free(pdata->irq_pin);
> -
> -	return error;
>  }
>  
>  static int edt_ft5x06_ts_remove(struct i2c_client *client)
>  {
> -	const struct edt_ft5x06_platform_data *pdata =
> -						dev_get_platdata(&client->dev);
>  	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
>  
>  	edt_ft5x06_ts_teardown_debugfs(tsdata);
>  	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
>  
> -	free_irq(client->irq, tsdata);
> -	input_unregister_device(tsdata->input);
> -
> -	if (gpio_is_valid(pdata->irq_pin))
> -		gpio_free(pdata->irq_pin);
> -	if (gpio_is_valid(pdata->reset_pin))
> -		gpio_free(pdata->reset_pin);
> -
> -	kfree(tsdata);
> -
>  	return 0;
>  }
>  
> -- 
> 1.7.2.5
> 

-- 
Dmitry

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

* Re: [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate
@ 2014-01-17  0:28     ` Dmitry Torokhov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2014-01-17  0:28 UTC (permalink / raw
  To: Lothar Waßmann
  Cc: linux-input, Simon Budig, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Rob Landley, Thierry Reding,
	Grant Likely, Jonathan Cameron, Shawn Guo, Silvio F,
	Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat,
	devicetree, linux-doc, linux-kernel

On Thu, Jan 16, 2014 at 09:02:16AM +0100, Lothar Waßmann wrote:
> Simplify the error path and remove() function by using devm_*
> functions for requesting gpios and irq and allocating the input
> device.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>

Applied, thank you.

> ---
>  drivers/input/touchscreen/edt-ft5x06.c |   62 ++++++++++---------------------
>  1 files changed, 20 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index af0d68b..acb6b9f 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -623,8 +623,9 @@ static int edt_ft5x06_ts_reset(struct i2c_client *client,
>  
>  	if (gpio_is_valid(reset_pin)) {
>  		/* this pulls reset down, enabling the low active reset */
> -		error = gpio_request_one(reset_pin, GPIOF_OUT_INIT_LOW,
> -					 "edt-ft5x06 reset");
> +		error = devm_gpio_request_one(&client->dev, reset_pin,
> +					GPIOF_OUT_INIT_LOW,
> +					"edt-ft5x06 reset");
>  		if (error) {
>  			dev_err(&client->dev,
>  				"Failed to request GPIO %d as reset pin, error %d\n",
> @@ -723,7 +724,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  		return error;
>  
>  	if (gpio_is_valid(pdata->irq_pin)) {
> -		error = gpio_request_one(pdata->irq_pin,
> +		error = devm_gpio_request_one(&client->dev, pdata->irq_pin,
>  					 GPIOF_IN, "edt-ft5x06 irq");
>  		if (error) {
>  			dev_err(&client->dev,
> @@ -734,11 +735,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  	}
>  
>  	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
> -	input = input_allocate_device();
> -	if (!tsdata || !input) {
> -		dev_err(&client->dev, "failed to allocate driver data.\n");
> -		error = -ENOMEM;
> -		goto err_free_mem;
> +	input = devm_input_allocate_device(&client->dev);
> +	if (!input) {
> +		dev_err(&client->dev, "failed to allocate input device.\n");
> +		return -ENOMEM;
>  	}
>  
>  	mutex_init(&tsdata->mutex);
> @@ -749,7 +749,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  	error = edt_ft5x06_ts_identify(client, tsdata->name, fw_version);
>  	if (error) {
>  		dev_err(&client->dev, "touchscreen probe failed\n");
> -		goto err_free_mem;
> +		return error;
>  	}
>  
>  	edt_ft5x06_ts_get_defaults(tsdata, pdata);
> @@ -776,27 +776,30 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  	error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
>  	if (error) {
>  		dev_err(&client->dev, "Unable to init MT slots.\n");
> -		goto err_free_mem;
> +		return error;
>  	}
>  
>  	input_set_drvdata(input, tsdata);
>  	i2c_set_clientdata(client, tsdata);
>  
> -	error = request_threaded_irq(client->irq, NULL, edt_ft5x06_ts_isr,
> -				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -				     client->name, tsdata);
> +	error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> +					edt_ft5x06_ts_isr,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					client->name, tsdata);
>  	if (error) {
>  		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
> -		goto err_free_mem;
> +		return error;
>  	}
>  
>  	error = sysfs_create_group(&client->dev.kobj, &edt_ft5x06_attr_group);
>  	if (error)
> -		goto err_free_irq;
> +		return error;
>  
>  	error = input_register_device(input);
> -	if (error)
> -		goto err_remove_attrs;
> +	if (error) {
> +		sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
> +		return error;
> +	}
>  
>  	edt_ft5x06_ts_prepare_debugfs(tsdata, dev_driver_string(&client->dev));
>  	device_init_wakeup(&client->dev, 1);
> @@ -806,40 +809,15 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  		pdata->irq_pin, pdata->reset_pin);
>  
>  	return 0;
> -
> -err_remove_attrs:
> -	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
> -err_free_irq:
> -	free_irq(client->irq, tsdata);
> -err_free_mem:
> -	input_free_device(input);
> -	kfree(tsdata);
> -
> -	if (gpio_is_valid(pdata->irq_pin))
> -		gpio_free(pdata->irq_pin);
> -
> -	return error;
>  }
>  
>  static int edt_ft5x06_ts_remove(struct i2c_client *client)
>  {
> -	const struct edt_ft5x06_platform_data *pdata =
> -						dev_get_platdata(&client->dev);
>  	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
>  
>  	edt_ft5x06_ts_teardown_debugfs(tsdata);
>  	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
>  
> -	free_irq(client->irq, tsdata);
> -	input_unregister_device(tsdata->input);
> -
> -	if (gpio_is_valid(pdata->irq_pin))
> -		gpio_free(pdata->irq_pin);
> -	if (gpio_is_valid(pdata->reset_pin))
> -		gpio_free(pdata->reset_pin);
> -
> -	kfree(tsdata);
> -
>  	return 0;
>  }
>  
> -- 
> 1.7.2.5
> 

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

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

* Re: [PATCHv2 3/3] Input: edt-ft5x06: Add DT support
  2014-01-17  0:26     ` Dmitry Torokhov
  (?)
@ 2014-01-17  1:36     ` Jingoo Han
  -1 siblings, 0 replies; 10+ messages in thread
From: Jingoo Han @ 2014-01-17  1:36 UTC (permalink / raw
  To: 'Lothar Waßmann', 'Dmitry Torokhov'
  Cc: linux-input, 'Simon Budig', 'Rob Herring',
	'Pawel Moll', 'Mark Rutland',
	'Ian Campbell', 'Kumar Gala',
	'Rob Landley', 'Thierry Reding',
	'Grant Likely', 'Jonathan Cameron',
	'Shawn Guo', 'Silvio F',
	'Guennadi Liakhovetski', 'Fugang Duan',
	'Sachin Kamat', devicetree, linux-doc, linux-kernel,
	'Jingoo Han'

On Friday, January 17, 2014 9:27 AM, Dmitry Torokhov wrote:
> On Thu, Jan 16, 2014 at 09:02:18AM +0100, Lothar Waßmann wrote:
> >
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> >  .../bindings/input/touchscreen/edt-ft5x06.txt      |   29 +++++
> >  drivers/input/touchscreen/edt-ft5x06.c             |  121 +++++++++++++++++---
> >  2 files changed, 132 insertions(+), 18 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> >

[.....]

> > +	if (gpio_is_valid(tsdata->reset_pin)) {
> >  		/* this pulls reset down, enabling the low active reset */
> > -		error = devm_gpio_request_one(&client->dev, reset_pin,
> > +		error = devm_gpio_request_one(&client->dev, tsdata->reset_pin,
> >  					GPIOF_OUT_INIT_LOW,
> >  					"edt-ft5x06 reset");
> >  		if (error) {
> >  			dev_err(&client->dev,
> >  				"Failed to request GPIO %d as reset pin, error %d\n",
> > -				reset_pin, error);
> > +				tsdata->reset_pin, error);
> >  			return error;
> >  		}
> >
> > -		mdelay(50);
> > -		gpio_set_value(reset_pin, 1);
> > -		mdelay(100);
> > +		mdelay(5);
> > +		gpio_set_value(tsdata->reset_pin, 1);
> > +		mdelay(300);
> 
> Hmm, this change seems unrelated to DT support.

Right, modifying delay timing is not related to DT support.
In this case, this patch should be split, and the patch should
address the reason why delay timing is modified.
Also, 300 msec is a huge delay. Thus, comment will be helpful.

Best regards,
Jingoo Han


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

* Re: [PATCHv2 3/3] Input: edt-ft5x06: Add DT support
  2014-01-17  0:26     ` Dmitry Torokhov
  (?)
  (?)
@ 2014-01-17  1:46     ` Simon Budig
  -1 siblings, 0 replies; 10+ messages in thread
From: Simon Budig @ 2014-01-17  1:46 UTC (permalink / raw
  To: Dmitry Torokhov
  Cc: Lothar Waßmann, linux-input, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley,
	Thierry Reding, Grant Likely, Jonathan Cameron, Shawn Guo,
	Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan,
	Sachin Kamat, devicetree, linux-doc, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

Hi Dmitry

On 17/01/14 01:26, Dmitry Torokhov wrote:
>> +  The following properties provide default values for the
>> +  corresponding parameters (see Documentation/input/edt-ft5x06.txt)
>> +- edt,threshold: allows setting the "click"-threshold in the range from 20 to 80.
>> +- edt,gain: sensitivity (0..31) (lower value -> higher sensitivity)
>> +- edt,offset: edge compensation (0..31)
>> +- edt,report-rate: report rate (3..14)
> 
> I wonder if we really need to have it in device tree? Can users needing
> top tweak the settings do it via udev rules?

IMO it makes sense to have these in the device tree. These values need
to be adjusted if you have glass or acrylics in front of the touch
glass. The defaults are tailored to the touchscreen without any
(additional) material in front of it.

So manufacturers need to provide different defaults depending on the way
a specific device is built. So these properties can be viewed in the
same way you specify display resolution or display type in the device tree.

Bye,
         Simon


-- 
       Simon Budig                        kernel concepts GmbH
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

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

end of thread, other threads:[~2014-01-17  1:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-16  8:02 [PATCHv2 0/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
2014-01-16  8:02 ` [PATCHv2 1/3] Input: edt_ft5x06: use devm_* functions where appropriate Lothar Waßmann
2014-01-17  0:28   ` Dmitry Torokhov
2014-01-17  0:28     ` Dmitry Torokhov
2014-01-16  8:02 ` [PATCHv2 2/3] DT: Add vendor prefix for Emerging Display Technologies Lothar Waßmann
2014-01-16  8:02 ` [PATCHv2 3/3] Input: edt-ft5x06: Add DT support Lothar Waßmann
2014-01-17  0:26   ` Dmitry Torokhov
2014-01-17  0:26     ` Dmitry Torokhov
2014-01-17  1:36     ` Jingoo Han
2014-01-17  1:46     ` Simon Budig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.