drivers: touchscreen: tsc2007 and ads7846/tsc2046 improvements (use common touchscreen bindings, pre-calibration, spi fix and provide iio raw values)

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 .../bindings/input/touchscreen/tsc2007.txt         |  20 ++--
 drivers/input/touchscreen/tsc2007.c                | 126 +++++++++++++++++----
 include/linux/i2c/tsc2007.h                        |   8 ++
 3 files changed, 123 insertions(+), 31 deletions(-)

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +1 -4

index ec365e1..6e9fd55 100644
@@ -6,6 +6,7 @@ Required properties:
 - ti,x-plate-ohms: X-plate resistance in ohms.
 
 Optional properties:
+- generic touch screen properties: see touchscreen binding [2].
 - gpios: the interrupt gpio the chip is connected to (trough the penirq pin).
   The penirq pin goes to low when the panel is touched.
   (see GPIO binding[1] for more details).

@@ -13,17 +14,20 @@ Optional properties:
   (see interrupt binding[0]).
 - interrupts: (gpio) interrupt to which the chip is connected
   (see interrupt binding[0]).
-- ti,max-rt: maximum pressure.
-- ti,fuzzx: specifies the absolute input fuzz x value.
-  If set, it will permit noise in the data up to +- the value given to the fuzz
-  parameter, that is used to filter noise from the event stream.

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +2 -1

 - ti,poll-period: how much time to wait (in milliseconds) before reading again the
