[PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[Maxime Ripard]

Hi,

The current ft5x06 reports to the user-space that its maximum
coordinates are, on both X and Y, way higher than what could be
actually usable on the screen (in my case, 5759x1151 instead of
480x800).

This causes trouble on some userspace stacks that then try to re-scale
these coordinates back to the framebuffer resolution, like QT does.

Use the of_touchscreen code to find the real touchscreen limits in the
DT case, and report that to the userspace.

Thanks,
Maxime

Changes from v2:
  - Rebased on top of 4.0-rc1

Changes from v1:
  - Do not use input_set_abs_params anymore to avoid enabling an axis
    that was not enabled by the driver itself in of_touchscreen
  - Emit a warning when an axis is set in the DT but not enabled in
    the driver
  - Remove EV_SYN from the edt-ft5x06 driver

Maxime Ripard (4):
  input: touchscreen: of: Use input_set_abs_params
  input: touchscreen: of: Register multitouch axes
  input: ft5x06: Allow to set the maximum axes value through the DT
  input: edt-ft5x06: Remove EV_SYN event report

 drivers/input/touchscreen/edt-ft5x06.c     |  6 ++-
 drivers/input/touchscreen/of_touchscreen.c | 62 +++++++++++++++++++++++++-----
 2 files changed, 57 insertions(+), 11 deletions(-)

-- 
2.3.0

[PATCH v3 1/4] input: touchscreen: of: Use input_set_abs_params

[Maxime Ripard]

Drivers are still required to call input_set_abs_params for their axes, as if
they only use the touchscreen_parse_of_params function, the axis bit in absbit
won't be set.

Switch to using input_set_abs_params to fully setup each and every available
axis so that drivers will be able to solely use this function.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/input/touchscreen/of_touchscreen.c | 50 ++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index f8f9b84230b1..fe80d8ba7efa 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -13,6 +13,33 @@
 #include <linux/input.h>
 #include <linux/input/touchscreen.h>
 
+static u32 of_get_optional_u32(struct device_node *np,
+			       const char *property)
+{
+	u32 val = 0;
+
+	of_property_read_u32(np, property, &val);
+
+	return val;
+}
+
+static void touchscreen_set_params(struct input_dev *dev,
+				   unsigned long axis,
+				   int max, int fuzz)
+{
+	struct input_absinfo *absinfo;
+
+	if (!test_bit(axis, dev->absbit)) {
+		dev_warn(&dev->dev,
+			 "DT specifies parameters but the axis is not set up\n");
+		return;
+	}
+
+	absinfo = &dev->absinfo[axis];
+	absinfo->maximum = max;
+	absinfo->fuzz = fuzz;
+}
+
 /**
  * touchscreen_parse_of_params - parse common touchscreen DT properties
  * @dev: device that should be parsed
@@ -24,22 +51,25 @@
 void touchscreen_parse_of_params(struct input_dev *dev)
 {
 	struct device_node *np = dev->dev.parent->of_node;
-	struct input_absinfo *absinfo;
+	u32 maximum, fuzz;
 
 	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);
+	maximum = of_get_optional_u32(np, "touchscreen-size-x");
+	fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
+	if (maximum || fuzz)
+		touchscreen_set_params(dev, ABS_X, maximum, 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);
+	maximum = of_get_optional_u32(np, "touchscreen-size-y");
+	fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
+	if (maximum || fuzz)
+		touchscreen_set_params(dev, ABS_Y, maximum, 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);
+	maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
+	fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
+	if (maximum || fuzz)
+		touchscreen_set_params(dev, ABS_PRESSURE, maximum, fuzz);
 }
 EXPORT_SYMBOL(touchscreen_parse_of_params);
-- 
2.3.0

[PATCH v3 2/4] input: touchscreen: of: Register multitouch axes

[Maxime Ripard]

So far, the DT parsing code was only setting up the regular input axes,
completely ignoring their multitouch counter parts.

Fill them with the same parameters than the regular axes.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/input/touchscreen/of_touchscreen.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index fe80d8ba7efa..b82b5207c78b 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -11,6 +11,7 @@
 
 #include <linux/of.h>
 #include <linux/input.h>
+#include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
 
 static u32 of_get_optional_u32(struct device_node *np,
@@ -30,8 +31,13 @@ static void touchscreen_set_params(struct input_dev *dev,
 	struct input_absinfo *absinfo;
 
 	if (!test_bit(axis, dev->absbit)) {
-		dev_warn(&dev->dev,
-			 "DT specifies parameters but the axis is not set up\n");
+		/*
+		 * Emit a warning only if the axis is not a multitouch
+		 * axis, which might not be set by the driver.
+		 */
+		if (!input_is_mt_axis(axis))
+			dev_warn(&dev->dev,
+				 "DT specifies parameters but the axis is not set up\n");
 		return;
 	}
 
@@ -59,17 +65,23 @@ void touchscreen_parse_of_params(struct input_dev *dev)
 
 	maximum = of_get_optional_u32(np, "touchscreen-size-x");
 	fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
