diff mbox

[PATCHv4,2/5] Input: edt-ft5x06: Add DT support

Message ID 1395234563-11034-3-git-send-email-LW@KARO-electronics.de
State Superseded, archived
Headers show

Commit Message

Lothar Waßmann March 19, 2014, 1:09 p.m. UTC
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |   41 ++++++
 drivers/input/touchscreen/edt-ft5x06.c             |  144 +++++++++++++++-----
 2 files changed, 154 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt

Comments

Mark Rutland March 20, 2014, 9:37 a.m. UTC | #1
On Wed, Mar 19, 2014 at 01:09:20PM +0000, Lothar Waßmann wrote:
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  .../bindings/input/touchscreen/edt-ft5x06.txt      |   41 ++++++
>  drivers/input/touchscreen/edt-ft5x06.c             |  144 +++++++++++++++-----
>  2 files changed, 154 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..e5adc76
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> @@ -0,0 +1,41 @@
> +FocalTech EDT-FT5x06 Polytouch driver
> +=====================================
> +
> +Required properties:
> + - compatible:  "edt,edt-ft5x06"

Is the 'x' part of a particular product name, or is this a class of
devices?

It's preferable to have a specific string which another similar variants
can claim compatibility with (while also additionally having a more
specific string), as this makes it possible to handle variants more
specially in future, target workarounds, etc.

> + - reg:         I2C slave address of the chip (0x38)
> + - interrupt-parent: a phandle pointing to the interrupt controller
> +                     serving the interrupt for this chip
> + - interrupts:       interrupt specification for this chip

How many? What are they for?

> +
> +Optional properties:
> + - reset-gpios: GPIO specification for the RESET input
> + - wake-gpios:  GPIO specification for the WAKE input
> +
> + - pinctrl-names: should be "default"
> + - pinctrl-0:   a phandle pointing to the pin settings for the
> +                control gpios

These all looks fine.

> +
> + - threshold:   allows setting the "click"-threshold in the range
> +                from 20 to 80.
> +
> + - gain:        allows setting the sensitivity in the range from 0 to
> +                31. Note that lower values indicate higher
> +                sensitivity.
> +
> + - offset:      allows setting the edge compensation in the range from
> +                0 to 31.

I can see why the sane values for these may differ between boards.

> + - report_rate: allows setting the report rate in the range from 3 to
> +                14.

However, why can the kernel not decide the report rate? This doesn't
sound like something that needs to vary per-board.

Also, s/_/-/ in property names, please.

[...]

> +#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, it'll handle endianness conversion for you.

Use of of_get_property is almost always wrong.

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Budig March 20, 2014, 11:18 a.m. UTC | #2
On 20/03/14 10:37, Mark Rutland wrote:
> On Wed, Mar 19, 2014 at 01:09:20PM +0000, Lothar Waßmann wrote:
>> +FocalTech EDT-FT5x06 Polytouch driver
>> +=====================================
>> +
>> +Required properties:
>> + - compatible:  "edt,edt-ft5x06"
> 
> Is the 'x' part of a particular product name, or is this a class of
> devices?

The driver is intended for the EDT "polytouch" family touches, which are
based on a focaltec controller. The current line of touches uses e.g.
the ft5306 as well as the ft5406 focaltec controller.

> It's preferable to have a specific string which another similar variants
> can claim compatibility with (while also additionally having a more
> specific string), as this makes it possible to handle variants more
> specially in future, target workarounds, etc.

I chose the driver name since I wanted to differentiate from other EDT
touches, which used a different controller. With hindsight it was
unfortunately confusing, since I get quite some request from people, who
also have a device based on the focaltec controllers, but with a very
different firmware (and communication protocol). They got tricked into
thinking that this driver would be suitable...

If I were to chose the name again I'd probably pick "edt-polytouch" or
something like this. But I doubt that it is useful to change the name now.

Bye,
        Simon
Lothar Waßmann March 20, 2014, 11:40 a.m. UTC | #3
Hi,