-  values from the tsc2007.
+  values from the tsc2007 (default 1).
 
 [0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
 [1]: Documentation/devicetree/bindings/gpio/gpio.txt
+[2]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
 
 Example:
 	&i2c1 {

Quoted 14 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

Quoted 7 lines

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

Seems like you are breaking compatibility with old DTs. I can't tell for sure though.

Quoted 35 lines

[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 .../bindings/input/touchscreen/tsc2007.txt         |  20 ++--
 drivers/input/touchscreen/tsc2007.c                | 126 +++++++++++++++++----
 include/linux/i2c/tsc2007.h                        |   8 ++
 3 files changed, 123 insertions(+), 31 deletions(-)

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +4 -6

index ec365e1..6e9fd55 100644
@@ -6,6 +6,7 @@ Required properties:
 - ti,x-plate-ohms: X-plate resistance in ohms.
 
 Optional properties:
+- generic touch screen properties: see touchscreen binding [2].
 - gpios: the interrupt gpio the chip is connected to (trough the penirq pin).
   The penirq pin goes to low when the panel is touched.
   (see GPIO binding[1] for more details).

@@ -13,17 +14,20 @@ Optional properties:
   (see interrupt binding[0]).
 - interrupts: (gpio) interrupt to which the chip is connected
   (see interrupt binding[0]).
-- ti,max-rt: maximum pressure.
-- ti,fuzzx: specifies the absolute input fuzz x value.
-  If set, it will permit noise in the data up to +- the value given to the fuzz
-  parameter, that is used to filter noise from the event stream.
-- ti,fuzzy: specifies the absolute input fuzz y value.
-- ti,fuzzz: specifies the absolute input fuzz z value.
+- ti,max-rt: maximum pressure resistance above which samples are ignored
+  (default: 4095).
+- ti,report-resistance: report resistance (no pressure = max_rt) instead

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt Optional properties: · line 24

Quoted 5 lines

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +5 -0

+  of pressure (no pressure = 0).
+- ti,min-x: minimum value reported by X axis ADC (default 0).
+- ti,max-x: maximum value reported by X axis ADC (default 4095).
+- ti,min-y: minimum value reported by Y axis ADC (default 0).
+- ti,max-y: maximum value reported by Y axis ADC (default 4095).

Seems like these could be common too. They make more sense than giving x and y sizes in pixel units which really should come from the panel.

Am 24.09.2016 um 02:31 schrieb Sebastian Reichel <[email protected]>:

Quoted 61 lines · Rob Herring

Quoted 14 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

Quoted 7 lines

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

Seems like you are breaking compatibility with old DTs. I can't tell for sure though.

Quoted 35 lines

[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

Signed-off-by H. Nikolaus Schaller <[email protected]>

---

.../bindings/input/touchscreen/tsc2007.txt         |  20 ++--
drivers/input/touchscreen/tsc2007.c                | 126 +++++++++++++++++----
include/linux/i2c/tsc2007.h                        |   8 ++
3 files changed, 123 insertions(+), 31 deletions(-)

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +0 -1

index ec365e1..6e9fd55 100644
@@ -6,6 +6,7 @@ Required properties:
- ti,x-plate-ohms: X-plate resistance in ohms.
 

Optional properties:
+- generic touch screen properties: see touchscreen binding [2].
- gpios: the interrupt gpio the chip is connected to (trough the penirq pin). The penirq pin goes to low when the panel is touched.
(see GPIO binding[1] for more details).

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +3 -7

@@ -13,17 +14,20 @@ Optional properties:
  (see interrupt binding[0]).
- interrupts: (gpio) interrupt to which the chip is connected
  (see interrupt binding[0]).
-- ti,max-rt: maximum pressure.
-- ti,fuzzx: specifies the absolute input fuzz x value.
-  If set, it will permit noise in the data up to +- the value given to the fuzz
-  parameter, that is used to filter noise from the event stream.
-- ti,fuzzy: specifies the absolute input fuzz y value.
-- ti,fuzzz: specifies the absolute input fuzz z value.
+- ti,max-rt: maximum pressure resistance above which samples are ignored
+  (default: 4095).
+- ti,report-resistance: report resistance (no pressure = max_rt) instead

Quoted 8 lines

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt Optional properties: · line 24

Quoted 5 lines

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +5 -0

+  of pressure (no pressure = 0).
+- ti,min-x: minimum value reported by X axis ADC (default 0).
+- ti,max-x: maximum value reported by X axis ADC (default 4095).
+- ti,min-y: minimum value reported by Y axis ADC (default 0).
+- ti,max-y: maximum value reported by Y axis ADC (default 4095).

Seems like these could be common too. They make more sense than giving x and y sizes in pixel units which really should come from the panel.

The generic touchscreen properties are described "(in pixels)" in the DT, but they are used in the same way.

So ti,max-[xy] is basically the same as touchscreen-size-[xy],

except, that the generic bindings don't support min-[xy] != 0.

So maybe change the generic bindings like this:

touchscreen-min-x: minimum value reported by X axis ADC (default 0)
touchscreen-max-x: maximum value reported by Y axis ADC
touchscreen-min-y: minimum value reported by Y axis ADC (default 0)
touchscreen-max-y: maximum value reported by Y axis ADC
touchscreen-size-x: deprecated alias for touchscreen-max-x
touchscreen-size-y: deprecated alias for touchscreen-max-y

Quoted 1 line

So ti,max-[xy] is basically the same as touchscreen-size-[xy],

No it is not the same and should be kept separate.

Quoted 1 line

except, that the generic bindings don't support min-[xy] != 0.

What would be the purpose of this? Every user-space I know about (X11, Replicant) expects coordinates in some range 0..max so setting min in device tree makes no sense to me.

Quoted 10 lines

So maybe change the generic bindings like this:

touchscreen-min-x: minimum value reported by X axis ADC (default 0)
touchscreen-max-x: maximum value reported by Y axis ADC
touchscreen-min-y: minimum value reported by Y axis ADC (default 0)
touchscreen-max-y: maximum value reported by Y axis ADC
touchscreen-size-x: deprecated alias for touchscreen-max-x
touchscreen-size-y: deprecated alias for touchscreen-max-y

Initially I had thought about this but it does not solve the problems with touch pre-calibration. Since it mixes raw coordinates with system coordinates.

To achieve the goal of having a roughly precalibrated touch which should provide (0,0) at the lower left corner and (touchscreen-size-x,touchscreen-size-y) in pixel coordinates of the panel. Hence it roughly works without a calibration matrix in user space (e.g. xorg.conf or Replicant).

Why do we need pre-calibration? Because some systems might need touch interaction before they can offer (force) the user into a touch calibration step. We use these drivers and approach in our production kernels for GTA04, OpenPandora and Pyra for a while and nobody was even missing a user-space calibration tool any more.

The underlaying problem is that you can have the same controller chip in different board designs and there are different touch panel types. Each one has certain physical properties but they can differ. But you certainly want touchscreen-size-x/y to be a constant.

Now if we make touchscreen-max-x/y the same as touchscreen-size-x/y and change the panel, we have to adjust user space transformation each time we change the panel. This does not seem to be right and can be done better by keeping them separately.

This is what this approach does: the roughly correct scaling of raw values to pixel values.

ti,min-x -> 0
...
x -> some value between 0 and touchscreen-size-x

                calculated by
			touchscreen-size-x * (x - min-x) / (max-x - min-x)

...
ti,max-x -> touchscreen-size-x

Hence the ti,min/max values describe the range of expected input values from the ADC and the touchscreen-size-x describes the touch in LCD pixels passed as input events.

Example:

ti,min-x = 64
ti,max-x = 4016
touchscreen-size-x = 480

If we change the panel type which presents a slightly different ADC range:

ti,min-x = 100
ti,max-x = 3900
touchscreen-size-x = 480

and we still get a coordinate range (0 .. 480).

Note that this feature can be effectively disabled if ti,min-x=0 and ti,max-x=4095 and touchscreen-size-x=4095, i.e. reports the full range of ADC values because then it multiplies by 1.

Our proposed driver does use these values if they are missing from DT and therefore it should not break old DT files which expect raw values to be reported.

I hope this clarifies what we need to achieve and you can agree.

Am 30.09.2016 um 16:16 schrieb Sebastian Reichel <[email protected]>:

Hi,

Quoted 21 lines · H. Nikolaus Schaller

Quoted 1 line

So ti,max-[xy] is basically the same as touchscreen-size-[xy],

No it is not the same and should be kept separate.

Quoted 1 line

except, that the generic bindings don't support min-[xy] != 0.

What would be the purpose of this? Every user-space I know about (X11, Replicant) expects coordinates in some range 0..max so setting min in device tree makes no sense to me.

Quoted 10 lines

So maybe change the generic bindings like this:

touchscreen-min-x: minimum value reported by X axis ADC (default 0)
touchscreen-max-x: maximum value reported by Y axis ADC
touchscreen-min-y: minimum value reported by Y axis ADC (default 0)
touchscreen-max-y: maximum value reported by Y axis ADC
touchscreen-size-x: deprecated alias for touchscreen-max-x
touchscreen-size-y: deprecated alias for touchscreen-max-y

Quoted 3 lines

Initially I had thought about this but it does not solve the problems with touch pre-calibration. Since it mixes raw coordinates with system coordinates.

touchscreen-size-x was actually refering to your definition of touchscreen-max-x and not system coordinates.

For that it would
make much more sense to use a phandle to the screen IMHO.

Quoted 6 lines

To achieve the goal of having a roughly precalibrated touch which should provide (0,0) at the lower left corner and (touchscreen-size-x,touchscreen-size-y) in pixel coordinates of the panel. Hence it roughly works without a calibration matrix in user space (e.g. xorg.conf or Replicant).

well I did not mean to use touchscreen-size-x/y for describing the size of screen, as visible in n900.dts (first implementation of the common binding), which sets the value to 4096.

Quoted 6 lines

Why do we need pre-calibration? Because some systems might need touch interaction before they can offer (force) the user into a touch calibration step. We use these drivers and approach in our production kernels for GTA04, OpenPandora and Pyra for a while and nobody was even missing a user-space calibration tool any more.

I have nothing against the feature. OTOH I'm quite in of kernel based TS calibration. Note, that you can only add it for hardware without pre-existing touchscreen support, since you break peoples systems otherwise (We have that problem for N900).

Quoted 17 lines

The underlaying problem is that you can have the same controller chip in different board designs and there are different touch panel types. Each one has certain physical properties but they can differ. But you certainly want touchscreen-size-x/y to be a constant.

Now if we make touchscreen-max-x/y the same as touchscreen-size-x/y and change the panel, we have to adjust user space transformation each time we change the panel. This does not seem to be right and can be done better by keeping them separately.

This is what this approach does: the roughly correct scaling of raw values to pixel values.

ti,min-x -> 0
...
x -> some value between 0 and touchscreen-size-x

Quoted 8 lines

               calculated by
			touchscreen-size-x * (x - min-x) / (max-x - min-x)

...
ti,max-x -> touchscreen-size-x

Hence the ti,min/max values describe the range of expected input values from the ADC and the touchscreen-size-x describes the touch in LCD pixels passed as input events.

so basically you use touchscreen-size-x to describe the screen and not the touchscreen.

When I added it, I did mean the max ADC value.
Actually I was under the impression, that X drivers would scale this to screen size automatically.

Since all my touchscreen HW required
calibration I did never test this, though.

Quoted 18 lines

Example:

ti,min-x = 64
ti,max-x = 4016
touchscreen-size-x = 480

If we change the panel type which presents a slightly different ADC range:

ti,min-x = 100
ti,max-x = 3900
touchscreen-size-x = 480

and we still get a coordinate range (0 .. 480).

Note that this feature can be effectively disabled if ti,min-x=0 and ti,max-x=4095 and touchscreen-size-x=4095, i.e. reports the full range of ADC values because then it multiplies by 1.

Quoted 6 lines

Our proposed driver does use these values if they are missing from DT and therefore it should not break old DT files which expect raw values to be reported.

I hope this clarifies what we need to achieve and you can agree.

I did understand what you want, but I disagreed about using touchscreen-size-x/y for system coordinates. I now see, that it's too late for that, as other people already did so.

I do agree with Rob, that the ti,min/max-x/y should become common, though. Also I would do s/minimum value/minimum raw value/g.

Additionally touchscreen-size-x/y should mention, that it's used to scale the raw values.

Am 30.09.2016 um 16:40 schrieb H. Nikolaus Schaller <[email protected]>:

Quoted 3 lines

I do agree with Rob, that the ti,min/max-x/y should become common, though. Also I would do s/minimum value/minimum raw value/g.

Yes, that is a good idea!

To me it seems as if "minimum-raw-value-x", "maximum-raw-value-y" could be good.

Or is the string too long (then we can drop the "-value" or the "imum")? It might blow up the DTB and string constant for finding the property?

If we come to some consensus on this, I can update the patch set.

Am 24.09.2016 um 00:47 schrieb Rob Herring <[email protected]>:

Quoted 19 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

Quoted 2 lines

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

Seems like you are breaking compatibility with old DTs. I can't tell for sure though.

Quoted 36 lines

[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

Signed-off-by H. Nikolaus Schaller <[email protected]>

---

.../bindings/input/touchscreen/tsc2007.txt         |  20 ++--
drivers/input/touchscreen/tsc2007.c                | 126 +++++++++++++++++----
include/linux/i2c/tsc2007.h                        |   8 ++
3 files changed, 123 insertions(+), 31 deletions(-)

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +0 -1

index ec365e1..6e9fd55 100644
@@ -6,6 +6,7 @@ Required properties:
- ti,x-plate-ohms: X-plate resistance in ohms.
 

Optional properties:
+- generic touch screen properties: see touchscreen binding [2].
- gpios: the interrupt gpio the chip is connected to (trough the penirq pin). The penirq pin goes to low when the panel is touched.
(see GPIO binding[1] for more details).

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +3 -7

@@ -13,17 +14,20 @@ Optional properties:
  (see interrupt binding[0]).
- interrupts: (gpio) interrupt to which the chip is connected
  (see interrupt binding[0]).
-- ti,max-rt: maximum pressure.
-- ti,fuzzx: specifies the absolute input fuzz x value.
-  If set, it will permit noise in the data up to +- the value given to the fuzz
-  parameter, that is used to filter noise from the event stream.
-- ti,fuzzy: specifies the absolute input fuzz y value.
-- ti,fuzzz: specifies the absolute input fuzz z value.
+- ti,max-rt: maximum pressure resistance above which samples are ignored
+  (default: 4095).
+- ti,report-resistance: report resistance (no pressure = max_rt) instead

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt Optional properties: · line 24

Quoted 5 lines

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +5 -0

+  of pressure (no pressure = 0).
+- ti,min-x: minimum value reported by X axis ADC (default 0).
+- ti,max-x: maximum value reported by X axis ADC (default 4095).
+- ti,min-y: minimum value reported by Y axis ADC (default 0).
+- ti,max-y: maximum value reported by Y axis ADC (default 4095).

Seems like these could be common too. They make more sense than giving x and y sizes in pixel units which really should come from the panel.

Quoted 19 lines

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +1 -2

- ti,poll-period: how much time to wait (in milliseconds) before reading again the
-  values from the tsc2007.
+  values from the tsc2007 (default 1).
 

[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt
+[2]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

Example:

	&i2c1 {

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +2 -0

@@ -35,6 +39,8 @@ Example:
 			interrupts = <0x0 0x8>;
 			gpios = <&gpio4 0 0>;
 			ti,x-plate-ohms = <180>;
+			touchscreen-size-x = <640>;
+			touchscreen-size-y = <480>;
 		};
 

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt Example: · line 46

Quoted 1 line

Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt +0 -0

 		/* ... */

Am 30.09.2016 um 18:23 schrieb Christ van Willegen <[email protected]>:

Hi,

I saw this earlier, but didn't think it important to mention, but:

drivers/input/touchscreen/tsc2007.c static irqreturn_t tsc2007_soft_irq(int irq, void *handle) · line 223

Quoted 12 lines · H. Nikolaus Schaller

drivers/input/touchscreen/tsc2007.c +7 -0

index 5d0cd51..9a11509 100644
 
+                       /* scale ADC values to desired output range */
+                       sx = (ts->prop.max_x * (tc.x - ts->min_x))
+                               / (ts->max_x - ts->min_x);
+                       sy = (ts->prop.max_y * (tc.y - ts->min_y))
+                               / (ts->max_y - ts->min_y);
+                       rt = (input->absinfo[ABS_PRESSURE].maximum * rt) /
+                               ts->max_rt;

If ts->max_rt is zero, or ts->max_x == ts->min_x, or ts->max_y == ts->min_y, these yield a division by zero error.

Ofcourse, this is under control of the DT-maintainer(s) of the device (if I'm not mistaking)

, but throwing an error on DT parse if (one of)
these condition(s) is met yields better info to the DT-maintainer than a /0 error...

If you (or others) agree, could you add this to the patch?

The tsc2007 chip not only has a resistive touch screen controller but also an external AUX adc imput which can be used for an ambient light sensor, battery voltage monitoring or any general purpose.

Additionally it can measure the chip temperature.

This extension provides an iio interface for these adc channels in addition to the raw x, y, z values and the estimated touch screen resistance. This can be used for debugging or special applications.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 drivers/input/touchscreen/Kconfig   |   1 +
 drivers/input/touchscreen/tsc2007.c | 137 +++++++++++++++++++++++++++++++++++-
 2 files changed, 136 insertions(+), 2 deletions(-)

drivers/input/touchscreen/Kconfig +0 -0

index 2fb1f43..84c28e8 100644

drivers/input/touchscreen/tsc2007.c +16 -0

+#include <linux/iio/driver.h>
 
 #define TSC2007_MEASURE_TEMP0		(0x0 << 4)
 #define TSC2007_MEASURE_AUX		(0x2 << 4)

@@ -61,6 +64,16 @@
 #define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
 #define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
 
+#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
+{ \
+	.datasheet_name = _name, \
+	.type = _type, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+			BIT(_chan_info), \
+	.indexed = 1, \
+	.channel = _chan, \
+}
+
 struct ts_event {
 	u16	x;
 	u16	y;

@@ -69,9 +82,11 @@ struct ts_event {
 
 struct tsc2007 {
 	struct input_dev	*input;
+	struct iio_dev		*indio;
 	char			phys[32];
 
 	struct i2c_client	*client;
+	struct mutex		mlock;
 
 	u16			model;
 	u16			x_plate_ohms;

@@ -192,7 +207,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
 
 		/* pen is down, continue with the measurement */
+
+		mutex_lock(&ts->mlock);
 		tsc2007_read_values(ts, &tc);
+		mutex_unlock(&ts->mlock);
 
 		rt = tsc2007_calculate_resistance(ts, &tc);
 

drivers/input/touchscreen/tsc2007.c +106 -2

+};
+
+static int tsc2007_read_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int *val, int *val2, long mask)
+{
+	struct  tsc2007 *tsc = iio_priv(indio_dev);
+	int adc_chan = chan->channel;
+	int ret = 0;
+
+	if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
+		return -EINVAL;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	mutex_lock(&tsc->mlock);
+
+	switch (chan->channel) {
+	case 0:
+		*val = tsc2007_xfer(tsc, READ_X);
+		break;
+	case 1:
+		*val = tsc2007_xfer(tsc, READ_Y);
+		break;
+	case 2:
+		*val = tsc2007_xfer(tsc, READ_Z1);
+		break;
+	case 3:
+		*val = tsc2007_xfer(tsc, READ_Z2);
+		break;
+	case 4:
+		*val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
+		break;
+	case 5: {
+		struct ts_event tc;
+
+		tc.x = tsc2007_xfer(tsc, READ_X);
+		tc.z1 = tsc2007_xfer(tsc, READ_Z1);
+		tc.z2 = tsc2007_xfer(tsc, READ_Z2);
+		*val = tsc2007_calculate_resistance(tsc, &tc);
+		break;
+	}
+	case 6:
+		*val = tsc2007_is_pen_down(tsc);
+		break;
+	case 7:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
+		break;
+	case 8:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
+		break;
+	}
+
+	/* Prepare for next touch reading - power down ADC, enable PENIRQ */
+	tsc2007_xfer(tsc, PWRDOWN);
+
+	mutex_unlock(&tsc->mlock);
+
+	ret = IIO_VAL_INT;
+
+	return ret;
+}
+
+static const struct iio_info tsc2007_iio_info = {
+	.read_raw = tsc2007_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
 #ifdef CONFIG_OF
 static int tsc2007_get_pendown_state_gpio(struct device *dev)
 {

@@ -453,15 +551,20 @@ static int tsc2007_probe(struct i2c_client *client,
 	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct tsc2007 *ts;
 	struct input_dev *input_dev;
+	struct iio_dev *indio_dev;
 	int err;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_READ_WORD_DATA))
 		return -EIO;
 
-	ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
-	if (!ts)
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*ts));
+	if (!indio_dev) {
+		dev_err(&client->dev, "iio_device_alloc failed\n");
 		return -ENOMEM;
+	}
+
+	ts = iio_priv(indio_dev);
 
 	input_dev = devm_input_allocate_device(&client->dev);
 	if (!input_dev)

@@ -469,10 +572,26 @@ static int tsc2007_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, ts);
 
+	indio_dev->name = "tsc2007";
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &tsc2007_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = tsc2007_iio_channel;
+	indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
+
+	err = iio_device_register(indio_dev);
+	if (err < 0) {
+		dev_err(&client->dev, "iio_device_register() failed: %d\n",
+			err);
+		return err;
+	}
+
 	ts->client = client;
 	ts->irq = client->irq;
 	ts->input = input_dev;
+	ts->indio = indio_dev;
 	init_waitqueue_head(&ts->wait);
+	mutex_init(&ts->mlock);
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));

@@ -548,6 +667,19 @@ static int tsc2007_probe(struct i2c_client *client,
 	return 0;
 }
 
+static int tsc2007_remove(struct i2c_client *client)
+{
+	struct tsc2007 *ts = i2c_get_clientdata(client);
+	struct input_dev *input_dev = ts->input;
+	struct iio_dev *indio_dev = ts->indio;
+
+	input_unregister_device(input_dev);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
 static const struct i2c_device_id tsc2007_idtable[] = {
 	{ "tsc2007", 0 },
 	{ }

Am 24.09.2016 um 18:07 schrieb Jonathan Cameron <[email protected]>:

Quoted 4 lines · H. Nikolaus Schaller

The tsc2007 chip not only has a resistive touch screen controller but also an external AUX adc imput which can be used for an ambient light sensor, battery voltage monitoring or any general purpose.

Quoted 6 lines

Additionally it can measure the chip temperature.

This extension provides an iio interface for these adc channels in addition to the raw x, y, z values and the estimated touch screen resistance. This can be used for debugging or special applications.

I'm unconvinced that exposing the touch related channels is terribly useful.

Fair enough on the aux channel and temperature though.

Otherwise a few minor bits and bobs.

Jonathan

Quoted 7 lines

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
drivers/input/touchscreen/Kconfig | 1 +
drivers/input/touchscreen/tsc2007.c | 137 +++++++++++++++++++++++++++++++++++-

2 files changed, 136 insertions(+), 2 deletions(-)

drivers/input/touchscreen/Kconfig config TOUCHSCREEN_TSC2005 · line 1022

Quoted 9 lines

drivers/input/touchscreen/Kconfig +0 -0

index 2fb1f43..84c28e8 100644
@@ -1019,6 +1019,7 @@ config TOUCHSCREEN_TSC2005

config TOUCHSCREEN_TSC2007
tristate "TSC2007 based touchscreens"
depends on I2C

drivers/input/touchscreen/Kconfig +1 -0

+	select IIO

This is pulling quite a bit of core IIO code in for a feature a lot of users of this chip won't care about...

drivers/input/touchscreen/tsc2007.c line 34

Quoted 13 lines

help
Say Y here if you have a TSC2007 based touchscreen.

drivers/input/touchscreen/tsc2007.c +2 -0

index e9d5086..559cbc6 100644
@@ -30,6 +30,9 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/input/touchscreen.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>

Why machine.h? (or driver.h for that reason).

drivers/input/touchscreen/tsc2007.c line 35

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +1 -0

+#include <linux/iio/driver.h>

Quoted 47 lines

drivers/input/touchscreen/tsc2007.c +15 -0

 #define TSC2007_MEASURE_TEMP0		(0x0 << 4)
 #define TSC2007_MEASURE_AUX		(0x2 << 4)

@@ -61,6 +64,16 @@
 #define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
 #define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
 
+#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
+{ \
+	.datasheet_name = _name, \
+	.type = _type, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+			BIT(_chan_info), \
+	.indexed = 1, \
+	.channel = _chan, \
+}
+
 struct ts_event {
 	u16	x;
 	u16	y;

@@ -69,9 +82,11 @@ struct ts_event {
 
 struct tsc2007 {
 	struct input_dev	*input;
+	struct iio_dev		*indio;
 	char			phys[32];
 
 	struct i2c_client	*client;
+	struct mutex		mlock;
 
 	u16			model;
 	u16			x_plate_ohms;

@@ -192,7 +207,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
 
 		/* pen is down, continue with the measurement */
+
+		mutex_lock(&ts->mlock);
 		tsc2007_read_values(ts, &tc);
+		mutex_unlock(&ts->mlock);
 
 		rt = tsc2007_calculate_resistance(ts, &tc);
 

@@ -319,6 +337,86 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 346

Quoted 7 lines

drivers/input/touchscreen/tsc2007.c +7 -0

+static const struct iio_chan_spec tsc2007_iio_channel[] = {
+	TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */

This one needs some explanation. What is it measuring?

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 347

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +1 -0

+	TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),

At the moment this is defined as barometric pressure.

I suppose there
is no reason it can't extend to touch pressure though, even if it's a boolean like this...

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 349

Quoted 3 lines

drivers/input/touchscreen/tsc2007.c +2 -0

+	TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),

I would be tempted to only expose those channels that have a meaning outside of touch screen usage (e.g. adc, temp0 and temp1).

Quoted 150 lines

drivers/input/touchscreen/tsc2007.c +107 -2

+};
+
+static int tsc2007_read_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int *val, int *val2, long mask)
+{
+	struct  tsc2007 *tsc = iio_priv(indio_dev);
+	int adc_chan = chan->channel;
+	int ret = 0;
+
+	if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
+		return -EINVAL;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	mutex_lock(&tsc->mlock);
+
+	switch (chan->channel) {
+	case 0:
+		*val = tsc2007_xfer(tsc, READ_X);
+		break;
+	case 1:
+		*val = tsc2007_xfer(tsc, READ_Y);
+		break;
+	case 2:
+		*val = tsc2007_xfer(tsc, READ_Z1);
+		break;
+	case 3:
+		*val = tsc2007_xfer(tsc, READ_Z2);
+		break;
+	case 4:
+		*val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
+		break;
+	case 5: {
+		struct ts_event tc;
+
+		tc.x = tsc2007_xfer(tsc, READ_X);
+		tc.z1 = tsc2007_xfer(tsc, READ_Z1);
+		tc.z2 = tsc2007_xfer(tsc, READ_Z2);
+		*val = tsc2007_calculate_resistance(tsc, &tc);
+		break;
+	}
+	case 6:
+		*val = tsc2007_is_pen_down(tsc);
+		break;
+	case 7:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
+		break;
+	case 8:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
+		break;
+	}
+
+	/* Prepare for next touch reading - power down ADC, enable PENIRQ */
+	tsc2007_xfer(tsc, PWRDOWN);
+
+	mutex_unlock(&tsc->mlock);
+
+	ret = IIO_VAL_INT;
+
+	return ret;
+}
+
+static const struct iio_info tsc2007_iio_info = {
+	.read_raw = tsc2007_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
 #ifdef CONFIG_OF
 static int tsc2007_get_pendown_state_gpio(struct device *dev)
 {

@@ -453,15 +551,20 @@ static int tsc2007_probe(struct i2c_client *client,
 	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct tsc2007 *ts;
 	struct input_dev *input_dev;
+	struct iio_dev *indio_dev;
 	int err;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_READ_WORD_DATA))
 		return -EIO;
 
-	ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
-	if (!ts)
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*ts));
+	if (!indio_dev) {
+		dev_err(&client->dev, "iio_device_alloc failed\n");
 		return -ENOMEM;
+	}
+
+	ts = iio_priv(indio_dev);
 
 	input_dev = devm_input_allocate_device(&client->dev);
 	if (!input_dev)

@@ -469,10 +572,26 @@ static int tsc2007_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, ts);
 
+	indio_dev->name = "tsc2007";
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &tsc2007_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = tsc2007_iio_channel;
+	indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
+
+	err = iio_device_register(indio_dev);
+	if (err < 0) {
+		dev_err(&client->dev, "iio_device_register() failed: %d\n",
+			err);
+		return err;
+	}
+
 	ts->client = client;
 	ts->irq = client->irq;
 	ts->input = input_dev;
+	ts->indio = indio_dev;
 	init_waitqueue_head(&ts->wait);
+	mutex_init(&ts->mlock);
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));

@@ -548,6 +667,19 @@ static int tsc2007_probe(struct i2c_client *client,
 	return 0;
 }
 
+static int tsc2007_remove(struct i2c_client *client)
+{
+	struct tsc2007 *ts = i2c_get_clientdata(client);
+	struct input_dev *input_dev = ts->input;
+	struct iio_dev *indio_dev = ts->indio;
+
+	input_unregister_device(input_dev);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
 static const struct i2c_device_id tsc2007_idtable[] = {
 	{ "tsc2007", 0 },
 	{ }

@@ -570,6 +702,7 @@ static struct i2c_driver tsc2007_driver = {
 	},
 	.id_table	= tsc2007_idtable,
 	.probe		= tsc2007_probe,
+	.remove		= tsc2007_remove,
 };
 

drivers/input/touchscreen/tsc2007.c static struct i2c_driver tsc2007_driver = { · line 708

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +0 -0

 module_i2c_driver(tsc2007_driver);

Hi Jonathan,

Quoted 13 lines

Am 24.09.2016 um 18:07 schrieb Jonathan Cameron <[email protected]>:

Quoted 10 lines · H. Nikolaus Schaller

The tsc2007 chip not only has a resistive touch screen controller but also an external AUX adc imput which can be used for an ambient light sensor, battery voltage monitoring or any general purpose.

Additionally it can measure the chip temperature.

This extension provides an iio interface for these adc channels in addition to the raw x, y, z values and the estimated touch screen resistance. This can be used for debugging or special applications.

Quoted 2 lines

I'm unconvinced that exposing the touch related channels is terribly useful.

Mostly for debugging of if someone wants to do a (better) touch detection in user space.

Quoted 13 lines

Fair enough on the aux channel and temperature though.

Otherwise a few minor bits and bobs.

Jonathan

Quoted 7 lines

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
drivers/input/touchscreen/Kconfig | 1 +
drivers/input/touchscreen/tsc2007.c | 137 +++++++++++++++++++++++++++++++++++-

2 files changed, 136 insertions(+), 2 deletions(-)

Quoted 11 lines

drivers/input/touchscreen/Kconfig config TOUCHSCREEN_TSC2005 · line 1022

Quoted 9 lines

drivers/input/touchscreen/Kconfig +0 -0

index 2fb1f43..84c28e8 100644
@@ -1019,6 +1019,7 @@ config TOUCHSCREEN_TSC2005

config TOUCHSCREEN_TSC2007
tristate "TSC2007 based touchscreens"
depends on I2C

drivers/input/touchscreen/Kconfig +1 -0

+	select IIO

This is pulling quite a bit of core IIO code in for a feature a lot of users of this chip won't care about...

Ah, yes.

We didn't notice because we use many other IIO devices on the same board (GTA04).

So I will add some #ifdef CONFIG_IIO where necessary.

Quoted 5 lines

Quoted 5 lines

help
Say Y here if you have a TSC2007 based touchscreen.

Quoted 11 lines

drivers/input/touchscreen/tsc2007.c line 34

Quoted 10 lines

drivers/input/touchscreen/tsc2007.c +2 -0

index e9d5086..559cbc6 100644
@@ -30,6 +30,9 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/input/touchscreen.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>

Why machine.h? (or driver.h for that reason).

Good question. Header files sometimes get in during development and are forgotten to take out... Or had been necessary when development started a while ago. Indeed I don't know why they are there.

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c line 35

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +1 -0

+#include <linux/iio/driver.h>

I have checked and we really neither need machine.h nor driver.h.

Quoted 47 lines

Quoted 47 lines

drivers/input/touchscreen/tsc2007.c +15 -0

 #define TSC2007_MEASURE_TEMP0		(0x0 << 4)
 #define TSC2007_MEASURE_AUX		(0x2 << 4)

@@ -61,6 +64,16 @@
 #define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
 #define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
 
+#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
+{ \
+	.datasheet_name = _name, \
+	.type = _type, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+			BIT(_chan_info), \
+	.indexed = 1, \
+	.channel = _chan, \
+}
+
 struct ts_event {
 	u16	x;
 	u16	y;

@@ -69,9 +82,11 @@ struct ts_event {
 
 struct tsc2007 {
 	struct input_dev	*input;
+	struct iio_dev		*indio;
 	char			phys[32];
 
 	struct i2c_client	*client;
+	struct mutex		mlock;
 
 	u16			model;
 	u16			x_plate_ohms;

@@ -192,7 +207,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
 
 		/* pen is down, continue with the measurement */
+
+		mutex_lock(&ts->mlock);
 		tsc2007_read_values(ts, &tc);
+		mutex_unlock(&ts->mlock);
 
 		rt = tsc2007_calculate_resistance(ts, &tc);
 

@@ -319,6 +337,86 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 

Quoted 8 lines

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 346

Quoted 7 lines

drivers/input/touchscreen/tsc2007.c +7 -0

+static const struct iio_chan_spec tsc2007_iio_channel[] = {
+	TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */

This one needs some explanation. What is it measuring?

Well, it is reporting the raw conductivity of the touch under certain conditions. But basically it is also a voltage measured by the ADC.

The real pressure value reported to user space is calculated using this and the z1 and z2 values.

If not pressed, conductivity is 0 and if lightly touched it jumps to some base value. When more pressed, conductivity increases while resistance goes down to some 300 Ohms or so.

But I have not found an IIO_OHM or IIO_MHO constant. Which would also not be the best description because it is not scaled to real ohms.

Quoted 3 lines

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 347

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +1 -0

+	TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),

At the moment this is defined as barometric pressure.

Ok, I see.

Quoted 3 lines

I suppose there
is no reason it can't extend to touch pressure though, even if it's a boolean like this...

So let's keep it this way?

Quoted 6 lines

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 349

Quoted 4 lines

drivers/input/touchscreen/tsc2007.c +2 -0

+	TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),

I would be tempted to only expose those channels that have a meaning outside of touch screen usage (e.g. adc, temp0 and temp1).

Well, for debugging and checking for non-responding or defective touch it is very handy to have them in /sys/bus/iio for direct inspection. For example we run a device test tool reading such values.

You might argue that we should run our own fork for this purpose to provide a patched driver. But we want people to use the kernel from kernel.org and still be able to do such debugging without asking them to replace the kernel first.

Quoted 150 lines

Quoted 150 lines

drivers/input/touchscreen/tsc2007.c +107 -2

+};
+
+static int tsc2007_read_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int *val, int *val2, long mask)
+{
+	struct  tsc2007 *tsc = iio_priv(indio_dev);
+	int adc_chan = chan->channel;
+	int ret = 0;
+
+	if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
+		return -EINVAL;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	mutex_lock(&tsc->mlock);
+
+	switch (chan->channel) {
+	case 0:
+		*val = tsc2007_xfer(tsc, READ_X);
+		break;
+	case 1:
+		*val = tsc2007_xfer(tsc, READ_Y);
+		break;
+	case 2:
+		*val = tsc2007_xfer(tsc, READ_Z1);
+		break;
+	case 3:
+		*val = tsc2007_xfer(tsc, READ_Z2);
+		break;
+	case 4:
+		*val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
+		break;
+	case 5: {
+		struct ts_event tc;
+
+		tc.x = tsc2007_xfer(tsc, READ_X);
+		tc.z1 = tsc2007_xfer(tsc, READ_Z1);
+		tc.z2 = tsc2007_xfer(tsc, READ_Z2);
+		*val = tsc2007_calculate_resistance(tsc, &tc);
+		break;
+	}
+	case 6:
+		*val = tsc2007_is_pen_down(tsc);
+		break;
+	case 7:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
+		break;
+	case 8:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
+		break;
+	}
+
+	/* Prepare for next touch reading - power down ADC, enable PENIRQ */
+	tsc2007_xfer(tsc, PWRDOWN);
+
+	mutex_unlock(&tsc->mlock);
+
+	ret = IIO_VAL_INT;
+
+	return ret;
+}
+
+static const struct iio_info tsc2007_iio_info = {
+	.read_raw = tsc2007_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
 #ifdef CONFIG_OF
 static int tsc2007_get_pendown_state_gpio(struct device *dev)
 {

@@ -453,15 +551,20 @@ static int tsc2007_probe(struct i2c_client *client,
 	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct tsc2007 *ts;
 	struct input_dev *input_dev;
+	struct iio_dev *indio_dev;
 	int err;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_READ_WORD_DATA))
 		return -EIO;
 
-	ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
-	if (!ts)
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*ts));
+	if (!indio_dev) {
+		dev_err(&client->dev, "iio_device_alloc failed\n");
 		return -ENOMEM;
+	}
+
+	ts = iio_priv(indio_dev);
 
 	input_dev = devm_input_allocate_device(&client->dev);
 	if (!input_dev)

@@ -469,10 +572,26 @@ static int tsc2007_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, ts);
 
+	indio_dev->name = "tsc2007";
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &tsc2007_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = tsc2007_iio_channel;
+	indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
+
+	err = iio_device_register(indio_dev);
+	if (err < 0) {
+		dev_err(&client->dev, "iio_device_register() failed: %d\n",
+			err);
+		return err;
+	}
+
 	ts->client = client;
 	ts->irq = client->irq;
 	ts->input = input_dev;
+	ts->indio = indio_dev;
 	init_waitqueue_head(&ts->wait);
+	mutex_init(&ts->mlock);
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));