-	if (maximum || fuzz)
+	if (maximum || fuzz) {
 		touchscreen_set_params(dev, ABS_X, maximum, fuzz);
+		touchscreen_set_params(dev, ABS_MT_POSITION_X, maximum, fuzz);
+	}
 
 	maximum = of_get_optional_u32(np, "touchscreen-size-y");
 	fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
-	if (maximum || fuzz)
+	if (maximum || fuzz) {
 		touchscreen_set_params(dev, ABS_Y, maximum, fuzz);
+		touchscreen_set_params(dev, ABS_MT_POSITION_Y, maximum, fuzz);
+	}
 
 	maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
 	fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
-	if (maximum || fuzz)
+	if (maximum || fuzz) {
 		touchscreen_set_params(dev, ABS_PRESSURE, maximum, fuzz);
+		touchscreen_set_params(dev, ABS_MT_PRESSURE, maximum, fuzz);
+	}
 }
 EXPORT_SYMBOL(touchscreen_parse_of_params);
-- 
2.3.0

[PATCH v3 3/4] input: ft5x06: Allow to set the maximum axes value through the DT

[Maxime Ripard]

Currently the driver relies on some obscure and undocumented register to set
the maximum axis value.

The reported value is way too high to be meaningful, which confuses some
userspace tools like QT's evdevtouch plugin which try to scale the reported
events to the maximum values.

Use the values from the DT to set meaningful values.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d4c24fb7704f..cd8a7472266b 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -37,6 +37,7 @@
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
 #include <linux/input/edt-ft5x06.h>
 
 #define MAX_SUPPORT_POINTS		5
@@ -1044,6 +1045,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 			     0, tsdata->num_x * 64 - 1, 0, 0);
 	input_set_abs_params(input, ABS_MT_POSITION_Y,
 			     0, tsdata->num_y * 64 - 1, 0, 0);