Mark Rutland wrote:
> On Wed, Mar 19, 2014 at 01:09:20PM +0000, Lothar Waßmann wrote:
> > 
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> >  .../bindings/input/touchscreen/edt-ft5x06.txt      |   41 ++++++
> >  drivers/input/touchscreen/edt-ft5x06.c             |  144 +++++++++++++++-----
> >  2 files changed, 154 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..e5adc76
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > @@ -0,0 +1,41 @@
> > +FocalTech EDT-FT5x06 Polytouch driver
> > +=====================================
> > +
> > +Required properties:
> > + - compatible:  "edt,edt-ft5x06"
> 
> Is the 'x' part of a particular product name, or is this a class of
> devices?
> 
The FT5x06 datasheet lists 3 variants for different panel sizes.
I'll add distinct compatible strings for those chips, though their SW
interface is (currently) identical:
|Required properties:
| - compatible:  "edt,edt-ft5206", "edt,edt-ft5x06"
|           or:  "edt,edt-ft5306", "edt,edt-ft5x06"
|           or:  "edt,edt-ft5406", "edt,edt-ft5x06"


> > + - reg:         I2C slave address of the chip (0x38)
> > + - interrupt-parent: a phandle pointing to the interrupt controller
> > +                     serving the interrupt for this chip
> > + - interrupts:       interrupt specification for this chip
> 
> How many? What are they for?
>
I'll change it to: 
| - interrupts:       interrupt specification for the touchdetect
|                     interrupt

> > + - report_rate: allows setting the report rate in the range from 3 to
> > +                14.
> 
> However, why can the kernel not decide the report rate? This doesn't
> sound like something that needs to vary per-board.
> 
> Also, s/_/-/ in property names, please.
> 
OK, I'll drop the property, which also simplyfies the code a bit.

> > +#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, it'll handle endianness conversion for you.
> 
> Use of of_get_property is almost always wrong.
> 
Sure.


Lothar Waßmann
Lothar Waßmann March 20, 2014, 11:44 a.m. UTC | #4
Hi,

fugang.duan@freescale.com wrote:
> From: Lothar Waßmann <LW@KARO-electronics.de>
> Data: Wednesday, March 19, 2014 9:09 PM
> 
> >To: Dmitry Torokhov; Duan Fugang-B38611; Grant Likely; Henrik Rydberg; Ian
> >Campbell; Jingoo Han; Kumar Gala; Mark Rutland; Pawel Moll; Rob Herring; Rob
> >Landley; Sachin Kamat; devicetree@vger.kernel.org; linux-doc@vger.kernel.org;
> >linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Simon Budig; Lothar
> >Waßmann
> >Subject: [PATCHv4 2/5] Input: edt-ft5x06: Add DT support
> >
> >
No need to quote the mail headers here.

[...]
> >diff --git a/drivers/input/touchscreen/edt-ft5x06.c
> >b/drivers/input/touchscreen/edt-ft5x06.c
> >index 7b4470d..257a1c8 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>
> >
> [...]
> >+#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;
> Don't need to check the device node valid. If the device node is not existed, the driver don't run probe.
> 
Perfectly right. I'll drop this in the next version.


Lothar Waßmann
Simon Budig March 20, 2014, 11:48 a.m. UTC | #5
On 20/03/14 12:40, Lothar Waßmann wrote:
> The FT5x06 datasheet lists 3 variants for different panel sizes.
> I'll add distinct compatible strings for those chips, though their SW
> interface is (currently) identical:
> |Required properties:
> | - compatible:  "edt,edt-ft5206", "edt,edt-ft5x06"
> |           or:  "edt,edt-ft5306", "edt,edt-ft5x06"
> |           or:  "edt,edt-ft5406", "edt,edt-ft5x06"

Please note that the M09 variants of the polytouch family will use the
same chips, but a different sw protocol (see Patch 5/5).

I don't know anything about the naming scheme used in the device tree,
but I am not sure if the chip-ID is actually useful here.

Bye,
        Simon
Lothar Waßmann March 20, 2014, 12:05 p.m. UTC | #6
Hi,

