linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: dmitry.torokhov@gmail.com (Dmitry Torokhov)
To: linux-arm-kernel@lists.infradead.org
Subject: Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.
Date: Tue, 2 Jun 2015 11:08:55 -0700	[thread overview]
Message-ID: <20150602180855.GC11297@dtor-ws> (raw)
In-Reply-To: <20150602175845.GB11297@dtor-ws>

On Tue, Jun 02, 2015 at 10:58:45AM -0700, Dmitry Torokhov wrote:
> On Tue, Jun 02, 2015 at 11:44:47AM +0200, Maxime Ripard wrote:
> > On Mon, Jun 01, 2015 at 02:32:13PM -0700, Dmitry Torokhov wrote:
> > > On Mon, Jun 01, 2015 at 11:22:26PM +0200, Maxime Ripard wrote:
> > > > Hi Dmitry,
> > > > 
> > > > On Mon, Jun 01, 2015 at 10:47:30AM -0700, Dmitry Torokhov wrote:
> > > > > -static u32 of_get_optional_u32(struct device_node *np,
> > > > > -			       const char *property)
> > > > > +static bool touchscreen_get_property_u32(struct device_node *np,
> > > > > +					 const char *property,
> > > > > +					 unsigned int default_value,
> > > > > +					 unsigned int *value)
> > > > >  {
> > > > >  	u32 val = 0;
> > > > > +	int error;
> > > > >  
> > > > > -	of_property_read_u32(np, property, &val);
> > > > > +	error = of_property_read_u32(np, property, &val);
> > > > > +	if (error) {
> > > > > +		*value = default_value;
> > > > > +		return false;
> > > > > +	}
> > > > >  
> > > > > -	return val;
> > > > > +	*value = val;
> > > > > +	return true;
> > > > 
> > > > This looks good.
> > > > 
> > > > However, of_property_read_u32 already does the right thing here by not
> > > > update val if the property is not found.
> > > 
> > > I know but it is not documented anywhere (as far as I know) so I'd
> > > rather not rely on the implementation detail that might change in the
> > > future. This is not a hot path so extra assignment should not hurt.
> > 
> > It is actually: http://lxr.free-electrons.com/source/drivers/of/base.c#L1231
> 
> OK, fair enough. But not for ACPI properties (and I think we should
> convert the parser to device_property_read_xxx() so it is usable
> everywhere).

By the way, the previous version was busted, this one should compile.

-- 
Dmitry


Input: improve parsing OF parameters for touchscreens

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

When applying touchscreen parameters specified in device tree let's make
sure we keep whatever setup was done by the driver and not reset the
missing values to zero.

Reported-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/edt-ft5x06.c     |    2 -
 drivers/input/touchscreen/of_touchscreen.c |   69 ++++++++++++++++++----------
 drivers/input/touchscreen/tsc2005.c        |    2 -
 include/linux/input/touchscreen.h          |    5 +-
 4 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 29d179a..394b1de 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1041,7 +1041,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 			     0, tsdata->num_y * 64 - 1, 0, 0);
 
 	if (!pdata)
-		touchscreen_parse_of_params(input);
+		touchscreen_parse_of_params(input, true);
 
 	error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, INPUT_MT_DIRECT);
 	if (error) {
diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index b82b520..806cd0a 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -14,14 +14,22 @@
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
 
-static u32 of_get_optional_u32(struct device_node *np,
-			       const char *property)
+static bool touchscreen_get_prop_u32(struct device_node *np,
+				     const char *property,
+				     unsigned int default_value,
+				     unsigned int *value)
 {
-	u32 val = 0;
+	u32 val;
+	int error;
 
-	of_property_read_u32(np, property, &val);
+	error = of_property_read_u32(np, property, &val);
+	if (error) {
+		*value = default_value;
+		return false;
+	}
 
-	return val;
+	*value = val;
+	return true;
 }
 
 static void touchscreen_set_params(struct input_dev *dev,
@@ -54,34 +62,45 @@ static void touchscreen_set_params(struct input_dev *dev,
  * input device accordingly. The function keeps previously setuped default
  * values if no value is specified via DT.
  */
-void touchscreen_parse_of_params(struct input_dev *dev)
+void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch)
 {
 	struct device_node *np = dev->dev.parent->of_node;
-	u32 maximum, fuzz;
+	unsigned int axis;
+	unsigned int maximum, fuzz;
+	bool data_present;
 
 	input_alloc_absinfo(dev);
 	if (!dev->absinfo)
 		return;
 
-	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);
-		touchscreen_set_params(dev, ABS_MT_POSITION_X, maximum, fuzz);
-	}
+	axis = multitouch ? ABS_MT_POSITION_X : ABS_X;
+	data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x",
+						input_abs_get_max(dev, axis),
+						&maximum) |
+		       touchscreen_get_prop_u32(np, "touchscreen-fuzz-x",
+						input_abs_get_fuzz(dev, axis),
+						&fuzz);
+	if (data_present)
+		touchscreen_set_params(dev, axis, maximum, 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);
-		touchscreen_set_params(dev, ABS_MT_POSITION_Y, maximum, fuzz);
-	}
+	axis = multitouch ? ABS_MT_POSITION_Y : ABS_Y;
+	data_present = touchscreen_get_prop_u32(np, "touchscreen-size-y",
+						input_abs_get_max(dev, axis),
+						&maximum) |
+		       touchscreen_get_prop_u32(np, "touchscreen-fuzz-y",
+						input_abs_get_fuzz(dev, axis),
+						&fuzz);
+	if (data_present)
+		touchscreen_set_params(dev, axis, maximum, 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);
-		touchscreen_set_params(dev, ABS_MT_PRESSURE, maximum, fuzz);
-	}
+	axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE;
+	data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure",
+						input_abs_get_max(dev, axis),
+						&maximum) |
+		       touchscreen_get_prop_u32(np, "touchscreen-fuzz-pressure",
+						input_abs_get_fuzz(dev, axis),
+						&fuzz);
+	if (data_present)
+		touchscreen_set_params(dev, axis, maximum, fuzz);
 }
 EXPORT_SYMBOL(touchscreen_parse_of_params);
diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 72657c5..d8c025b 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -709,7 +709,7 @@ static int tsc2005_probe(struct spi_device *spi)
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, max_p, fudge_p, 0);
 
 	if (np)
-		touchscreen_parse_of_params(input_dev);
+		touchscreen_parse_of_params(input_dev, false);
 
 	input_dev->open = tsc2005_open;
 	input_dev->close = tsc2005_close;
diff --git a/include/linux/input/touchscreen.h b/include/linux/input/touchscreen.h
index 08a5ef6..eecc9ea 100644
--- a/include/linux/input/touchscreen.h
+++ b/include/linux/input/touchscreen.h
@@ -12,9 +12,10 @@
 #include <linux/input.h>
 
 #ifdef CONFIG_OF
-void touchscreen_parse_of_params(struct input_dev *dev);
+void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch);
 #else
-static inline void touchscreen_parse_of_params(struct input_dev *dev)
+static inline void touchscreen_parse_of_params(struct input_dev *dev,
+					       bool multitouch)
 {
 }
 #endif

  reply	other threads:[~2015-06-02 18:08 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <201505241444.41039@pali>
     [not found] ` <20150527132545.GA23434@amd>
     [not found]   ` <20150527133311.GJ30798@pali>
     [not found]     ` <20150527143722.GA28108@amd>
     [not found]       ` <20150527145837.GA13223@earth>
     [not found]         ` <20150529190816.GA20232@amd>
     [not found]           ` <20150529192505.GA28987@amd>
2015-05-29 19:32             ` [PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900 Pavel Machek
2015-05-29 19:49               ` Felipe Balbi
2015-05-29 19:56                 ` Pavel Machek
2015-05-29 20:17                   ` Maxime Ripard
2015-05-29 20:21                     ` Felipe Balbi
2015-05-29 20:29                       ` Dmitry Torokhov
2015-05-29 20:34                         ` Pavel Machek
2015-05-29 20:48                           ` Dmitry Torokhov
2015-05-29 21:02                             ` Pavel Machek
2015-05-29 21:38                               ` Dmitry Torokhov
2015-06-01  9:55                           ` Maxime Ripard
     [not found]                             ` <20150601140605.GA26908@amd>
2015-06-01 14:58                               ` Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen " Maxime Ripard
2015-06-01 15:21                                 ` Pavel Machek
2015-06-01 17:47                                   ` Dmitry Torokhov
2015-06-01 20:27                                     ` Pavel Machek
2015-06-01 20:45                                       ` Dmitry Torokhov
2015-06-01 20:54                                         ` Tony Lindgren
2015-06-01 21:22                                     ` Maxime Ripard
2015-06-01 21:32                                       ` Dmitry Torokhov
2015-06-02  8:25                                         ` Pavel Machek
2015-06-02  9:44                                         ` Maxime Ripard
2015-06-02 17:58                                           ` Dmitry Torokhov
2015-06-02 18:08                                             ` Dmitry Torokhov [this message]
2015-06-25 20:25                                               ` Pavel Machek
2015-05-29 20:22                   ` [PATCH] fix n900 dts file to work around 4.1 touchscreen regression " Dmitry Torokhov
2015-05-29 20:03                 ` Maxime Ripard
2015-05-29 20:18                   ` Tony Lindgren
2015-05-29 20:30                     ` Pavel Machek
2015-05-30 10:14                   ` Pavel Machek
2015-06-01  9:49                     ` Maxime Ripard
2015-06-01  9:54                       ` Pavel Machek
2015-05-30  2:21               ` Sebastian Reichel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150602180855.GC11297@dtor-ws \
    --to=dmitry.torokhov@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).