2014-04-05 22:27:14

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCHv2 0/5] tsc2005 DT binding

Hi,

This adds device tree support for the tsc2005 touchscreen
controller, which is currently only used by the Nokia N900
board.

The patch does not update the reset pin handling for platform
data based probe to avoid merge conflicts. The n900 platform
code will be removed in the near future (3.16?) and the driver
can be simplified once that has happened.

Changes since v1 [0]:
* rename "reset-gpio" to "reset-gpios", so that the code can easily
switch to the gpiod interface (once the boardcode is gone).
* rename ti,fuzz-* and ti,max-* to touchscreen-fuzz-* and
touchscreen-size-* and specify them as common touchscreen binding.
* implement input_parse_touchscreen_of_params(), which parses the
common touchscreen binding.

[0] https://lkml.org/lkml/2013/12/5/607

-- Sebastian

Sebastian Reichel (5):
Input: add common DT binding for touchscreens
Input: tsc2005: use dev_err for error messages
Input: tsc2005: convert driver to use devm_*
Input: tsc2005: add DT support
Documentation: dt: Document TSC2005 DT binding

.../bindings/input/touchscreen/touchscreen.txt | 9 ++
.../bindings/input/touchscreen/tsc2005.txt | 39 +++++++
drivers/input/input.c | 34 ++++++
drivers/input/touchscreen/tsc2005.c | 130 ++++++++++++++-------
include/linux/input.h | 8 ++
5 files changed, 179 insertions(+), 41 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt

--
1.9.1


2014-04-05 22:28:10

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCHv2 4/5] Input: tsc2005: add DT support

This adds DT support to the tsc2005 touchscreen
driver.

Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/input/touchscreen/tsc2005.c | 96 +++++++++++++++++++++++++++++--------
1 file changed, 77 insertions(+), 19 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 9daaddd..a83d1be 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -28,6 +28,8 @@
#include <linux/interrupt.h>
#include <linux/delay.h>
#include <linux/pm.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
#include <linux/spi/spi.h>
#include <linux/spi/tsc2005.h>

@@ -100,6 +102,11 @@
TSC2005_CFR2_AVG_7)

#define MAX_12BIT 0xfff
+#define TSC2005_DEF_X_FUZZ 4
+#define TSC2005_DEF_Y_FUZZ 8
+#define TSC2005_DEF_P_FUZZ 2
+#define TSC2005_DEF_RESISTOR 280
+
#define TSC2005_SPI_MAX_SPEED_HZ 10000000
#define TSC2005_PENUP_TIME_MS 40

@@ -143,6 +150,7 @@ struct tsc2005 {

bool pen_down;

+ int reset_gpio;
void (*set_reset)(bool enable);
};

@@ -337,6 +345,14 @@ static void tsc2005_stop_scan(struct tsc2005 *ts)
tsc2005_cmd(ts, TSC2005_CMD_STOP);
}