Simon Budig wrote:
> On 20/03/14 12:40, Lothar Waßmann wrote:
> > The FT5x06 datasheet lists 3 variants for different panel sizes.
> > I'll add distinct compatible strings for those chips, though their SW
> > interface is (currently) identical:
> > |Required properties:
> > | - compatible:  "edt,edt-ft5206", "edt,edt-ft5x06"
> > |           or:  "edt,edt-ft5306", "edt,edt-ft5x06"
> > |           or:  "edt,edt-ft5406", "edt,edt-ft5x06"
> 
> Please note that the M09 variants of the polytouch family will use the
> same chips, but a different sw protocol (see Patch 5/5).
> 
The firmware version can be detected by SW, thus there is no need to
distinguish it via DT.

> I don't know anything about the naming scheme used in the device tree,
> but I am not sure if the chip-ID is actually useful here.
> 
Since the chip variants handle different panel sizes, it might be useful
to be able to distinguish them.


Lothar Waßmann
Mark Rutland March 20, 2014, 1:19 p.m. UTC | #7
On Thu, Mar 20, 2014 at 11:18:10AM +0000, Simon Budig wrote:
> On 20/03/14 10:37, Mark Rutland wrote:
> > On Wed, Mar 19, 2014 at 01:09:20PM +0000, Lothar Waßmann wrote:
> >> +FocalTech EDT-FT5x06 Polytouch driver
> >> +=====================================
> >> +
> >> +Required properties:
> >> + - compatible:  "edt,edt-ft5x06"
> > 
> > Is the 'x' part of a particular product name, or is this a class of
> > devices?
> 
> The driver is intended for the EDT "polytouch" family touches, which are
> based on a focaltec controller. The current line of touches uses e.g.
> the ft5306 as well as the ft5406 focaltec controller.

Ok. The name of the driver and the strings used in DT bindings don't
have to be the same. The bindings strings should describe the hardware
as accurately as possible.

> 
> > It's preferable to have a specific string which another similar variants
> > can claim compatibility with (while also additionally having a more
> > specific string), as this makes it possible to handle variants more
> > specially in future, target workarounds, etc.
> 
> I chose the driver name since I wanted to differentiate from other EDT
> touches, which used a different controller. With hindsight it was
> unfortunately confusing, since I get quite some request from people, who
> also have a device based on the focaltec controllers, but with a very
> different firmware (and communication protocol). They got tricked into
> thinking that this driver would be suitable...
> 
> If I were to chose the name again I'd probably pick "edt-polytouch" or
> something like this. But I doubt that it is useful to change the name now.

I'm not arguing to rename the driver. All I want are the compatible
strings to be as specific as possible.

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland March 20, 2014, 1:21 p.m. UTC | #8
On Thu, Mar 20, 2014 at 11:40:55AM +0000, Lothar Waßmann wrote:
> Hi,
> 
> Mark Rutland wrote:
> > On Wed, Mar 19, 2014 at 01:09:20PM +0000, Lothar Waßmann wrote:
> > > 
> > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > ---
> > >  .../bindings/input/touchscreen/edt-ft5x06.txt      |   41 ++++++
> > >  drivers/input/touchscreen/edt-ft5x06.c             |  144 +++++++++++++++-----
> > >  2 files changed, 154 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..e5adc76
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > > @@ -0,0 +1,41 @@
> > > +FocalTech EDT-FT5x06 Polytouch driver
> > > +=====================================
> > > +
> > > +Required properties:
> > > + - compatible:  "edt,edt-ft5x06"
> > 
> > Is the 'x' part of a particular product name, or is this a class of
> > devices?
> > 
> The FT5x06 datasheet lists 3 variants for different panel sizes.
> I'll add distinct compatible strings for those chips, though their SW
> interface is (currently) identical:
> |Required properties:
> | - compatible:  "edt,edt-ft5206", "edt,edt-ft5x06"
> |           or:  "edt,edt-ft5306", "edt,edt-ft5x06"
> |           or:  "edt,edt-ft5406", "edt,edt-ft5x06"

Drop the "edt,edt-ft5x06" string. If they really appear to be identical
from a programmer's perspective, choose a particular variant to be used
as the fallback.

There could be a future variant that would appear to match the x string
yet was completely incompatible with the expected programming interface.