+
+	if (!pdata)
+		touchscreen_parse_of_params(input);
+
 	error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, 0);
 	if (error) {
 		dev_err(&client->dev, "Unable to init MT slots.\n");
-- 
2.3.0

[PATCH v3 4/4] input: edt-ft5x06: Remove EV_SYN event report

[Maxime Ripard]

input_register_device already sets the EV_SYN event since all devices can
generate them.

Remove the redundant affectation.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index cd8a7472266b..e6aef3e48bd9 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1035,7 +1035,6 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 	input->id.bustype = BUS_I2C;
 	input->dev.parent = &client->dev;
 
-	__set_bit(EV_SYN, input->evbit);
 	__set_bit(EV_KEY, input->evbit);
 	__set_bit(EV_ABS, input->evbit);
 	__set_bit(BTN_TOUCH, input->keybit);
-- 
2.3.0

Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[Markus Pargmann]

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

Hi Maxime,

On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote:
> Hi,
> 
> The current ft5x06 reports to the user-space that its maximum
> coordinates are, on both X and Y, way higher than what could be
> actually usable on the screen (in my case, 5759x1151 instead of
> 480x800).
> 
> This causes trouble on some userspace stacks that then try to re-scale
> these coordinates back to the framebuffer resolution, like QT does.
> 
> Use the of_touchscreen code to find the real touchscreen limits in the
> DT case, and report that to the userspace.

Thanks for sending it again. This series works for me on imx6q with an
edt-ft5x06 touchscreen.

Tested-by: Markus Pargmann <mpa@pengutronix.de>

Best Regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

Hi Lothar, Dmitry,

On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote:
> Hi,
> 
> The current ft5x06 reports to the user-space that its maximum
> coordinates are, on both X and Y, way higher than what could be
> actually usable on the screen (in my case, 5759x1151 instead of
> 480x800).
> 
> This causes trouble on some userspace stacks that then try to re-scale
> these coordinates back to the framebuffer resolution, like QT does.
> 
> Use the of_touchscreen code to find the real touchscreen limits in the
> DT case, and report that to the userspace.

Do you have any comments on that?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[Dmitry Torokhov]

On Mon, Mar 23, 2015 at 05:10:02PM -0700, Maxime Ripard wrote:
> Hi Lothar, Dmitry,
> 
> On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote:
> > Hi,
> > 
> > The current ft5x06 reports to the user-space that its maximum
> > coordinates are, on both X and Y, way higher than what could be
> > actually usable on the screen (in my case, 5759x1151 instead of
> > 480x800).
> > 
> > This causes trouble on some userspace stacks that then try to re-scale
> > these coordinates back to the framebuffer resolution, like QT does.
> > 
> > Use the of_touchscreen code to find the real touchscreen limits in the
> > DT case, and report that to the userspace.
> 
> Do you have any comments on that?

Yes, I'll apply it, although if you could supply the updated patch
description for #1 that would be great. The current one seems from the
old version and is no longer accurate.

Thanks.

-- 
Dmitry

Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

On Tue, Mar 24, 2015 at 09:47:22AM -0700, Dmitry Torokhov wrote:
> On Mon, Mar 23, 2015 at 05:10:02PM -0700, Maxime Ripard wrote:
> > Hi Lothar, Dmitry,
> > 
> > On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote:
> > > Hi,
> > > 
> > > The current ft5x06 reports to the user-space that its maximum
> > > coordinates are, on both X and Y, way higher than what could be
> > > actually usable on the screen (in my case, 5759x1151 instead of
> > > 480x800).
> > > 
> > > This causes trouble on some userspace stacks that then try to re-scale
> > > these coordinates back to the framebuffer resolution, like QT does.
> > > 
> > > Use the of_touchscreen code to find the real touchscreen limits in the
> > > DT case, and report that to the userspace.
> > 
> > Do you have any comments on that?
> 
> Yes, I'll apply it, although if you could supply the updated patch
> description for #1 that would be great. The current one seems from the
> old version and is no longer accurate.
> 
> Thanks.

You're right.

Would something like:

"""
input: touchscreen: of: Rework the DT parsing function

The DT parsing function currently duplicates a lot of the code to
parse the touchscreen DT properties.

In order to ease further additions to this parsing routine, rework it
slightly to create new helper functions.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
"""

be good enough?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[Dmitry Torokhov]

On Tue, Mar 24, 2015 at 11:45:20AM -0700, Maxime Ripard wrote:
> On Tue, Mar 24, 2015 at 09:47:22AM -0700, Dmitry Torokhov wrote:
> > On Mon, Mar 23, 2015 at 05:10:02PM -0700, Maxime Ripard wrote:
> > > Hi Lothar, Dmitry,
> > > 
> > > On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote:
> > > > Hi,
> > > > 
> > > > The current ft5x06 reports to the user-space that its maximum
> > > > coordinates are, on both X and Y, way higher than what could be
> > > > actually usable on the screen (in my case, 5759x1151 instead of
> > > > 480x800).
> > > > 
> > > > This causes trouble on some userspace stacks that then try to re-scale
> > > > these coordinates back to the framebuffer resolution, like QT does.
> > > > 
> > > > Use the of_touchscreen code to find the real touchscreen limits in the
> > > > DT case, and report that to the userspace.
> > > 
> > > Do you have any comments on that?
> > 
> > Yes, I'll apply it, although if you could supply the updated patch
> > description for #1 that would be great. The current one seems from the
> > old version and is no longer accurate.
> > 
> > Thanks.
> 
> You're right.
> 
> Would something like:
> 
> """
> input: touchscreen: of: Rework the DT parsing function
> 
> The DT parsing function currently duplicates a lot of the code to
> parse the touchscreen DT properties.
> 
> In order to ease further additions to this parsing routine, rework it
> slightly to create new helper functions.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> """
> 
> be good enough?

Yep, thanks, I'll update the description, consider applied.

Thanks.

-- 
Dmitry

Re: [PATCH v3 0/4] input: ft5x06: Fix userspace reported maximum value

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

On Tue, Mar 24, 2015 at 11:56:14AM -0700, Dmitry Torokhov wrote:
> On Tue, Mar 24, 2015 at 11:45:20AM -0700, Maxime Ripard wrote:
> > On Tue, Mar 24, 2015 at 09:47:22AM -0700, Dmitry Torokhov wrote:
> > > On Mon, Mar 23, 2015 at 05:10:02PM -0700, Maxime Ripard wrote:
> > > > Hi Lothar, Dmitry,
> > > > 
> > > > On Tue, Mar 03, 2015 at 02:50:42PM +0100, Maxime Ripard wrote:
> > > > > Hi,
> > > > > 
> > > > > The current ft5x06 reports to the user-space that its maximum
> > > > > coordinates are, on both X and Y, way higher than what could be
> > > > > actually usable on the screen (in my case, 5759x1151 instead of
> > > > > 480x800).
> > > > > 
> > > > > This causes trouble on some userspace stacks that then try to re-scale
> > > > > these coordinates back to the framebuffer resolution, like QT does.
> > > > > 
> > > > > Use the of_touchscreen code to find the real touchscreen limits in the
> > > > > DT case, and report that to the userspace.
> > > > 
> > > > Do you have any comments on that?
> > > 
> > > Yes, I'll apply it, although if you could supply the updated patch
> > > description for #1 that would be great. The current one seems from the
> > > old version and is no longer accurate.
> > > 
> > > Thanks.
> > 
> > You're right.
> > 
> > Would something like:
> > 
> > """
> > input: touchscreen: of: Rework the DT parsing function
> > 
> > The DT parsing function currently duplicates a lot of the code to
> > parse the touchscreen DT properties.
> > 
> > In order to ease further additions to this parsing routine, rework it
> > slightly to create new helper functions.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > """
> > 
> > be good enough?
> 
> Yep, thanks, I'll update the description, consider applied.
> 
> Thanks.

Great, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]