+static void tsc2005_set_reset(struct tsc2005 *ts, bool enable)
+{
+ if (ts->reset_gpio >= 0)
+ gpio_set_value(ts->reset_gpio, enable);
+ else if (ts->set_reset)
+ ts->set_reset(enable);
+}
+
/* must be called with ts->mutex held */
static void __tsc2005_disable(struct tsc2005 *ts)
{
@@ -355,7 +371,7 @@ static void __tsc2005_enable(struct tsc2005 *ts)
{
tsc2005_start_scan(ts);

- if (ts->esd_timeout && ts->set_reset) {
+ if (ts->esd_timeout && (ts->set_reset || ts->reset_gpio)) {
ts->last_valid_interrupt = jiffies;
schedule_delayed_work(&ts->esd_work,
round_jiffies_relative(
@@ -414,9 +430,9 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
}

/* hardware reset */
- ts->set_reset(false);
+ tsc2005_set_reset(ts, false);
usleep_range(100, 500); /* only 10us required */
- ts->set_reset(true);
+ tsc2005_set_reset(ts, true);

if (!success)
goto out;
@@ -459,7 +475,7 @@ static umode_t tsc2005_attr_is_visible(struct kobject *kobj,
umode_t mode = attr->mode;

if (attr == &dev_attr_selftest.attr) {
- if (!ts->set_reset)
+ if (!ts->set_reset && !ts->reset_gpio)
mode = 0;
}

@@ -509,9 +525,9 @@ static void tsc2005_esd_work(struct work_struct *work)

tsc2005_update_pen_state(ts, 0, 0, 0);

- ts->set_reset(false);
+ tsc2005_set_reset(ts, false);
usleep_range(100, 500); /* only 10us required */
- ts->set_reset(true);
+ tsc2005_set_reset(ts, true);

enable_irq(ts->spi->irq);
tsc2005_start_scan(ts);
@@ -572,29 +588,47 @@ static void tsc2005_setup_spi_xfer(struct tsc2005 *ts)
static int tsc2005_probe(struct spi_device *spi)
{
const struct tsc2005_platform_data *pdata = dev_get_platdata(&spi->dev);
+ struct device_node *np = spi->dev.of_node;
+
struct tsc2005 *ts;
struct input_dev *input_dev;
- unsigned int max_x, max_y, max_p;
- unsigned int fudge_x, fudge_y, fudge_p;
+ unsigned int max_x = MAX_12BIT;
+ unsigned int max_y = MAX_12BIT;
+ unsigned int max_p = MAX_12BIT;
+ unsigned int fudge_x = TSC2005_DEF_X_FUZZ;
+ unsigned int fudge_y = TSC2005_DEF_Y_FUZZ;
+ unsigned int fudge_p = TSC2005_DEF_P_FUZZ;
+ unsigned int x_plate_ohm = TSC2005_DEF_RESISTOR;
+ unsigned int esd_timeout;
int error;

- if (!pdata) {
+ if (!np && !pdata) {
dev_err(&spi->dev, "no platform data\n");
return -ENODEV;
}

- fudge_x = pdata->ts_x_fudge ? : 4;
- fudge_y = pdata->ts_y_fudge ? : 8;
- fudge_p = pdata->ts_pressure_fudge ? : 2;
- max_x = pdata->ts_x_max ? : MAX_12BIT;
- max_y = pdata->ts_y_max ? : MAX_12BIT;
- max_p = pdata->ts_pressure_max ? : MAX_12BIT;
-
if (spi->irq <= 0) {
dev_err(&spi->dev, "no irq\n");
return -ENODEV;
}

+ if (pdata) {
+ fudge_x = pdata->ts_x_fudge;
+ fudge_y = pdata->ts_y_fudge;
+ fudge_p = pdata->ts_pressure_fudge;
+ max_x = pdata->ts_x_max;
+ max_y = pdata->ts_y_max;
+ max_p = pdata->ts_pressure_max;
+ x_plate_ohm = pdata->ts_x_plate_ohm;
+ esd_timeout = pdata->esd_timeout_ms;
+ } else {
+ x_plate_ohm = TSC2005_DEF_RESISTOR;
+ of_property_read_u32(np, "ti,x-plate-ohms", &x_plate_ohm);
+ esd_timeout = 0;
+ of_property_read_u32(np, "ti,esd-recovery-timeout-ms",
+ &esd_timeout);
+ }
+
spi->mode = SPI_MODE_0;
spi->bits_per_word = 8;
if (!spi->max_speed_hz)
@@ -612,9 +646,30 @@ static int tsc2005_probe(struct spi_device *spi)
ts->spi = spi;
ts->idev = input_dev;

- ts->x_plate_ohm = pdata->ts_x_plate_ohm ? : 280;
- ts->esd_timeout = pdata->esd_timeout_ms;
- ts->set_reset = pdata->set_reset;
+ ts->x_plate_ohm = x_plate_ohm;
+ ts->esd_timeout = esd_timeout;
+
+ if (np) {
+ ts->reset_gpio = of_get_named_gpio(np, "reset-gpios", 0);
+ if (ts->reset_gpio == -EPROBE_DEFER)
+ return ts->reset_gpio;
+ if (ts->reset_gpio < 0) {
+ dev_err(&spi->dev, "error acquiring reset gpio: %d\n",
+ ts->reset_gpio);
+ return ts->reset_gpio;
+ }
+
+ error = devm_gpio_request_one(&spi->dev, ts->reset_gpio, 0,
+ "reset-gpios");
+ if (error) {
+ dev_err(&spi->dev, "error requesting reset gpio: %d\n",
+ error);
+ return error;
+ }
+ } else {
+ ts->reset_gpio = -1;
+ ts->set_reset = pdata->set_reset;
+ }

mutex_init(&ts->mutex);

@@ -639,6 +694,9 @@ static int tsc2005_probe(struct spi_device *spi)
input_set_abs_params(input_dev, ABS_Y, 0, max_y, fudge_y, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, max_p, fudge_p, 0);

+ if (np)
+ input_parse_touchscreen_of_params(input_dev);
+
input_dev->open = tsc2005_open;
input_dev->close = tsc2005_close;

--
1.9.1

2014-04-05 22:28:09

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCHv2 5/5] Documentation: dt: Document TSC2005 DT binding

Add devicetree binding documentation for TSC2005 touchscreen.

Signed-off-by: Sebastian Reichel <[email protected]>
---
.../bindings/input/touchscreen/tsc2005.txt | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
new file mode 100644
index 0000000..a0e5bf5
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
@@ -0,0 +1,39 @@
+* Texas Instruments tsc2005 touchscreen controller
+
+Required properties:
+ - compatible : "ti,tsc2005"
+ - reg : SPI device address
+ - spi-max-frequency : Maximal SPI speed
+ - interrupts : IRQ specifier
+ - reset-gpios : GPIO specifier
+
+Optional properties:
+ - ti,x-plate-resistance : integer, resistance of the touchscreen's X plates
+ in ohm (defaults to 280)
+ - ti,esd-recovery-timeout-ms : integer, if the touchscreen does not respond after
+ the configured time (in milli seconds), the driver
+ will reset it. This is disabled by default.
+ - properties defined in touchscreen.txt
+
+Example:
+
+&mcspi1 {
+ tsc2005@0 {
+ compatible = "ti,tsc2005";
+ spi-max-frequency = <6000000>;
+ reg = <0>;
+
+ reset-gpios = <&gpio4 8 GPIO_ACTIVE_HIGH>; /* 104 */
+ interrupts-extended = <&gpio4 4 IRQ_TYPE_NONE>; /* 100 */
+
+ touchscreen-fuzz-x = <4>;
+ touchscreen-fuzz-y = <7>;
+ touchscreen-fuzz-pressure = <2>;
+ touchscreen-max-x = <4096>;
+ touchscreen-max-y = <4096>;
+ touchscreen-max-pressure = <2048>;
+
+ ti,x-plate-resistance = <280>;
+ ti,esd-recovery-timeout-ms = <8000>;
+ };
+}
--
1.9.1

2014-04-05 22:28:07

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCHv2 3/5] Input: tsc2005: convert driver to use devm_*

Simplify the driver by using managed resources for memory allocation of
internal struct, input device allocation and irq request.

Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/input/touchscreen/tsc2005.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 520e673..9daaddd 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -604,12 +604,10 @@ static int tsc2005_probe(struct spi_device *spi)
if (error)
return error;

- ts = kzalloc(sizeof(*ts), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!ts || !input_dev) {
- error = -ENOMEM;
- goto err_free_mem;
- }
+ ts = devm_kzalloc(&spi->dev, sizeof(*ts), GFP_KERNEL);
+ input_dev = devm_input_allocate_device(&spi->dev);
+ if (!ts || !input_dev)
+ return -ENOMEM;

ts->spi = spi;
ts->idev = input_dev;
@@ -649,12 +647,13 @@ static int tsc2005_probe(struct spi_device *spi)
/* Ensure the touchscreen is off */
tsc2005_stop_scan(ts);

- error = request_threaded_irq(spi->irq, NULL, tsc2005_irq_thread,
- IRQF_TRIGGER_RISING | IRQF_ONESHOT,
- "tsc2005", ts);
+ error = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
+ tsc2005_irq_thread,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "tsc2005", ts);
if (error) {
dev_err(&spi->dev, "Failed to request irq, err: %d\n", error);
- goto err_free_mem;
+ return error;
}

spi_set_drvdata(spi, ts);
@@ -662,7 +661,7 @@ static int tsc2005_probe(struct spi_device *spi)
if (error) {
dev_err(&spi->dev,
"Failed to create sysfs attributes, err: %d\n", error);
- goto err_clear_drvdata;
+ return error;
}

error = input_register_device(ts->idev);
@@ -677,11 +676,6 @@ static int tsc2005_probe(struct spi_device *spi)

err_remove_sysfs:
sysfs_remove_group(&spi->dev.kobj, &tsc2005_attr_group);
-err_clear_drvdata:
- free_irq(spi->irq, ts);
-err_free_mem:
- input_free_device(input_dev);
- kfree(ts);
return error;
}

@@ -691,10 +685,6 @@ static int tsc2005_remove(struct spi_device *spi)

sysfs_remove_group(&ts->spi->dev.kobj, &tsc2005_attr_group);

- free_irq(ts->spi->irq, ts);
- input_unregister_device(ts->idev);
- kfree(ts);
-
return 0;
}

--
1.9.1

2014-04-05 22:28:05

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCHv2 2/5] Input: tsc2005: use dev_err for error messages

Change some dev_dbg() invocations to dev_err() ones, because they
are supposed to output error messages.

Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/input/touchscreen/tsc2005.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 550adcb..520e673 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -579,7 +579,7 @@ static int tsc2005_probe(struct spi_device *spi)
int error;

if (!pdata) {
- dev_dbg(&spi->dev, "no platform data\n");
+ dev_err(&spi->dev, "no platform data\n");
return -ENODEV;
}

@@ -591,7 +591,7 @@ static int tsc2005_probe(struct spi_device *spi)
max_p = pdata->ts_pressure_max ? : MAX_12BIT;

if (spi->irq <= 0) {
- dev_dbg(&spi->dev, "no irq\n");
+ dev_err(&spi->dev, "no irq\n");
return -ENODEV;
}

--
1.9.1

2014-04-05 22:28:00

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCHv2 1/5] Input: add common DT binding for touchscreens

Add common DT binding documentation for touchscreen devices and
implement input_parse_touchscreen_of_params, which parses the common
properties and configures the input device accordingly.

Signed-off-by: Sebastian Reichel <[email protected]>
---
.../bindings/input/touchscreen/touchscreen.txt | 9 ++++++
drivers/input/input.c | 34 ++++++++++++++++++++++
include/linux/input.h | 8 +++++
3 files changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
new file mode 100644
index 0000000..a2ff0a0
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
@@ -0,0 +1,9 @@
+General Touchscreen Properties:
+
+Optional properties for Touchscreens:
+ - touchscreen-size-x : horizontal resolution of touchscreen
+ - touchscreen-size-y : vertical resolution of touchscreen
+ - touchscreen-max-pressure : maximum reported pressure
+ - touchscreen-fuzz-x : horizontal noise value of the absolute input device
+ - touchscreen-fuzz-y : vertical noise value of the absolute input device
+ - touchscreen-fuzz-pressure : pressure noise value of the absolute input device
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 1c4c0db..97966d93 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -27,6 +27,7 @@
#include <linux/device.h>
#include <linux/mutex.h>
#include <linux/rcupdate.h>
+#include <linux/of.h>
#include "input-compat.h"

MODULE_AUTHOR("Vojtech Pavlik <[email protected]>");
@@ -2398,6 +2399,39 @@ void input_free_minor(unsigned int minor)
}
EXPORT_SYMBOL(input_free_minor);

