linux-kernel - Re: Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150601212226.GH23777@lukather>
Date:	Mon, 1 Jun 2015 23:22:26 +0200
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	Pavel Machek <pavel@....cz>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Felipe Balbi <balbi@...com>,
	Sebastian Reichel <sre@...nel.org>,
	kernel list <linux-kernel@...r.kernel.org>,
	pali.rohar@...il.com, sre@...ian.org, sre@...g0.de,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	linux-omap@...r.kernel.org, tony@...mide.com, khilman@...nel.org,
	aaro.koskinen@....fi, ivo.g.dimitrov.75@...il.com,
	patrikbachan@...il.com
Subject: Re: Please revert 3eea8b5d68c801fec788b411582b803463834752 as it
 breaks touchscreen on n900.

Hi Dmitry,

On Mon, Jun 01, 2015 at 10:47:30AM -0700, Dmitry Torokhov wrote:
> On Mon, Jun 01, 2015 at 05:21:11PM +0200, Pavel Machek wrote:
> > 
> > 
> > > > > > The 3eea8b5d68c801fec788b411582b803463834752 is just bad.
> > > > > 
> > > > > You were very welcome to review this patch at the time and/or suggest
> > > > > a fix that pleases everyone.
> > > > 
> > > > You should be the one that should suggest fixes, as you broke it in
> > > > the first place. But clearly you don't understand that.
> > > 
> > > You actually never asked for a fix, and went head first calling this
> > > patch "bad" and asking for nothing but reverting it.
> > 
> > Date: Fri, 29 May 2015 21:08:16 +0200
> > Subject: 4.1 touchscreen regression on n900 -- pinpointed [was Re:
> > linux-n900
> > ...
> > Maxime, can you suggest a fix?
> 
> How about we do something like below (it needs a small edt-ft5x06 fixup
> that I'll send separately). Not tested.
> 
> Thanks.
> 
> -- 
> Dmitry
> 
> 
> Input: improve parsing OF parameters for touchscreens
> 
> From: Dmitry Torokhov <dmitry.torokhov@...il.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@....cz>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
>  drivers/input/touchscreen/edt-ft5x06.c     |    2 -
>  drivers/input/touchscreen/of_touchscreen.c |   67 ++++++++++++++++++----------
>  drivers/input/touchscreen/tsc2005.c        |    2 -
>  include/linux/input/touchscreen.h          |    5 +-
>  4 files changed, 48 insertions(+), 28 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..c132624 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_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.

So I guess you could just do:

*value = default_value;
return of_property_read_u32(np, property, value) ? true : false;

It looks good otherwise.

Thanks!
Maxime

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

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)