@@ -548,6 +667,19 @@ static int tsc2007_probe(struct i2c_client *client,
 	return 0;
 }
 
+static int tsc2007_remove(struct i2c_client *client)
+{
+	struct tsc2007 *ts = i2c_get_clientdata(client);
+	struct input_dev *input_dev = ts->input;
+	struct iio_dev *indio_dev = ts->indio;
+
+	input_unregister_device(input_dev);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
 static const struct i2c_device_id tsc2007_idtable[] = {
 	{ "tsc2007", 0 },
 	{ }

@@ -570,6 +702,7 @@ static struct i2c_driver tsc2007_driver = {
 	},
 	.id_table	= tsc2007_idtable,
 	.probe		= tsc2007_probe,
+	.remove		= tsc2007_remove,
 };
 

Quoted 3 lines

drivers/input/touchscreen/tsc2007.c static struct i2c_driver tsc2007_driver = { · line 708

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +0 -0

 module_i2c_driver(tsc2007_driver);

BR and thanks,
Nikolaus

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Am 24.09.2016 um 19:26 schrieb Jonathan Cameron <[email protected]>:

Quoted 15 lines · H. Nikolaus Schaller

Hi Jonathan,

Quoted 13 lines

Am 24.09.2016 um 18:07 schrieb Jonathan Cameron <[email protected]>:

Quoted 10 lines · H. Nikolaus Schaller

The tsc2007 chip not only has a resistive touch screen controller but also an external AUX adc imput which can be used for an ambient light sensor, battery voltage monitoring or any general purpose.

Additionally it can measure the chip temperature.

This extension provides an iio interface for these adc channels in addition to the raw x, y, z values and the estimated touch screen resistance. This can be used for debugging or special applications.

Quoted 5 lines

Quoted 2 lines

I'm unconvinced that exposing the touch related channels is terribly useful.

Mostly for debugging of if someone wants to do a (better) touch detection in user space.

Not going to do anything at the rates you will get out of that interface.

I suspect this is a non starter. Lets see what Dmitry thinks.

Jonathan

Quoted 25 lines

Quoted 24 lines

Fair enough on the aux channel and temperature though.

Otherwise a few minor bits and bobs.

Jonathan

Quoted 15 lines

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
drivers/input/touchscreen/Kconfig | 1 +
drivers/input/touchscreen/tsc2007.c | 137 +++++++++++++++++++++++++++++++++++-

2 files changed, 136 insertions(+), 2 deletions(-)

drivers/input/touchscreen/Kconfig +0 -0

index 2fb1f43..84c28e8 100644
@@ -1019,6 +1019,7 @@ config TOUCHSCREEN_TSC2005

config TOUCHSCREEN_TSC2007
tristate "TSC2007 based touchscreens"
depends on I2C

drivers/input/touchscreen/Kconfig config TOUCHSCREEN_TSC2005 · line 1022

Quoted 1 line

drivers/input/touchscreen/Kconfig +1 -0

+	select IIO

This is pulling quite a bit of core IIO code in for a feature a lot of users of this chip won't care about...

Quoted 5 lines

Ah, yes.

We didn't notice because we use many other IIO devices on the same board (GTA04).

So I will add some #ifdef CONFIG_IIO where necessary.

Cool

Quoted 100 lines

Quoted 5 lines

Quoted 5 lines

help
Say Y here if you have a TSC2007 based touchscreen.

Quoted 11 lines

drivers/input/touchscreen/tsc2007.c line 34

Quoted 10 lines

drivers/input/touchscreen/tsc2007.c +2 -0

index e9d5086..559cbc6 100644
@@ -30,6 +30,9 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/input/touchscreen.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>

Why machine.h? (or driver.h for that reason).

Good question. Header files sometimes get in during development and are forgotten to take out... Or had been necessary when development started a while ago. Indeed I don't know why they are there.

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c line 35

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +1 -0

+#include <linux/iio/driver.h>

I have checked and we really neither need machine.h nor driver.h.

Quoted 47 lines

Quoted 47 lines

drivers/input/touchscreen/tsc2007.c +15 -0

 #define TSC2007_MEASURE_TEMP0		(0x0 << 4)
 #define TSC2007_MEASURE_AUX		(0x2 << 4)

@@ -61,6 +64,16 @@
 #define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
 #define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
 
+#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
+{ \
+	.datasheet_name = _name, \
+	.type = _type, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+			BIT(_chan_info), \
+	.indexed = 1, \
+	.channel = _chan, \
+}
+
 struct ts_event {
 	u16	x;
 	u16	y;

@@ -69,9 +82,11 @@ struct ts_event {
 
 struct tsc2007 {
 	struct input_dev	*input;
+	struct iio_dev		*indio;
 	char			phys[32];
 
 	struct i2c_client	*client;
+	struct mutex		mlock;
 
 	u16			model;
 	u16			x_plate_ohms;

@@ -192,7 +207,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
 
 		/* pen is down, continue with the measurement */
+
+		mutex_lock(&ts->mlock);
 		tsc2007_read_values(ts, &tc);
+		mutex_unlock(&ts->mlock);
 
 		rt = tsc2007_calculate_resistance(ts, &tc);
 

@@ -319,6 +337,86 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 

Quoted 8 lines

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 346

Quoted 7 lines

drivers/input/touchscreen/tsc2007.c +7 -0

+static const struct iio_chan_spec tsc2007_iio_channel[] = {
+	TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */

This one needs some explanation. What is it measuring?

Well, it is reporting the raw conductivity of the touch under certain conditions. But basically it is also a voltage measured by the ADC.

The real pressure value reported to user space is calculated using this and the z1 and z2 values.

If not pressed, conductivity is 0 and if lightly touched it jumps to some base value. When more pressed, conductivity increases while resistance goes down to some 300 Ohms or so.

But I have not found an IIO_OHM or IIO_MHO constant. Which would also not be the best description because it is not scaled to real ohms.

Quoted 3 lines

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 347

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +1 -0

+	TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),

At the moment this is defined as barometric pressure.

Quoted 7 lines

Ok, I see.

Quoted 3 lines

I suppose there
is no reason it can't extend to touch pressure though, even if it's a boolean like this...

So let's keep it this way?

Except I'm saying we should drop it entirely.

Quoted 7 lines

Quoted 6 lines

drivers/input/touchscreen/tsc2007.c static void tsc2007_close(struct input_dev *input_dev) · line 349

Quoted 4 lines

drivers/input/touchscreen/tsc2007.c +2 -0

+	TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),

I would be tempted to only expose those channels that have a meaning outside of touch screen usage (e.g. adc, temp0 and temp1).

Quoted 9 lines

Well, for debugging and checking for non-responding or defective touch it is very handy to have them in /sys/bus/iio for direct inspection. For example we run a device test tool reading such values.

You might argue that we should run our own fork for this purpose to provide a patched driver. But we want people to use the kernel from kernel.org and still be able to do such debugging without asking them to replace the kernel first.

Quoted 151 lines

Quoted 151 lines

Quoted 151 lines

drivers/input/touchscreen/tsc2007.c +107 -2

+};
+
+static int tsc2007_read_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int *val, int *val2, long mask)
+{
+	struct  tsc2007 *tsc = iio_priv(indio_dev);
+	int adc_chan = chan->channel;
+	int ret = 0;
+
+	if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
+		return -EINVAL;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	mutex_lock(&tsc->mlock);
+
+	switch (chan->channel) {
+	case 0:
+		*val = tsc2007_xfer(tsc, READ_X);
+		break;
+	case 1:
+		*val = tsc2007_xfer(tsc, READ_Y);
+		break;
+	case 2:
+		*val = tsc2007_xfer(tsc, READ_Z1);
+		break;
+	case 3:
+		*val = tsc2007_xfer(tsc, READ_Z2);
+		break;
+	case 4:
+		*val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
+		break;
+	case 5: {
+		struct ts_event tc;
+
+		tc.x = tsc2007_xfer(tsc, READ_X);
+		tc.z1 = tsc2007_xfer(tsc, READ_Z1);
+		tc.z2 = tsc2007_xfer(tsc, READ_Z2);
+		*val = tsc2007_calculate_resistance(tsc, &tc);
+		break;
+	}
+	case 6:
+		*val = tsc2007_is_pen_down(tsc);
+		break;
+	case 7:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
+		break;
+	case 8:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
+		break;
+	}
+
+	/* Prepare for next touch reading - power down ADC, enable PENIRQ */
+	tsc2007_xfer(tsc, PWRDOWN);
+
+	mutex_unlock(&tsc->mlock);
+
+	ret = IIO_VAL_INT;
+
+	return ret;
+}
+
+static const struct iio_info tsc2007_iio_info = {
+	.read_raw = tsc2007_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
 #ifdef CONFIG_OF
 static int tsc2007_get_pendown_state_gpio(struct device *dev)
 {

@@ -453,15 +551,20 @@ static int tsc2007_probe(struct i2c_client *client,
 	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct tsc2007 *ts;
 	struct input_dev *input_dev;
+	struct iio_dev *indio_dev;
 	int err;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_READ_WORD_DATA))
 		return -EIO;
 
-	ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
-	if (!ts)
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*ts));
+	if (!indio_dev) {
+		dev_err(&client->dev, "iio_device_alloc failed\n");
 		return -ENOMEM;
+	}
+
+	ts = iio_priv(indio_dev);
 
 	input_dev = devm_input_allocate_device(&client->dev);
 	if (!input_dev)

@@ -469,10 +572,26 @@ static int tsc2007_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, ts);
 