+#if defined(CONFIG_OF)
+/**
+ * input_parse_touchscreen_of_params - parse common touchscreen DT properties
+ * @dev: device that should be parsed
+ *
+ * This function parses common DT properties for touchscreens and setups the
+ * input device accordingly. The function keeps previously setuped default
+ * values if no value is specified via DT.
+ */
+void input_parse_touchscreen_of_params(struct input_dev *dev)
+{
+ struct device_node *np = dev->dev.parent->of_node;
+ struct input_absinfo *absinfo;
+
+ input_alloc_absinfo(dev);
+ if (!dev->absinfo)
+ return;
+
+ absinfo = &dev->absinfo[ABS_X];
+ of_property_read_u32(np, "touchscreen-size-x", &absinfo->maximum);
+ of_property_read_u32(np, "touchscreen-fuzz-x", &absinfo->fuzz);
+
+ absinfo = &dev->absinfo[ABS_Y];
+ of_property_read_u32(np, "touchscreen-size-y", &absinfo->maximum);
+ of_property_read_u32(np, "touchscreen-fuzz-y", &absinfo->fuzz);
+
+ absinfo = &dev->absinfo[ABS_PRESSURE];
+ of_property_read_u32(np, "touchscreen-max-pressure", &absinfo->maximum);
+ of_property_read_u32(np, "touchscreen-fuzz-pressure", &absinfo->fuzz);
+}
+EXPORT_SYMBOL(input_parse_touchscreen_of_params);
+#endif
+
static int __init input_init(void)
{
int err;
diff --git a/include/linux/input.h b/include/linux/input.h
index 82ce323..3dc3b1e 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -531,4 +531,12 @@ int input_ff_erase(struct input_dev *dev, int effect_id, struct file *file);
int input_ff_create_memless(struct input_dev *dev, void *data,
int (*play_effect)(struct input_dev *, void *, struct ff_effect *));

+#if defined(CONFIG_OF)
+void input_parse_touchscreen_of_params(struct input_dev *dev);
+#else
+static inline void input_parse_touchscreen_of_params(struct input_dev *dev) {
+ return;
+}
+#endif
+
#endif
--
1.9.1

2014-04-07 16:38:11

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCHv2 1/5] Input: add common DT binding for touchscreens

