[PATCH 1/2] DT: Add vendor prefix for Emerging Display Technologies

Subsystems: open firmware and flattened device tree bindings, the rest

STALE4630d

6 messages, 4 authors, 2014-01-17 · open the first message on its own page

[PATCH 1/2] DT: Add vendor prefix for Emerging Display Technologies

From: Lothar Waßmann <hidden>
Date: 2014-01-13 10:19:18

Signed-off-by: Lothar Waßmann <redacted>
---
 .../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

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

[PATCH 2/2] Input: edt-ft5x06: Add DT support

From: Lothar Waßmann <hidden>
Date: 2014-01-13 10:20:11

This patch allows the edt-ft5x06 multitouch panel driver to be
configured via DT.

Signed-off-by: Lothar Waßmann <redacted>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |   31 ++++
 drivers/input/touchscreen/edt-ft5x06.c             |  145 +++++++++++++++----
 2 files changed, 145 insertions(+), 31 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..629dbdd
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -0,0 +1,31 @@
+* 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
+- wakeup-gpios: the gpio pin to be used for waking up the controller
+
+  The following properties provide default values for the
+  corresponding parameters configurable via sysfs
+  (see Documentation/input/edt-ft5x06.txt)
+- threshold: allows setting the "click"-threshold in the range from 20 to 80.
+- gain: sensitivity (0..31) (lower value -> higher sensitivity)
+- offset: edge compensation (0..31)
+- report_rate: report rate (3..14)
+
+Example:
+
+	edt_ft5x06@38 {
+		compatible = "edt,ft5x06";
+		reg = <0x38>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <5 0>;
+		wakeup-gpios = <&gpio1 9 0>;
+		reset-gpios = <&gpio2 6 1>;
+		wakeup-gpios = <&gpio4 9 0>;
+	};
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index af0d68b..02dce42 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,24 +622,37 @@ 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 = gpio_request_one(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 = gpio_request_one(reset_pin, GPIOF_OUT_INIT_LOW,
+		error = gpio_request_one(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;
@@ -674,6 +692,21 @@ 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, reg) {					\
+	const u32 *prop = of_get_property(np, #name, NULL);		\
+	if (prop)							\
+		edt_ft5x06_register_write(tsdata, reg, be32_to_cpu(*prop)); \
+}
+
+static void edt_ft5x06_ts_get_dt_defaults(struct device_node *np,
+					struct edt_ft5x06_ts_data *tsdata)
+{
+	EDT_GET_PROP(threshold, WORK_REGISTER_THRESHOLD);
+	EDT_GET_PROP(gain, WORK_REGISTER_GAIN);
+	EDT_GET_PROP(offset, WORK_REGISTER_OFFSET);
+	EDT_GET_PROP(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)
@@ -701,6 +734,39 @@ 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)
+{
+	int ret;
+	struct device_node *np = dev->of_node;
+	enum of_gpio_flags gpio_flags;
+
+	if (!np)
+		return -ENODEV;
+
+	/*
+	 * irq_pin is not needed for DT setup.
+	 * irq is associated via 'interrupts' property in DT
+	 */
+	tsdata->irq_pin = -EINVAL;
+
+	ret = of_get_named_gpio_flags(np, "reset-gpios", 0, &gpio_flags);
+	tsdata->reset_pin = ret;
+
+	ret = of_get_named_gpio_flags(np, "wake-gpios", 0, &gpio_flags);
+	tsdata->wake_pin = ret;
+
+	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)
 {
@@ -713,30 +779,43 @@ 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 = gpio_request_one(pdata->irq_pin,
+	if (gpio_is_valid(tsdata->irq_pin)) {
+		error = gpio_request_one(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 = input_allocate_device();
-	if (!tsdata || !input) {
-		dev_err(&client->dev, "failed to allocate driver data.\n");
+	if (!input) {
+		dev_err(&client->dev, "failed to allocate input device.\n");
 		error = -ENOMEM;
 		goto err_free_mem;
 	}
@@ -752,7 +831,11 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		goto err_free_mem;
 	}
 
-	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,
@@ -802,8 +885,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;
 
@@ -813,18 +896,18 @@ 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);
+	if (gpio_is_valid(tsdata->irq_pin))
+		gpio_free(tsdata->irq_pin);
+	if (gpio_is_valid(tsdata->reset_pin))
+		gpio_free(tsdata->reset_pin);
+	if (gpio_is_valid(tsdata->wake_pin))
+		gpio_free(tsdata->wake_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);
@@ -833,12 +916,12 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
 	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);
+	if (gpio_is_valid(tsdata->irq_pin))
+		gpio_free(tsdata->irq_pin);
+	if (gpio_is_valid(tsdata->reset_pin))
+		gpio_free(tsdata->reset_pin);
+	if (gpio_is_valid(tsdata->wake_pin))
+		gpio_free(tsdata->wake_pin);
 
 	return 0;
 }
-- 
1.7.2.5

Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support

From: Mark Rutland <mark.rutland@arm.com>
Date: 2014-01-13 13:44:46

On Mon, Jan 13, 2014 at 10:17:04AM +0000, Lothar Waßmann wrote:
quoted hunk
This patch allows the edt-ft5x06 multitouch panel driver to be
configured via DT.

Signed-off-by: Lothar Waßmann <redacted>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |   31 ++++
 drivers/input/touchscreen/edt-ft5x06.c             |  145 +++++++++++++++----
 2 files changed, 145 insertions(+), 31 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..629dbdd
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -0,0 +1,31 @@
+* 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
+- wakeup-gpios: the gpio pin to be used for waking up the controller
+
+  The following properties provide default values for the
+  corresponding parameters configurable via sysfs
+  (see Documentation/input/edt-ft5x06.txt)
The sysfs interface shouldn't matter for the binding, it's a Linux
detail. There's no reason for it to be mentioned in the binding.
+- threshold: allows setting the "click"-threshold in the range from 20 to 80.
+- gain: sensitivity (0..31) (lower value -> higher sensitivity)
+- offset: edge compensation (0..31)
+- report_rate: report rate (3..14)
s/_/-/ on property names please. Also, it may make sense to prefix these
as they're rather generic sounding names.

Could you elaborate on these a litle please? What units are each of
these in? Why does it make sense to have them in the dt?
+
+Example:
+
+	edt_ft5x06@38 {
+		compatible = "edt,ft5x06";
+		reg = <0x38>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <5 0>;
+		wakeup-gpios = <&gpio1 9 0>;
+		reset-gpios = <&gpio2 6 1>;
+		wakeup-gpios = <&gpio4 9 0>;
+	};
One of those wakeup-gpios properties should go.

[...]
+#define EDT_GET_PROP(name, reg) {					\
+	const u32 *prop = of_get_property(np, #name, NULL);		\
+	if (prop)							\
+		edt_ft5x06_register_write(tsdata, reg, be32_to_cpu(*prop)); \
+}
Use of_property_read_u32, which does endianness conversion and property
length checking (which you're not doing). Sparse _will_ complain here
because you've not maintained the __be32 annotation.
+static void edt_ft5x06_ts_get_dt_defaults(struct device_node *np,
+					struct edt_ft5x06_ts_data *tsdata)
+{
+	EDT_GET_PROP(threshold, WORK_REGISTER_THRESHOLD);
+	EDT_GET_PROP(gain, WORK_REGISTER_GAIN);
+	EDT_GET_PROP(offset, WORK_REGISTER_OFFSET);
+	EDT_GET_PROP(report_rate, WORK_REGISTER_REPORT_RATE);
These should probably have sanity checks given you've described valid
ranges in the documentation.

[...]
+static int edt_ft5x06_i2c_ts_probe_dt(struct device *dev,
+				struct edt_ft5x06_ts_data *tsdata)
+{
+	int ret;
+	struct device_node *np = dev->of_node;
+	enum of_gpio_flags gpio_flags;
+
+	if (!np)
+		return -ENODEV;
+
+	/*
+	 * irq_pin is not needed for DT setup.
+	 * irq is associated via 'interrupts' property in DT
+	 */
+	tsdata->irq_pin = -EINVAL;
+
+	ret = of_get_named_gpio_flags(np, "reset-gpios", 0, &gpio_flags);
+	tsdata->reset_pin = ret;
+
+	ret = of_get_named_gpio_flags(np, "wake-gpios", 0, &gpio_flags);
+	tsdata->wake_pin = ret;
Shouldn't that be "wakeup-gpios"?

Do you not need the value of flags? I'm not that familiar with the GPIO
subsystem, and it feels odd to rtequest the flags and throw them away.

Thanks,
Mark.

Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support

From: Jingoo Han <hidden>
Date: 2014-01-15 00:46:50

On Monday, January 13, 2014 7:17 PM, Lothar Waßmann wrote:
This patch allows the edt-ft5x06 multitouch panel driver to be
configured via DT.

Signed-off-by: Lothar Waßmann <redacted>
(+cc Simon Budig)

This 'edt-ft5x06' driver was made by Simon Budig.
Please, add him to CC list.

Also, I added some comments. :-)
quoted hunk
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |   31 ++++
 drivers/input/touchscreen/edt-ft5x06.c             |  145 +++++++++++++++----
 2 files changed, 145 insertions(+), 31 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..629dbdd
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -0,0 +1,31 @@
+* 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
+- wakeup-gpios: the gpio pin to be used for waking up the controller
+
+  The following properties provide default values for the
+  corresponding parameters configurable via sysfs
+  (see Documentation/input/edt-ft5x06.txt)
+- threshold: allows setting the "click"-threshold in the range from 20 to 80.
+- gain: sensitivity (0..31) (lower value -> higher sensitivity)
+- offset: edge compensation (0..31)
+- report_rate: report rate (3..14)
s/report_rate/report-rate
quoted hunk
+
+Example:
+
+	edt_ft5x06@38 {
+		compatible = "edt,ft5x06";
+		reg = <0x38>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <5 0>;
+		wakeup-gpios = <&gpio1 9 0>;
+		reset-gpios = <&gpio2 6 1>;
+		wakeup-gpios = <&gpio4 9 0>;
+	};
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index af0d68b..02dce42 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,24 +622,37 @@ 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 = gpio_request_one(tsdata->wake_pin, GPIOF_OUT_INIT_LOW,
+					 "edt-ft5x06 wake");
Please use devm_gpio_request_one(), because it makes the code
simpler.
+		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 = gpio_request_one(reset_pin, GPIOF_OUT_INIT_LOW,
+		error = gpio_request_one(tsdata->reset_pin, GPIOF_OUT_INIT_LOW,
 					 "edt-ft5x06 reset");
Please use devm_gpio_request_one() too.
quoted hunk
 		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;
@@ -674,6 +692,21 @@ 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, reg) {					\
+	const u32 *prop = of_get_property(np, #name, NULL);		\
+	if (prop)							\
+		edt_ft5x06_register_write(tsdata, reg, be32_to_cpu(*prop)); \
+}
+
+static void edt_ft5x06_ts_get_dt_defaults(struct device_node *np,
+					struct edt_ft5x06_ts_data *tsdata)
+{
+	EDT_GET_PROP(threshold, WORK_REGISTER_THRESHOLD);
+	EDT_GET_PROP(gain, WORK_REGISTER_GAIN);
+	EDT_GET_PROP(offset, WORK_REGISTER_OFFSET);
+	EDT_GET_PROP(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)
@@ -701,6 +734,39 @@ 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)
+{
+	int ret;
+	struct device_node *np = dev->of_node;
+	enum of_gpio_flags gpio_flags;
+
+	if (!np)
+		return -ENODEV;
+
+	/*
+	 * irq_pin is not needed for DT setup.
+	 * irq is associated via 'interrupts' property in DT
+	 */
+	tsdata->irq_pin = -EINVAL;
+
+	ret = of_get_named_gpio_flags(np, "reset-gpios", 0, &gpio_flags);
+	tsdata->reset_pin = ret;
This code looks awkward.
There are two ways, please choose one of them.

1. Check the return value

	ret = of_get_named_gpio_flags(np, "reset-gpios", 0, &gpio_flags);
	if (ret < 0) {
		dev_err(dev, "reset-gpios pin is not provided.\n");
		return ret;
	}
	tsdata->reset_pin = ret;

2. No check the return value, passing it directly

	tsdata->reset_pin = of_get_named_gpio_flags(np, "reset-gpios", 0, &gpio_flags);

+
+	ret = of_get_named_gpio_flags(np, "wake-gpios", 0, &gpio_flags);
According to the Documentation 'edt-ft5x06.txt',
'wakeup-gpios' is used, instead of 'wake-gpios'.
What is the right name?
quoted hunk
+	tsdata->wake_pin = ret;
+
+	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)
 {
@@ -713,30 +779,43 @@ 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 = gpio_request_one(pdata->irq_pin,
+	if (gpio_is_valid(tsdata->irq_pin)) {
+		error = gpio_request_one(tsdata->irq_pin,
 					 GPIOF_IN, "edt-ft5x06 irq");
Please use devm_gpio_request_one() too.
quoted hunk
 		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 = input_allocate_device();
-	if (!tsdata || !input) {
-		dev_err(&client->dev, "failed to allocate driver data.\n");
+	if (!input) {
+		dev_err(&client->dev, "failed to allocate input device.\n");
 		error = -ENOMEM;
 		goto err_free_mem;
 	}
@@ -752,7 +831,11 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		goto err_free_mem;
 	}

-	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,
@@ -802,8 +885,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;
@@ -813,18 +896,18 @@ 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);
+	if (gpio_is_valid(tsdata->irq_pin))
+		gpio_free(tsdata->irq_pin);
+	if (gpio_is_valid(tsdata->reset_pin))
+		gpio_free(tsdata->reset_pin);
+	if (gpio_is_valid(tsdata->wake_pin))
+		gpio_free(tsdata->wake_pin);
If you use devm_gpio_request_one(), these gpio_free()s are not
necessary.
quoted hunk
 	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);
@@ -833,12 +916,12 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
 	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);
+	if (gpio_is_valid(tsdata->irq_pin))
+		gpio_free(tsdata->irq_pin);
+	if (gpio_is_valid(tsdata->reset_pin))
+		gpio_free(tsdata->reset_pin);
+	if (gpio_is_valid(tsdata->wake_pin))
+		gpio_free(tsdata->wake_pin);
If you use devm_gpio_request_one(), these gpio_free()s are not
necessary.

Best regards,
Jingoo Han

Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support

From: Lothar Waßmann <hidden>
Date: 2014-01-16 07:50:11

Hi,

Mark Rutland wrote:
On Mon, Jan 13, 2014 at 10:17:04AM +0000, Lothar Waßmann wrote:
quoted
This patch allows the edt-ft5x06 multitouch panel driver to be
configured via DT.

Signed-off-by: Lothar Waßmann <redacted>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |   31 ++++
 drivers/input/touchscreen/edt-ft5x06.c             |  145 +++++++++++++++----
 2 files changed, 145 insertions(+), 31 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..629dbdd
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -0,0 +1,31 @@
+* EDT FT5x06 Multiple Touch Controller
+
[...]
quoted
+- threshold: allows setting the "click"-threshold in the range from 20 to 80.
+- gain: sensitivity (0..31) (lower value -> higher sensitivity)
+- offset: edge compensation (0..31)
+- report_rate: report rate (3..14)
s/_/-/ on property names please. Also, it may make sense to prefix these
as they're rather generic sounding names.

Could you elaborate on these a litle please? What units are each of
these in? Why does it make sense to have them in the dt?
I just converted them from being passed via platform_data. I have no
idea what they actually control and what units they use. I could not
even find a manual where they are documented.
Maybe Simon Budig can help out here as the original driver author.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

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

Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support

From: Simon Budig <hidden>
Date: 2014-01-17 01:50:21

On 16/01/14 08:49, Lothar Waßmann wrote:
quoted
quoted
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -0,0 +1,31 @@
+* EDT FT5x06 Multiple Touch Controller
+
[...]
quoted
quoted
+- threshold: allows setting the "click"-threshold in the range from 20 to 80.
+- gain: sensitivity (0..31) (lower value -> higher sensitivity)
+- offset: edge compensation (0..31)
+- report_rate: report rate (3..14)
s/_/-/ on property names please. Also, it may make sense to prefix these
as they're rather generic sounding names.

Could you elaborate on these a litle please? What units are each of
these in? Why does it make sense to have them in the dt?
I just converted them from being passed via platform_data. I have no
idea what they actually control and what units they use. I could not
even find a manual where they are documented.
Maybe Simon Budig can help out here as the original driver author.
The units are not specified in the datasheets available to me. I suspect
that these are some sort of counter values related to the cap sensing.

The defaults differ for the different size glasses, so I really suspect
these are basically just numbers.

The only somewhat reasonable unit is available for the report-rate: it
is specified as about <value> * 10 touch reports per second.

Bye,
        Simon

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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help