> > > + - reg:         I2C slave address of the chip (0x38)
> > > + - interrupt-parent: a phandle pointing to the interrupt controller
> > > +                     serving the interrupt for this chip
> > > + - interrupts:       interrupt specification for this chip
> > 
> > How many? What are they for?
> >
> I'll change it to: 
> | - interrupts:       interrupt specification for the touchdetect
> |                     interrupt

That sounds fine to me.

> > > + - report_rate: allows setting the report rate in the range from 3 to
> > > +                14.
> > 
> > However, why can the kernel not decide the report rate? This doesn't
> > sound like something that needs to vary per-board.
> > 
> > Also, s/_/-/ in property names, please.
> > 
> OK, I'll drop the property, which also simplyfies the code a bit.

Sounds good to me.

> 
> > > +#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, it'll handle endianness conversion for you.
> > 
> > Use of of_get_property is almost always wrong.
> > 
> Sure.

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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..e5adc76
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -0,0 +1,41 @@ 
+FocalTech EDT-FT5x06 Polytouch driver
+=====================================
+
+Required properties:
+ - compatible:  "edt,edt-ft5x06"
+ - reg:         I2C slave address of the chip (0x38)
+ - interrupt-parent: a phandle pointing to the interrupt controller
+                     serving the interrupt for this chip
+ - interrupts:       interrupt specification for this chip
+
+Optional properties:
+ - reset-gpios: GPIO specification for the RESET input
+ - wake-gpios:  GPIO specification for the WAKE input
+
+ - pinctrl-names: should be "default"
+ - pinctrl-0:   a phandle pointing to the pin settings for the
+                control gpios
+
+ - threshold:   allows setting the "click"-threshold in the range
+                from 20 to 80.
+
+ - gain:        allows setting the sensitivity in the range from 0 to
+                31. Note that lower values indicate higher
+                sensitivity.
+
+ - offset:      allows setting the edge compensation in the range from
+                0 to 31.
+ - report_rate: allows setting the report rate in the range from 3 to
+                14.
+
+Example:
+	polytouch: edt-ft5x06@38 {
+		compatible = "edt,edt-ft5x06";
+		reg = <0x38>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&edt_ft5x06_pins>;
+		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 7b4470d..257a1c8 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,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,
-					      GPIOF_OUT_INIT_LOW,
-					      "edt-ft5x06 reset");
+		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);
+		gpio_set_value(tsdata->reset_pin, 1);
 		mdelay(100);
 	}
 
@@ -675,6 +694,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)
@@ -702,6 +736,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,32 +775,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 = devm_kzalloc(&client->dev, sizeof(*tsdata), GFP_KERNEL);
-	if (!tsdata) {
-		dev_err(&client->dev, "failed to allocate driver data.\n");
-		return -ENOMEM;
-	}
-
 	input = devm_input_allocate_device(&client->dev);
 	if (!input) {
 		dev_err(&client->dev, "failed to allocate input device.\n");
@@ -757,7 +826,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,
@@ -787,10 +860,10 @@  static int edt_ft5x06_ts_probe(struct i2c_client *client,
 	input_set_drvdata(input, tsdata);
 	i2c_set_clientdata(client, tsdata);
 
-	error = devm_request_threaded_irq(&client->dev, 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");
 		return error;
@@ -801,19 +874,21 @@  static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		return error;
 
 	error = input_register_device(input);
-	if (error) {
-		sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
-		return error;
-	}
+	if (error)
+		goto err_remove_attrs;
 
 	edt_ft5x06_ts_prepare_debugfs(tsdata, dev_driver_string(&client->dev));
 	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;
+
+err_remove_attrs:
+	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_attr_group);
+	return error;
 }
 
 static int edt_ft5x06_ts_remove(struct i2c_client *client)
@@ -857,10 +932,17 @@  static const struct i2c_device_id edt_ft5x06_ts_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, edt_ft5x06_ts_id);
 
+static const struct of_device_id edt_ft5x06_of_match[] = {
+	{ .compatible = "edt,edt-ft5x06", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, edt_ft5x06_of_match);
+
 static struct i2c_driver edt_ft5x06_ts_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "edt_ft5x06",
+		.of_match_table = edt_ft5x06_of_match,
 		.pm = &edt_ft5x06_ts_pm_ops,
 	},
 	.id_table = edt_ft5x06_ts_id,