+	indio_dev->name = "tsc2007";
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &tsc2007_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = tsc2007_iio_channel;
+	indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
+
+	err = iio_device_register(indio_dev);
+	if (err < 0) {
+		dev_err(&client->dev, "iio_device_register() failed: %d\n",
+			err);
+		return err;
+	}
+
 	ts->client = client;
 	ts->irq = client->irq;
 	ts->input = input_dev;
+	ts->indio = indio_dev;
 	init_waitqueue_head(&ts->wait);
+	mutex_init(&ts->mlock);
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));

@@ -548,6 +667,19 @@ static int tsc2007_probe(struct i2c_client *client,
 	return 0;
 }
 
+static int tsc2007_remove(struct i2c_client *client)
+{
+	struct tsc2007 *ts = i2c_get_clientdata(client);
+	struct input_dev *input_dev = ts->input;
+	struct iio_dev *indio_dev = ts->indio;
+
+	input_unregister_device(input_dev);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
 static const struct i2c_device_id tsc2007_idtable[] = {
 	{ "tsc2007", 0 },
 	{ }

@@ -570,6 +702,7 @@ static struct i2c_driver tsc2007_driver = {
 	},
 	.id_table	= tsc2007_idtable,
 	.probe		= tsc2007_probe,
+	.remove		= tsc2007_remove,
 };
 

Quoted 6 lines

Quoted 3 lines

drivers/input/touchscreen/tsc2007.c static struct i2c_driver tsc2007_driver = { · line 708

Quoted 2 lines

drivers/input/touchscreen/tsc2007.c +0 -0

 module_i2c_driver(tsc2007_driver);

BR and thanks,
Nikolaus

Hi Jonathan,

Quoted 13 lines

Am 24.09.2016 um 18:07 schrieb Jonathan Cameron <[email protected]>:

Quoted 10 lines · H. Nikolaus Schaller

The tsc2007 chip not only has a resistive touch screen controller but also an external AUX adc imput which can be used for an ambient light sensor, battery voltage monitoring or any general purpose.

Additionally it can measure the chip temperature.

This extension provides an iio interface for these adc channels in addition to the raw x, y, z values and the estimated touch screen resistance. This can be used for debugging or special applications.

Quoted 2 lines

I'm unconvinced that exposing the touch related channels is terribly useful.

Mostly for debugging of if someone wants to do a (better) touch detection in user space.

Am 24.09.2016 um 19:44 schrieb Dmitry Torokhov <[email protected]>:

Quoted 19 lines · H. Nikolaus Schaller

Hi Jonathan,

Quoted 7 lines

Am 24.09.2016 um 18:07 schrieb Jonathan Cameron <[email protected]>:

Quoted 4 lines · H. Nikolaus Schaller

The tsc2007 chip not only has a resistive touch screen controller but also an external AUX adc imput which can be used for an ambient light sensor, battery voltage monitoring or any general purpose.

Quoted 8 lines

Quoted 6 lines

Additionally it can measure the chip temperature.

This extension provides an iio interface for these adc channels in addition to the raw x, y, z values and the estimated touch screen resistance. This can be used for debugging or special applications.

I'm unconvinced that exposing the touch related channels is terribly useful.

Mostly for debugging of if someone wants to do a (better) touch detection

Quoted 1 line

in user space.

For that we should probably export data as a heat map via V4L2 interface, similarly to what Nick Dyer did for Synaptics and Atmel drivers (albeit their heatmap functions are too slow for real processing and only suitable for debug/validation).

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 .../devicetree/bindings/input/ads7846.txt          |  9 +++-
 drivers/input/touchscreen/ads7846.c                | 60 ++++++++++++++++++----
 2 files changed, 57 insertions(+), 12 deletions(-)

Documentation/devicetree/bindings/input/ads7846.txt +8 -1

index 9fc47b0..29f91ed 100644
@@ -26,6 +26,12 @@ Additional required properties:
 
 Optional properties:
 
+You can optionally specify any of the touchscreen parameters described in
+
+	Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
+
+This allows to scale, invert or swap coordinates and define the fuzz factors.
+
 	ti,vref-delay-usecs		vref supply delay in usecs, 0 for
 					external vref (u16).
 	ti,vref-mv			The VREF voltage, in millivolts (u16).

@@ -33,7 +39,7 @@ Optional properties:
 					(ADS7846).
 	ti,keep-vref-on			set to keep vref on for differential
 					measurements as well
-	ti,swap-xy			swap x and y axis
+	ti,swap-xy			deprecated name for touchscreen-swapped-x-y
 	ti,settle-delay-usec		Settling time of the analog signals;
 					a function of Vcc and the capacitance
 					on the X/Y drivers.  If set to non-zero,

@@ -82,6 +88,7 @@ Example for a TSC2046 chip connected to an McSPI controller of an OMAP SoC::
 			pendown-gpio = <&gpio1 8 0>;
 			vcc-supply = <&reg_vcc3>;
 
+			touchscreen-swapped-x-y;
 			ti,x-min = /bits/ 16 <0>;
 			ti,x-max = /bits/ 16 <8000>;
 			ti,y-min = /bits/ 16 <0>;

drivers/input/touchscreen/ads7846.c +49 -11

index 1ce3ecb..400e421 100644
@@ -34,6 +34,7 @@
 #include <linux/spi/ads7846.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
+#include <linux/input/touchscreen.h>
 #include <asm/irq.h>
 
 /*

@@ -109,8 +110,13 @@ struct ads7846 {
 	u16			vref_delay_usecs;
 	u16			x_plate_ohms;
 	u16			pressure_max;
+	u16			x_min;
+	u16			x_max;
+	u16			y_min;
+	u16			y_max;
+
+	struct touchscreen_properties prop;
 
-	bool			swap_xy;
 	bool			use_internal;
 
 	struct ads7846_packet	*packet;

@@ -825,22 +831,36 @@ static void ads7846_report_state(struct ads7846 *ts)
 	 */
 	if (Rt) {
 		struct input_dev *input = ts->input;
+		int sx, sy;
+
+		dev_dbg(&ts->spi->dev,
+			"Raw point(%4d,%4d), pressure (%4u)\n",
+				x, y, Rt);
+
+		/* scale ADC values to desired output range */
+		sx = (ts->prop.max_x * (x - ts->x_min))
+			/ (ts->x_max - ts->x_min);
+		sy = (ts->prop.max_y * (y - ts->y_min))
+			/ (ts->y_max - ts->y_min);
 
-		if (ts->swap_xy)
-			swap(x, y);
+		dev_dbg(&ts->spi->dev,
+			"Scaled point(%4d,%4d), pressure (%4u)\n",
+				sx, sy, Rt);
 
+		/* report event */
 		if (!ts->pendown) {
 			input_report_key(input, BTN_TOUCH, 1);
 			ts->pendown = true;
 			dev_vdbg(&ts->spi->dev, "DOWN\n");
 		}
 
-		input_report_abs(input, ABS_X, x);
-		input_report_abs(input, ABS_Y, y);
+		touchscreen_report_pos(ts->input, &ts->prop,
+				       (unsigned int) sx, (unsigned int) sy,
+				       false);
 		input_report_abs(input, ABS_PRESSURE, ts->pressure_max - Rt);
 
 		input_sync(input);
-		dev_vdbg(&ts->spi->dev, "%4d/%4d/%4d\n", x, y, Rt);
+		dev_vdbg(&ts->spi->dev, "%4d/%4d/%4d\n", sx, sy, Rt);
 	}
 }
 

@@ -1212,6 +1232,8 @@ static const struct ads7846_platform_data *ads7846_probe_dt(struct device *dev)
 	pdata->keep_vref_on = of_property_read_bool(node, "ti,keep-vref-on");
 
 	pdata->swap_xy = of_property_read_bool(node, "ti,swap-xy");
+	if (pdata->swap_xy)
+		dev_notice(dev, "please update device tree to use touchscreen-swapped-x-y");
 
 	of_property_read_u16(node, "ti,settle-delay-usec",
 			     &pdata->settle_delay_usecs);

@@ -1315,7 +1337,6 @@ static int ads7846_probe(struct spi_device *spi)
 	ts->pressure_max = pdata->pressure_max ? : ~0;
 
 	ts->vref_mv = pdata->vref_mv;
-	ts->swap_xy = pdata->swap_xy;
 
 	if (pdata->filter != NULL) {
 		if (pdata->filter_init != NULL) {

@@ -1355,18 +1376,35 @@ static int ads7846_probe(struct spi_device *spi)
 	input_dev->dev.parent = &spi->dev;
 
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input_dev->absbit[0] = BIT_MASK(ABS_X) | BIT_MASK(ABS_Y) |
+				BIT_MASK(ABS_PRESSURE);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+	ts->x_min = pdata->x_min ? : 0;
+	ts->x_max = pdata->x_max ? : MAX_12BIT;
+	ts->y_min = pdata->y_min ? : 0;
+	ts->y_max = pdata->y_max ? : MAX_12BIT;
+
 	input_set_abs_params(input_dev, ABS_X,
-			pdata->x_min ? : 0,
-			pdata->x_max ? : MAX_12BIT,
+			ts->x_min,
+			ts->x_max,
 			0, 0);
 	input_set_abs_params(input_dev, ABS_Y,
-			pdata->y_min ? : 0,
-			pdata->y_max ? : MAX_12BIT,
+			ts->y_min,
+			ts->y_max,
 			0, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE,
 			pdata->pressure_min, pdata->pressure_max, 0, 0);
 
+	if (spi->dev.of_node) {
+		input_abs_set_min(input_dev, ABS_X, 0);
+		input_abs_set_min(input_dev, ABS_Y, 0);
+
+		touchscreen_parse_properties(ts->input, false, &ts->prop);
+	}
+
+	ts->prop.swap_x_y |= pdata->swap_xy;
+
 	ads7846_setup_spi_msg(ts, pdata);
 
 	ts->reg = regulator_get(&spi->dev, "vcc");

Signature · 6 lines

2.7.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html