On Sat, Apr 5, 2014 at 5:26 PM, Sebastian Reichel <[email protected]> wrote:
> Add common DT binding documentation for touchscreen devices and
> implement input_parse_touchscreen_of_params, which parses the common
> properties and configures the input device accordingly.

Good.

>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> .../bindings/input/touchscreen/touchscreen.txt | 9 ++++++
> drivers/input/input.c | 34 ++++++++++++++++++++++
> include/linux/input.h | 8 +++++
> 3 files changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> new file mode 100644
> index 0000000..a2ff0a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> @@ -0,0 +1,9 @@
> +General Touchscreen Properties:
> +
> +Optional properties for Touchscreens:
> + - touchscreen-size-x : horizontal resolution of touchscreen
> + - touchscreen-size-y : vertical resolution of touchscreen

While I like the consistency, x-size and y-size are already commonly
used. Perhaps the common binding should have both and x-size/y-size be
marked deprecated.

> + - touchscreen-max-pressure : maximum reported pressure
> + - touchscreen-fuzz-x : horizontal noise value of the absolute input device
> + - touchscreen-fuzz-y : vertical noise value of the absolute input device
> + - touchscreen-fuzz-pressure : pressure noise value of the absolute input device

What are the units or are they just an arbitrary range dependent on
the controller? Several existing bindings appear to be in pixels, but
that seems wrong to me.

