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

[PATCH v2 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 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.1.1

[PATCH v2 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.1.1

[PATCH v2 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.1.1

[PATCH v2 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 ee3434f1e949..3ebc1a7e46c5 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
@@ -1042,6 +1043,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.1.1

[PATCH v2 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 3ebc1a7e46c5..a87f0d50f45c 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1033,7 +1033,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.1.1

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

[Markus Pargmann]

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

Hi,

On Thu, Nov 13, 2014 at 03:06:54PM +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,
> Maxime

any news on this?

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 v2 0/4] input: ft5x06: Fix userspace reported maximum value

[Maxime Ripard]

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

Hi Markus,

On Mon, Feb 23, 2015 at 10:49:36PM +0100, Markus Pargmann wrote:
> Hi,
> 
> On Thu, Nov 13, 2014 at 03:06:54PM +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,
> > Maxime
> 
> any news on this?

Unfortunately, no.

I never got any review on this, and somehow forgot about it.

I will rebase the patches on 4.0, and resend them.

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 v2 0/4] input: ft5x06: Fix userspace reported maximum value

[Markus Pargmann]

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

Hi Maxime,

On Tue, Feb 24, 2015 at 05:55:08PM +0100, Maxime Ripard wrote:
> Hi Markus,
> 
> On Mon, Feb 23, 2015 at 10:49:36PM +0100, Markus Pargmann wrote:
> > Hi,
> > 
> > On Thu, Nov 13, 2014 at 03:06:54PM +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,
> > > Maxime
> > 
> > any news on this?
> 
> Unfortunately, no.
> 
> I never got any review on this, and somehow forgot about it.
> 
> I will rebase the patches on 4.0, and resend them.

That's great, thanks. I will have some hardware next week to test it.

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 --]