* [PATCH 1/2] DT: Add vendor prefix for Emerging Display Technologies @ 2014-01-13 10:17 Lothar Waßmann 2014-01-13 10:17 ` [PATCH 2/2] Input: edt-ft5x06: Add DT support Lothar Waßmann 0 siblings, 1 reply; 6+ messages in thread From: Lothar Waßmann @ 2014-01-13 10:17 UTC (permalink / raw) To: linux-input-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov, Grant Likely, Thierry Reding, Jonathan Cameron, Shawn Guo, Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Lothar Waßmann Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org> --- .../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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] Input: edt-ft5x06: Add DT support 2014-01-13 10:17 [PATCH 1/2] DT: Add vendor prefix for Emerging Display Technologies Lothar Waßmann @ 2014-01-13 10:17 ` Lothar Waßmann 2014-01-13 13:44 ` Mark Rutland 2014-01-15 0:46 ` Jingoo Han 0 siblings, 2 replies; 6+ messages in thread From: Lothar Waßmann @ 2014-01-13 10:17 UTC (permalink / raw) To: linux-input, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov, Grant Likely, Thierry Reding, Jonathan Cameron, Shawn Guo, Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat, devicetree, linux-doc, linux-kernel Cc: Lothar Waßmann This patch allows the edt-ft5x06 multitouch panel driver to be configured via DT. Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> --- .../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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support 2014-01-13 10:17 ` [PATCH 2/2] Input: edt-ft5x06: Add DT support Lothar Waßmann @ 2014-01-13 13:44 ` Mark Rutland 2014-01-16 7:49 ` Lothar Waßmann 2014-01-15 0:46 ` Jingoo Han 1 sibling, 1 reply; 6+ messages in thread From: Mark Rutland @ 2014-01-13 13:44 UTC (permalink / raw) To: Lothar Waßmann Cc: linux-input, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov, grant.likely, Thierry Reding, Jonathan Cameron, Shawn Guo, Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat, devicetree, linux-doc, linux-kernel On Mon, Jan 13, 2014 at 10:17:04AM +0000, Lothar Waßmann wrote: > This patch allows the edt-ft5x06 multitouch panel driver to be > configured via DT. > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> > --- > .../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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support 2014-01-13 13:44 ` Mark Rutland @ 2014-01-16 7:49 ` Lothar Waßmann 2014-01-17 1:39 ` Simon Budig 0 siblings, 1 reply; 6+ messages in thread From: Lothar Waßmann @ 2014-01-16 7:49 UTC (permalink / raw) To: Mark Rutland Cc: linux-input, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov, grant.likely, Thierry Reding, Jonathan Cameron, Shawn Guo, Silvio F, Guennadi Liakhovetski, Jingoo Han, Fugang Duan, Sachin Kamat, devicetree, linux-doc, linux-kernel, Simon Budig Hi, Mark Rutland wrote: > On Mon, Jan 13, 2014 at 10:17:04AM +0000, Lothar Waßmann wrote: > > This patch allows the edt-ft5x06 multitouch panel driver to be > > configured via DT. > > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de> > > --- > > .../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 > > + [...] > > +- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support 2014-01-16 7:49 ` Lothar Waßmann @ 2014-01-17 1:39 ` Simon Budig 0 siblings, 0 replies; 6+ messages in thread From: Simon Budig @ 2014-01-17 1:39 UTC (permalink / raw) To: Lothar Waßmann Cc: Mark Rutland, linux-input, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala, Rob Landley, Dmitry Torokhov, grant.likely, Thierry Reding, 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: 1608 bytes --] On 16/01/14 08:49, Lothar Waßmann wrote: >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt >>> @@ -0,0 +1,31 @@ >>> +* EDT FT5x06 Multiple Touch Controller >>> + > [...] >>> +- 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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 242 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Input: edt-ft5x06: Add DT support 2014-01-13 10:17 ` [PATCH 2/2] Input: edt-ft5x06: Add DT support Lothar Waßmann 2014-01-13 13:44 ` Mark Rutland @ 2014-01-15 0:46 ` Jingoo Han 1 sibling, 0 replies; 6+ messages in thread From: Jingoo Han @ 2014-01-15 0:46 UTC (permalink / raw) To: 'Lothar Waßmann' Cc: linux-input, 'Rob Herring', 'Pawel Moll', 'Mark Rutland', 'Ian Campbell', 'Kumar Gala', 'Rob Landley', 'Dmitry Torokhov', 'Grant Likely', 'Thierry Reding', 'Jonathan Cameron', 'Shawn Guo', 'Silvio F', 'Guennadi Liakhovetski', 'Fugang Duan', 'Sachin Kamat', devicetree, linux-doc, linux-kernel, 'Jingoo Han', 'Simon Budig' 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 <LW@KARO-electronics.de> (+cc Simon Budig) This 'edt-ft5x06' driver was made by Simon Budig. Please, add him to CC list. Also, I added some comments. :-) > --- > .../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 > + > +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. > 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? > + 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. > 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. > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-17 1:39 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-01-13 10:17 [PATCH 1/2] DT: Add vendor prefix for Emerging Display Technologies Lothar Waßmann 2014-01-13 10:17 ` [PATCH 2/2] Input: edt-ft5x06: Add DT support Lothar Waßmann 2014-01-13 13:44 ` Mark Rutland 2014-01-16 7:49 ` Lothar Waßmann 2014-01-17 1:39 ` Simon Budig 2014-01-15 0:46 ` Jingoo Han
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).