There's also these various properties that should have common versions created:

ti,x-plate-resistance and ti,x-plate-ohms (tsc2007)
- rohm,flip-x : Flip touch coordinates on the X axis
- rohm,flip-y : Flip touch coordinates on the Y axis
- x-invert: invert X axis
- y-invert: invert Y axis
- contact-threshold:
- moving-threshold:

Rob

2014-04-07 20:00:34

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCHv2 1/5] Input: add common DT binding for touchscreens

On Mon, Apr 07, 2014 at 11:38:01AM -0500, Rob Herring wrote:
> On Sat, Apr 5, 2014 at 5:26 PM, Sebastian Reichel <[email protected]> wrote:
> > Add common DT binding documentation for touchscreen devices and
> > implement input_parse_touchscreen_of_params, which parses the common
> > properties and configures the input device accordingly.
>
> Good.
>
> [...]
> > +Optional properties for Touchscreens:
> > + - touchscreen-size-x : horizontal resolution of touchscreen
> > + - touchscreen-size-y : vertical resolution of touchscreen
>
> While I like the consistency, x-size and y-size are already commonly
> used. Perhaps the common binding should have both and x-size/y-size be
> marked deprecated.

So you want me to add something like the following?

- x-size : deprecated name for touchscreen-size-x
- y-size : deprecated name for touchscreen-size-y

> > + - touchscreen-max-pressure : maximum reported pressure
> > + - touchscreen-fuzz-x : horizontal noise value of the absolute input device
> > + - touchscreen-fuzz-y : vertical noise value of the absolute input device
> > + - touchscreen-fuzz-pressure : pressure noise value of the absolute input device
>
> What are the units or are they just an arbitrary range dependent on
> the controller? Several existing bindings appear to be in pixels, but
> that seems wrong to me.

x/y related properties: pixels
pressure related properties: arbitrary range dependent on the controller

> There's also these various properties that should have common versions created:
>
> ti,x-plate-resistance and ti,x-plate-ohms (tsc2007)

I think this is ti specific. But I should probably name the tsc2005
property "ti,x-plate-ohms", so that its in sync with tsc2007.

> - rohm,flip-x : Flip touch coordinates on the X axis
> - rohm,flip-y : Flip touch coordinates on the Y axis
> - x-invert: invert X axis
> - y-invert: invert Y axis

like this?

- touchscreen-inverted-x: X axis is inverted
- touchscreen-inverted-y: Y axis is inverted
- x-invert: deprecated name for touchscreen-inverted-x
- y-invert: deprecated name for touchscreen-inverted-y

Inverting is currently not supported by the input system, though. So
adding support for it to input_parse_touchscreen_of_params() is not
trivial :/

> - contact-threshold:
> - moving-threshold:

I think those are hardware specific. The properties are currently
used by one driver, which writes them directly into some registers.
The DT binding does not give more information about them.

I think those should have been vendor-prefixed.

-- Sebastian


Attachments:
(No filename) (2.54 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-04-08 02:53:54

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCHv2 1/5] Input: add common DT binding for touchscreens

On Mon, Apr 7, 2014 at 3:00 PM, Sebastian Reichel <[email protected]> wrote:
> On Mon, Apr 07, 2014 at 11:38:01AM -0500, Rob Herring wrote:
>> On Sat, Apr 5, 2014 at 5:26 PM, Sebastian Reichel <[email protected]> wrote:
>> > Add common DT binding documentation for touchscreen devices and
>> > implement input_parse_touchscreen_of_params, which parses the common
>> > properties and configures the input device accordingly.
>>
>> Good.
>>
>> [...]
>> > +Optional properties for Touchscreens:
>> > + - touchscreen-size-x : horizontal resolution of touchscreen
>> > + - touchscreen-size-y : vertical resolution of touchscreen
>>
>> While I like the consistency, x-size and y-size are already commonly
>> used. Perhaps the common binding should have both and x-size/y-size be
>> marked deprecated.
>
> So you want me to add something like the following?
>
> - x-size : deprecated name for touchscreen-size-x
> - y-size : deprecated name for touchscreen-size-y

Yes.

>
>> > + - touchscreen-max-pressure : maximum reported pressure
>> > + - touchscreen-fuzz-x : horizontal noise value of the absolute input device
>> > + - touchscreen-fuzz-y : vertical noise value of the absolute input device
>> > + - touchscreen-fuzz-pressure : pressure noise value of the absolute input device
>>
>> What are the units or are they just an arbitrary range dependent on
>> the controller? Several existing bindings appear to be in pixels, but
>> that seems wrong to me.
>
> x/y related properties: pixels
> pressure related properties: arbitrary range dependent on the controller

Please make this clear in the binding description.

>> There's also these various properties that should have common versions created:
>>
>> ti,x-plate-resistance and ti,x-plate-ohms (tsc2007)
>
> I think this is ti specific. But I should probably name the tsc2005
> property "ti,x-plate-ohms", so that its in sync with tsc2007.
>
>> - rohm,flip-x : Flip touch coordinates on the X axis
>> - rohm,flip-y : Flip touch coordinates on the Y axis
>> - x-invert: invert X axis
>> - y-invert: invert Y axis
>
> like this?
>
> - touchscreen-inverted-x: X axis is inverted
> - touchscreen-inverted-y: Y axis is inverted
> - x-invert: deprecated name for touchscreen-inverted-x
> - y-invert: deprecated name for touchscreen-inverted-y
>
> Inverting is currently not supported by the input system, though. So
> adding support for it to input_parse_touchscreen_of_params() is not
> trivial :/

Does not matter. I'm only asking you to define the property. Linux
does not have to use it currently.

>> - contact-threshold:
>> - moving-threshold:
>
> I think those are hardware specific. The properties are currently
> used by one driver, which writes them directly into some registers.
> The DT binding does not give more information about them.

moving-threshold is probably the same as fuzz. contact-threshold is
sounds like min pressure. I remember having to tune the minimum
pressure on a touchscreen I worked on long ago. So think think is
definitely not an uncommon property.

Rob