[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Pavel Machek]

Fix dts to match what the Linux kernel expects. This works around
touchscreen problems in 4.1 linux on Nokia n900.
    
Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
index 4b641c7..09089a6 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
@@ -32,8 +32,8 @@ Example:
 		touchscreen-fuzz-x = <4>;
 		touchscreen-fuzz-y = <7>;
 		touchscreen-fuzz-pressure = <2>;
-		touchscreen-max-x = <4096>;
-		touchscreen-max-y = <4096>;
+		touchscreen-size-x = <4096>;
+		touchscreen-size-y = <4096>;
 		touchscreen-max-pressure = <2048>;
 
 		ti,x-plate-ohms = <280>;
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 5c16145..5f5e0f3 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -832,8 +832,8 @@
 		touchscreen-fuzz-x = <4>;
 		touchscreen-fuzz-y = <7>;
 		touchscreen-fuzz-pressure = <2>;
-		touchscreen-max-x = <4096>;
-		touchscreen-max-y = <4096>;
+		touchscreen-size-x = <4096>;
+		touchscreen-size-y = <4096>;
 		touchscreen-max-pressure = <2048>;
 
 		ti,x-plate-ohms = <280>;


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Felipe Balbi]

Hi,

On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> Fix dts to match what the Linux kernel expects. This works around
> touchscreen problems in 4.1 linux on Nokia n900.
>     
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> index 4b641c7..09089a6 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> @@ -32,8 +32,8 @@ Example:
>  		touchscreen-fuzz-x = <4>;
>  		touchscreen-fuzz-y = <7>;
>  		touchscreen-fuzz-pressure = <2>;
> -		touchscreen-max-x = <4096>;
> -		touchscreen-max-y = <4096>;
> +		touchscreen-size-x = <4096>;
> +		touchscreen-size-y = <4096>;

IMHO, the older binding needs to be supported as well. It's fine to
update the DTS for the new binding, but even Documentation says
touchscreen-max-[xy] and if the driver changed that, the driver should
be fixed too. Besides, it seems like this has been in tree since v3.16:

$ git describe a38cfebb56898633687ab337fd53710e63a0aedd
v3.15-rc5-72-ga38cfebb5689

So, because this has been wrongly documented for so long, we should
support both bindings. Sure, deprecate touchscreen-max-[xy], but they
must still be supported, IMO.

In any case, for this patch:

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150529/f83a90c8/attachment-0001.sig>

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Pavel Machek]

On Fri 2015-05-29 14:49:55, Felipe Balbi wrote:
> Hi,
> 
> On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > Fix dts to match what the Linux kernel expects. This works around
> > touchscreen problems in 4.1 linux on Nokia n900.
> >     
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > index 4b641c7..09089a6 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > @@ -32,8 +32,8 @@ Example:
> >  		touchscreen-fuzz-x = <4>;
> >  		touchscreen-fuzz-y = <7>;
> >  		touchscreen-fuzz-pressure = <2>;
> > -		touchscreen-max-x = <4096>;
> > -		touchscreen-max-y = <4096>;
> > +		touchscreen-size-x = <4096>;
> > +		touchscreen-size-y = <4096>;
> 
> IMHO, the older binding needs to be supported as well. It's fine to
> update the DTS for the new binding, but even Documentation says
> touchscreen-max-[xy] and if the driver changed that, the driver should
> be fixed too. Besides, it seems like this has been in tree since
> v3.16:

Agreed. In parent email, I have list of two commits that should be
reverted.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Maxime Ripard]

On Fri, May 29, 2015 at 09:56:29PM +0200, Pavel Machek wrote:
> On Fri 2015-05-29 14:49:55, Felipe Balbi wrote:
> > Hi,
> > 
> > On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > > Fix dts to match what the Linux kernel expects. This works around
> > > touchscreen problems in 4.1 linux on Nokia n900.
> > >     
> > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > index 4b641c7..09089a6 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > @@ -32,8 +32,8 @@ Example:
> > >  		touchscreen-fuzz-x = <4>;
> > >  		touchscreen-fuzz-y = <7>;
> > >  		touchscreen-fuzz-pressure = <2>;
> > > -		touchscreen-max-x = <4096>;
> > > -		touchscreen-max-y = <4096>;
> > > +		touchscreen-size-x = <4096>;
> > > +		touchscreen-size-y = <4096>;
> > 
> > IMHO, the older binding needs to be supported as well. It's fine to
> > update the DTS for the new binding, but even Documentation says
> > touchscreen-max-[xy] and if the driver changed that, the driver should
> > be fixed too. Besides, it seems like this has been in tree since
> > v3.16:
> 
> Agreed. In parent email, I have list of two commits that should be
> reverted.

So, if we sums things up. You introduce in some documentation example
some property, that you never document, that you still use in one
single DT, you don't even use that property in your driver, and now
that you realise you meant something else, you want the code that
actually parse the *right* property and does the right thing, that all
other DT agree (and depend on) to be reverted?

This is a joke, right?

If not, I can show you a handful of DTs that will be equally broken if
you revert the commits. Who wins?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150529/8c07edc5/attachment.sig>

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Felipe Balbi]

On Fri, May 29, 2015 at 10:17:45PM +0200, Maxime Ripard wrote:
> On Fri, May 29, 2015 at 09:56:29PM +0200, Pavel Machek wrote:
> > On Fri 2015-05-29 14:49:55, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > > > Fix dts to match what the Linux kernel expects. This works around
> > > > touchscreen problems in 4.1 linux on Nokia n900.
> > > >     
> > > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > > index 4b641c7..09089a6 100644
> > > > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > > @@ -32,8 +32,8 @@ Example:
> > > >  		touchscreen-fuzz-x = <4>;
> > > >  		touchscreen-fuzz-y = <7>;
> > > >  		touchscreen-fuzz-pressure = <2>;
> > > > -		touchscreen-max-x = <4096>;
> > > > -		touchscreen-max-y = <4096>;
> > > > +		touchscreen-size-x = <4096>;
> > > > +		touchscreen-size-y = <4096>;
> > > 
> > > IMHO, the older binding needs to be supported as well. It's fine to
> > > update the DTS for the new binding, but even Documentation says
> > > touchscreen-max-[xy] and if the driver changed that, the driver should
> > > be fixed too. Besides, it seems like this has been in tree since
> > > v3.16:
> > 
> > Agreed. In parent email, I have list of two commits that should be
> > reverted.
> 
> So, if we sums things up. You introduce in some documentation example
> some property, that you never document, that you still use in one

it was Documented in DT bindings document for this particular driver.
What are you talking about ?

> single DT, you don't even use that property in your driver, and now
> that you realise you meant something else, you want the code that

not Pali, Sebastian.

> actually parse the *right* property and does the right thing, that all
> other DT agree (and depend on) to be reverted?

We shouldn't revert, that I agree. But both properties should be parsed.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150529/0fb63681/attachment.sig>

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Dmitry Torokhov]

On Fri, May 29, 2015 at 03:21:23PM -0500, Felipe Balbi wrote:
> On Fri, May 29, 2015 at 10:17:45PM +0200, Maxime Ripard wrote:
> > On Fri, May 29, 2015 at 09:56:29PM +0200, Pavel Machek wrote:
> > > On Fri 2015-05-29 14:49:55, Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > > > > Fix dts to match what the Linux kernel expects. This works around
> > > > > touchscreen problems in 4.1 linux on Nokia n900.
> > > > >     
> > > > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > > > index 4b641c7..09089a6 100644
> > > > > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > > > @@ -32,8 +32,8 @@ Example:
> > > > >  		touchscreen-fuzz-x = <4>;
> > > > >  		touchscreen-fuzz-y = <7>;
> > > > >  		touchscreen-fuzz-pressure = <2>;
> > > > > -		touchscreen-max-x = <4096>;
> > > > > -		touchscreen-max-y = <4096>;
> > > > > +		touchscreen-size-x = <4096>;
> > > > > +		touchscreen-size-y = <4096>;
> > > > 
> > > > IMHO, the older binding needs to be supported as well. It's fine to
> > > > update the DTS for the new binding, but even Documentation says
> > > > touchscreen-max-[xy] and if the driver changed that, the driver should
> > > > be fixed too. Besides, it seems like this has been in tree since
> > > > v3.16:
> > > 
> > > Agreed. In parent email, I have list of two commits that should be
> > > reverted.
> > 
> > So, if we sums things up. You introduce in some documentation example
> > some property, that you never document, that you still use in one
> 
> it was Documented in DT bindings document for this particular driver.
> What are you talking about ?

It was documented in "example", not in the documentation that says:

 - properties defined in touchscreen.txt

which says nothing about touchscreen-max-x. And _noone_ has ever parsed this
property, so adding support for it does not make any sense.

> 
> > single DT, you don't even use that property in your driver, and now
> > that you realise you meant something else, you want the code that
> 
> not Pali, Sebastian.
> 
> > actually parse the *right* property and does the right thing, that all
> > other DT agree (and depend on) to be reverted?
> 
> We shouldn't revert, that I agree. But both properties should be parsed.

No. If the property is wrong, and nobody parsed it, I do not see any reason to
start now.


-- 
Dmitry

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Pavel Machek]

Hi!

> > > single DT, you don't even use that property in your driver, and now
> > > that you realise you meant something else, you want the code that
> > 
> > not Pali, Sebastian.
> > 
> > > actually parse the *right* property and does the right thing, that all
> > > other DT agree (and depend on) to be reverted?
> > 
> > We shouldn't revert, that I agree. But both properties should be parsed.
> 
> No. If the property is wrong, and nobody parsed it, I do not see any reason to
> start now.

Agreed.

But that's not what I'm asking. See a changelog of
3eea8b5d68c801fec788b411582b803463834752 and compare it with what it
actually does.

It is buggy. If fuzz is specified but maximum is not, it overwites
maximum with zero.

Plus it introduces new failure "if (!test_bit(axis, dev->absbit))".

Plus it fails to distinguish between "value not specified in the dt"
and "zero is specified in the dt".

The 3eea8b5d68c801fec788b411582b803463834752 is just bad.
									
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Dmitry Torokhov]

On Fri, May 29, 2015 at 10:34:56PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > > single DT, you don't even use that property in your driver, and now
> > > > that you realise you meant something else, you want the code that
> > > 
> > > not Pali, Sebastian.
> > > 
> > > > actually parse the *right* property and does the right thing, that all
> > > > other DT agree (and depend on) to be reverted?
> > > 
> > > We shouldn't revert, that I agree. But both properties should be parsed.
> > 
> > No. If the property is wrong, and nobody parsed it, I do not see any reason to
> > start now.
> 
> Agreed.
> 
> But that's not what I'm asking. See a changelog of
> 3eea8b5d68c801fec788b411582b803463834752 and compare it with what it
> actually does.
> 
> It is buggy. If fuzz is specified but maximum is not, it overwites
> maximum with zero.

Yes.

> 
> Plus it introduces new failure "if (!test_bit(axis, dev->absbit))".

That is not a new failure. It actually warns users that they trying to
specify in DT something that will be ignored by the kernel (because
without that absbit kernel will ignore all requests to that event code).

> 
> Plus it fails to distinguish between "value not specified in the dt"
> and "zero is specified in the dt".

Yes. I am not sure if we should care and support all permutations (ah, I
pre-setup fuzz in the driver, but override max on X, and I pre-setup
max on X, but take fuzz from DT). Maybe we should simply document that
specifying one parameter for an axis will change the rest of them to be
0. Not sure though...

Thanks.

-- 
Dmitry

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Pavel Machek]

On Fri 2015-05-29 13:48:47, Dmitry Torokhov wrote:
> On Fri, May 29, 2015 at 10:34:56PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > > > single DT, you don't even use that property in your driver, and now
> > > > > that you realise you meant something else, you want the code that
> > > > 
> > > > not Pali, Sebastian.
> > > > 
> > > > > actually parse the *right* property and does the right thing, that all
> > > > > other DT agree (and depend on) to be reverted?
> > > > 
> > > > We shouldn't revert, that I agree. But both properties should be parsed.
> > > 
> > > No. If the property is wrong, and nobody parsed it, I do not see any reason to
> > > start now.
> > 
> > Agreed.
> > 
> > But that's not what I'm asking. See a changelog of
> > 3eea8b5d68c801fec788b411582b803463834752 and compare it with what it
> > actually does.
> > 
> > It is buggy. If fuzz is specified but maximum is not, it overwites
> > maximum with zero.
> 
> Yes.
> 
> > 
> > Plus it introduces new failure "if (!test_bit(axis, dev->absbit))".
> 
> That is not a new failure. It actually warns users that they trying to
> specify in DT something that will be ignored by the kernel (because
> without that absbit kernel will ignore all requests to that event code).

What if driver sets the bits after parsing device tree?

> > Plus it fails to distinguish between "value not specified in the dt"
> > and "zero is specified in the dt".
> 
> Yes. I am not sure if we should care and support all permutations (ah, I
> pre-setup fuzz in the driver, but override max on X, and I pre-setup
> max on X, but take fuzz from DT). Maybe we should simply document that
> specifying one parameter for an axis will change the rest of them to be
> 0. Not sure though...

Well, the old code did support all permutations. "cleanups" should not
change such details...
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Dmitry Torokhov]

On Fri, May 29, 2015 at 11:02:59PM +0200, Pavel Machek wrote:
> On Fri 2015-05-29 13:48:47, Dmitry Torokhov wrote:
> > On Fri, May 29, 2015 at 10:34:56PM +0200, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > > > single DT, you don't even use that property in your driver, and now
> > > > > > that you realise you meant something else, you want the code that
> > > > > 
> > > > > not Pali, Sebastian.
> > > > > 
> > > > > > actually parse the *right* property and does the right thing, that all
> > > > > > other DT agree (and depend on) to be reverted?
> > > > > 
> > > > > We shouldn't revert, that I agree. But both properties should be parsed.
> > > > 
> > > > No. If the property is wrong, and nobody parsed it, I do not see any reason to
> > > > start now.
> > > 
> > > Agreed.
> > > 
> > > But that's not what I'm asking. See a changelog of
> > > 3eea8b5d68c801fec788b411582b803463834752 and compare it with what it
> > > actually does.
> > > 
> > > It is buggy. If fuzz is specified but maximum is not, it overwites
> > > maximum with zero.
> > 
> > Yes.
> > 
> > > 
> > > Plus it introduces new failure "if (!test_bit(axis, dev->absbit))".
> > 
> > That is not a new failure. It actually warns users that they trying to
> > specify in DT something that will be ignored by the kernel (because
> > without that absbit kernel will ignore all requests to that event code).
> 
> What if driver sets the bits after parsing device tree?

It should not.

Thanks.

-- 
Dmitry

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Maxime Ripard]

On Fri, May 29, 2015 at 10:34:56PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > > single DT, you don't even use that property in your driver, and now
> > > > that you realise you meant something else, you want the code that
> > > 
> > > not Pali, Sebastian.
> > > 
> > > > actually parse the *right* property and does the right thing, that all
> > > > other DT agree (and depend on) to be reverted?
> > > 
> > > We shouldn't revert, that I agree. But both properties should be parsed.
> > 
> > No. If the property is wrong, and nobody parsed it, I do not see any reason to
> > start now.
> 
> Agreed.
> 
> But that's not what I'm asking. See a changelog of
> 3eea8b5d68c801fec788b411582b803463834752 and compare it with what it
> actually does.
> 
> It is buggy. If fuzz is specified but maximum is not, it overwites
> maximum with zero.

If maximum is not set, you'll have other issues anyway. But it really
boils down on what the default behaviour should be.

> Plus it introduces new failure "if (!test_bit(axis, dev->absbit))".

It's not a new failure, it's testing against stupid code.

If an axis is setup in the DT but not registered in the driver,
something is wrong, most probably the DT.

> Plus it fails to distinguish between "value not specified in the dt"
> and "zero is specified in the dt".

Again, default behaviour.

> The 3eea8b5d68c801fec788b411582b803463834752 is just bad.

You were very welcome to review this patch at the time and/or suggest
a fix that pleases everyone.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150601/a21a0745/attachment-0001.sig>

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Maxime Ripard]

On Mon, Jun 01, 2015 at 04:06:06PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > But that's not what I'm asking. See a changelog of
> > > 3eea8b5d68c801fec788b411582b803463834752 and compare it with what it
> > > actually does.
> > > 
> > > It is buggy. If fuzz is specified but maximum is not, it overwites
> > > maximum with zero.
> > 
> > If maximum is not set, you'll have other issues anyway. But it really
> > boils down on what the default behaviour should be.
> 
> It was not broken before commit
> 3eea8b5d68c801fec788b411582b803463834752. Maximum was set, but after
> your patch, it is overwritten with zero.
> 
> > > Plus it introduces new failure "if (!test_bit(axis, dev->absbit))".
> > 
> > It's not a new failure, it's testing against stupid code.
> 
> Yes. In a commit marked "cleanup". We call this "undocumented
> feature".

It has never been said that it was a "cleanup" (whatever the double
quotes mean) but a rework. And it's not a feature either.

> > If an axis is setup in the DT but not registered in the driver,
> > something is wrong, most probably the DT.
> 
> Yes, we have fixed the DT, so that bug you introduced will not happen
> on n900 with updated device tree.

s/updated/fixed/

> > > Plus it fails to distinguish between "value not specified in the dt"
> > > and "zero is specified in the dt".
> > 
> > Again, default behaviour.
> 
> Again, regression from 4.0 kernel, you are not willing to fix.
>
> > > 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.

It's not really a very good way to move forward and start a productive
discussion.

I'm sorry if this cause you any issues, but reverting this patch will
break users (and this time at the kernel level only, the DT has
nothing to do with that) just like you are right now.

What about:

-- >8 --
diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index b82b5207c78b..7e98c2e443ab 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -14,10 +14,10 @@
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
 
-static u32 of_get_optional_u32(struct device_node *np,
+static int of_get_optional_u32(struct device_node *np,
 			       const char *property)
 {
-	u32 val = 0;
+	int val = -1;
 
 	of_property_read_u32(np, property, &val);
 
@@ -42,8 +42,12 @@ static void touchscreen_set_params(struct input_dev *dev,
 	}
 
 	absinfo = &dev->absinfo[axis];
-	absinfo->maximum = max;
-	absinfo->fuzz = fuzz;
+
+	if (max >= 0)
+		absinfo->maximum = max;
+
+	if (fuzz >= 0)
+		absinfo->fuzz = fuzz;
 }
 
 /**
@@ -57,7 +61,7 @@ static void touchscreen_set_params(struct input_dev *dev,
 void touchscreen_parse_of_params(struct input_dev *dev)
 {
 	struct device_node *np = dev->dev.parent->of_node;
-	u32 maximum, fuzz;
+	int maximum, fuzz;
 
 	input_alloc_absinfo(dev);
 	if (!dev->absinfo)
-- >8 --

That reduces the max size of the screens, but I don't really expect the screen
size to reach that order of magnitude before a few years...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150601/f3f3e427/attachment.sig>

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Pavel Machek]

> > > > 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?
...


> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
> index b82b5207c78b..7e98c2e443ab 100644
> --- a/drivers/input/touchscreen/of_touchscreen.c
> +++ b/drivers/input/touchscreen/of_touchscreen.c
> @@ -14,10 +14,10 @@
>  #include <linux/input/mt.h>
>  #include <linux/input/touchscreen.h>
>  
> -static u32 of_get_optional_u32(struct device_node *np,
> +static int of_get_optional_u32(struct device_node *np,
>  			       const char *property)
>  {
> -	u32 val = 0;
> +	int val = -1;
>  
>  	of_property_read_u32(np, property, &val);
>  
> @@ -42,8 +42,12 @@ static void touchscreen_set_params(struct input_dev *dev,
>  	}
>  
>  	absinfo = &dev->absinfo[axis];
> -	absinfo->maximum = max;
> -	absinfo->fuzz = fuzz;
> +
> +	if (max >= 0)
> +		absinfo->maximum = max;
> +
> +	if (fuzz >= 0)
> +		absinfo->fuzz = fuzz;
>  }
>  
>  /**
> @@ -57,7 +61,7 @@ static void touchscreen_set_params(struct input_dev *dev,
>  void touchscreen_parse_of_params(struct input_dev *dev)
>  {
>  	struct device_node *np = dev->dev.parent->of_node;
> -	u32 maximum, fuzz;
> +	int maximum, fuzz;
>  
>  	input_alloc_absinfo(dev);
>  	if (!dev->absinfo)
> -- >8 --
> 
> That reduces the max size of the screens, but I don't really expect the screen
> size to reach that order of magnitude before a few years...

Umm. Won't you have to update

-     if (maximum || fuzz)
+     if (maximum >= 0 || fuzz >= 0)

? Thanks,

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Dmitry Torokhov]

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@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 |   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;
 }
 
 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 maximum, fuzz;
+	int axis;
+	bool 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_maximum(axis),
+						&maximum) |
+		       touchscreen_get_prop_u32(np, "touchscreen-fuzz-x",
+						input_abs_get_fuzz(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_maximum(axis),
+						&maximum) |
+		       touchscreen_get_prop_u32(np, "touchscreen-fuzz-y",
+						input_abs_get_fuzz(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_maximum(axis),
+						&maximum) |
+		       touchscreen_get_prop_u32(np, "touchscreen-fuzz-pressure",
+						input_abs_get_fuzz(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

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Pavel Machek]

On Mon 2015-06-01 10:47:30, 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.

+       data_present = touchscreen_get_prop_u32(np,
"touchscreen-size-x",
+
input_abs_get_maximum(axis),
+                                               &maximum) |
+                      touchscreen_get_prop_u32(np,
"touchscreen-fuzz-x",
+
input_abs_get_fuzz(axis),
+                                               &fuzz);
+       if (data_present)
+               touchscreen_set_params(dev, axis, maximum, fuzz);

Umm. So you are changing behaviour from "whatever was there" to
"input_abs_get_maximum"... in n900 case. Is that a good idea for a
regression fix this late in release cycle?

Maxime's patch should be easy to fix...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Dmitry Torokhov]

On Mon, Jun 01, 2015 at 10:27:40PM +0200, Pavel Machek wrote:
> On Mon 2015-06-01 10:47:30, 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.
> 
> +       data_present = touchscreen_get_prop_u32(np,
> "touchscreen-size-x",
> +
> input_abs_get_maximum(axis),
> +                                               &maximum) |
> +                      touchscreen_get_prop_u32(np,
> "touchscreen-fuzz-x",
> +
> input_abs_get_fuzz(axis),
> +                                               &fuzz);
> +       if (data_present)
> +               touchscreen_set_params(dev, axis, maximum, fuzz);
> 
> Umm. So you are changing behaviour from "whatever was there" to
> "input_abs_get_maximum"... in n900 case.o

That _is_ "whatever was there".

> Is that a good idea for a
> regression fix this late in release cycle?

As Maxime mentioned the new behavior is needed for other touchscreens.
Given that fixed DTS is going into 4.1 (as far as I understand) we do
not need to rush this change into 4.1.

Thanks.

-- 
Dmitry

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Tony Lindgren]

* Dmitry Torokhov <dmitry.torokhov@gmail.com> [150601 13:47]:
> On Mon, Jun 01, 2015 at 10:27:40PM +0200, Pavel Machek wrote:
> > On Mon 2015-06-01 10:47:30, 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.
> > 
> > +       data_present = touchscreen_get_prop_u32(np,
> > "touchscreen-size-x",
> > +
> > input_abs_get_maximum(axis),
> > +                                               &maximum) |
> > +                      touchscreen_get_prop_u32(np,
> > "touchscreen-fuzz-x",
> > +
> > input_abs_get_fuzz(axis),
> > +                                               &fuzz);
> > +       if (data_present)
> > +               touchscreen_set_params(dev, axis, maximum, fuzz);
> > 
> > Umm. So you are changing behaviour from "whatever was there" to
> > "input_abs_get_maximum"... in n900 case.o
> 
> That _is_ "whatever was there".
> 
> > Is that a good idea for a
> > regression fix this late in release cycle?
> 
> As Maxime mentioned the new behavior is needed for other touchscreens.
> Given that fixed DTS is going into 4.1 (as far as I understand) we do
> not need to rush this change into 4.1.

OK great. I'm planning to send a pull request to arm-soc for Pavel's
DTS fix today along with few other fixes.

Regards,

Tony

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Maxime Ripard]

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@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 |   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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150601/02f632fa/attachment-0001.sig>

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Dmitry Torokhov]

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:
> > 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@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 |   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.

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.

Thanks.

-- 
Dmitry

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Pavel Machek]

On Mon 2015-06-01 14:32:13, 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:
> > > 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@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 |   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.
> 
> 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.

Seriously? Submit a patch documenting it, instead of writing strange
code. I bet everyone and their dog relies on it.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Maxime Ripard]

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:
> > > 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@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 |   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.
> 
> 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

But you're right that it's mostly a cosmetic change.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150602/79e21402/attachment-0001.sig>

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Dmitry Torokhov]

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:
> > > > 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@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 |   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.
> > 
> > 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).

Thanks.

-- 
Dmitry

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Dmitry Torokhov]

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

Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.

[Pavel Machek]

Hi!

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

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

Tested-by: Pavel Machek <pavel@ucw.cz>

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Dmitry Torokhov]

On Fri, May 29, 2015 at 09:56:29PM +0200, Pavel Machek wrote:
> On Fri 2015-05-29 14:49:55, Felipe Balbi wrote:
> > Hi,
> > 
> > On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > > Fix dts to match what the Linux kernel expects. This works around
> > > touchscreen problems in 4.1 linux on Nokia n900.
> > >     
> > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > index 4b641c7..09089a6 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > @@ -32,8 +32,8 @@ Example:
> > >  		touchscreen-fuzz-x = <4>;
> > >  		touchscreen-fuzz-y = <7>;
> > >  		touchscreen-fuzz-pressure = <2>;
> > > -		touchscreen-max-x = <4096>;
> > > -		touchscreen-max-y = <4096>;
> > > +		touchscreen-size-x = <4096>;
> > > +		touchscreen-size-y = <4096>;
> > 
> > IMHO, the older binding needs to be supported as well. It's fine to
> > update the DTS for the new binding, but even Documentation says
> > touchscreen-max-[xy] and if the driver changed that, the driver should
> > be fixed too. Besides, it seems like this has been in tree since
> > v3.16:
> 
> Agreed. In parent email, I have list of two commits that should be
> reverted.

Well, not exactly. The kernel has never parsed touchscreen-max-x and
touchscreen-max-y, however we did indeed change behavior of
touchscreen_parse_of_params() to reset both max and fuzz if either one is
present... I guess we can adjust the behavior it for the sake of tsc2005.

-- 
Dmitry

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Maxime Ripard]

On Fri, May 29, 2015 at 02:49:55PM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > Fix dts to match what the Linux kernel expects. This works around
> > touchscreen problems in 4.1 linux on Nokia n900.
> >     
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > index 4b641c7..09089a6 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > @@ -32,8 +32,8 @@ Example:
> >  		touchscreen-fuzz-x = <4>;
> >  		touchscreen-fuzz-y = <7>;
> >  		touchscreen-fuzz-pressure = <2>;
> > -		touchscreen-max-x = <4096>;
> > -		touchscreen-max-y = <4096>;
> > +		touchscreen-size-x = <4096>;
> > +		touchscreen-size-y = <4096>;
> 
> IMHO, the older binding needs to be supported as well. It's fine to
> update the DTS for the new binding, but even Documentation says
> touchscreen-max-[xy] and if the driver changed that, the driver should
> be fixed too. Besides, it seems like this has been in tree since v3.16:
> 
> $ git describe a38cfebb56898633687ab337fd53710e63a0aedd
> v3.15-rc5-72-ga38cfebb5689
> 
> So, because this has been wrongly documented for so long, we should
> support both bindings. Sure, deprecate touchscreen-max-[xy], but they
> must still be supported, IMO.

This property has never been anything but a typo in a documentation of
a single driver.

Feel free to fix that in that driver, but I don't see why the core
code should handle that isolated typo.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150529/0eb41258/attachment.sig>

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Tony Lindgren]

* Maxime Ripard <maxime.ripard@free-electrons.com> [150529 13:06]:
> On Fri, May 29, 2015 at 02:49:55PM -0500, Felipe Balbi wrote:
> > Hi,
> > 
> > On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > > Fix dts to match what the Linux kernel expects. This works around
> > > touchscreen problems in 4.1 linux on Nokia n900.
> > >     
> > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > index 4b641c7..09089a6 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > @@ -32,8 +32,8 @@ Example:
> > >  		touchscreen-fuzz-x = <4>;
> > >  		touchscreen-fuzz-y = <7>;
> > >  		touchscreen-fuzz-pressure = <2>;
> > > -		touchscreen-max-x = <4096>;
> > > -		touchscreen-max-y = <4096>;
> > > +		touchscreen-size-x = <4096>;
> > > +		touchscreen-size-y = <4096>;
> > 
> > IMHO, the older binding needs to be supported as well. It's fine to
> > update the DTS for the new binding, but even Documentation says
> > touchscreen-max-[xy] and if the driver changed that, the driver should
> > be fixed too. Besides, it seems like this has been in tree since v3.16:
> > 
> > $ git describe a38cfebb56898633687ab337fd53710e63a0aedd
> > v3.15-rc5-72-ga38cfebb5689
> > 
> > So, because this has been wrongly documented for so long, we should
> > support both bindings. Sure, deprecate touchscreen-max-[xy], but they
> > must still be supported, IMO.
> 
> This property has never been anything but a typo in a documentation of
> a single driver.
> 
> Feel free to fix that in that driver, but I don't see why the core
> code should handle that isolated typo.

OK thanks for confirming. Will apply this into omap-for-v4.1/fixes.

Tony

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Pavel Machek]

> > > So, because this has been wrongly documented for so long, we should
> > > support both bindings. Sure, deprecate touchscreen-max-[xy], but they
> > > must still be supported, IMO.
> > 
> > This property has never been anything but a typo in a documentation of
> > a single driver.
> > 
> > Feel free to fix that in that driver, but I don't see why the core
> > code should handle that isolated typo.
> 
> OK thanks for confirming. Will apply this into omap-for-v4.1/fixes.

Thanks!
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Pavel Machek]

On Fri 2015-05-29 22:03:06, Maxime Ripard wrote:
> On Fri, May 29, 2015 at 02:49:55PM -0500, Felipe Balbi wrote:
> > Hi,
> > 
> > On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> > > Fix dts to match what the Linux kernel expects. This works around
> > > touchscreen problems in 4.1 linux on Nokia n900.
> > >     
> > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > index 4b641c7..09089a6 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> > > @@ -32,8 +32,8 @@ Example:
> > >  		touchscreen-fuzz-x = <4>;
> > >  		touchscreen-fuzz-y = <7>;
> > >  		touchscreen-fuzz-pressure = <2>;
> > > -		touchscreen-max-x = <4096>;
> > > -		touchscreen-max-y = <4096>;
> > > +		touchscreen-size-x = <4096>;
> > > +		touchscreen-size-y = <4096>;
> > 
> > IMHO, the older binding needs to be supported as well. It's fine to
> > update the DTS for the new binding, but even Documentation says
> > touchscreen-max-[xy] and if the driver changed that, the driver should
> > be fixed too. Besides, it seems like this has been in tree since v3.16:
> > 
> > $ git describe a38cfebb56898633687ab337fd53710e63a0aedd
> > v3.15-rc5-72-ga38cfebb5689
> > 
> > So, because this has been wrongly documented for so long, we should
> > support both bindings. Sure, deprecate touchscreen-max-[xy], but they
> > must still be supported, IMO.
> 
> This property has never been anything but a typo in a documentation of
> a single driver.
> 
> Feel free to fix that in that driver, but I don't see why the core
> code should handle that isolated typo.

Well... the driver was not broken... before you did "cleanup" that did
two functional changes. And yes, the dts should be fixed, but that
does not make your "cleanup" good.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Maxime Ripard]

On Sat, May 30, 2015 at 12:14:30PM +0200, Pavel Machek wrote:
> Well... the driver was not broken... before you did "cleanup" that did
> two functional changes. And yes, the dts should be fixed, but that
> does not make your "cleanup" good.

Whether it's good or not is arguable, and it really boils down to what
we consider the default value when properties are missing.

If it's 0, then my code does the right thing. If it's undefined, well,
I'd expect undefined behaviour to change without any notice.

Feel free to suggest any better solution.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150601/ed3687f0/attachment.sig>

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Pavel Machek]

On Mon 2015-06-01 11:49:19, Maxime Ripard wrote:
> On Sat, May 30, 2015 at 12:14:30PM +0200, Pavel Machek wrote:
> > Well... the driver was not broken... before you did "cleanup" that did
> > two functional changes. And yes, the dts should be fixed, but that
> > does not make your "cleanup" good.
> 
> Whether it's good or not is arguable, and it really boils down to what
> we consider the default value when properties are missing.
> 
> If it's 0, then my code does the right thing. If it's undefined, well,
> I'd expect undefined behaviour to change without any notice.

It is "whatever was in the structure in the first place", as it was
before you patched the code.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900

[Sebastian Reichel]

Hi,

On Fri, May 29, 2015 at 09:32:11PM +0200, Pavel Machek wrote:
> Fix dts to match what the Linux kernel expects. This works around
> touchscreen problems in 4.1 linux on Nokia n900.
>     
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> index 4b641c7..09089a6 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
> @@ -32,8 +32,8 @@ Example:
>  		touchscreen-fuzz-x = <4>;
>  		touchscreen-fuzz-y = <7>;
>  		touchscreen-fuzz-pressure = <2>;
> -		touchscreen-max-x = <4096>;
> -		touchscreen-max-y = <4096>;
> +		touchscreen-size-x = <4096>;
> +		touchscreen-size-y = <4096>;
>  		touchscreen-max-pressure = <2048>;
>  
>  		ti,x-plate-ohms = <280>;
> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> index 5c16145..5f5e0f3 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -832,8 +832,8 @@
>  		touchscreen-fuzz-x = <4>;
>  		touchscreen-fuzz-y = <7>;
>  		touchscreen-fuzz-pressure = <2>;
> -		touchscreen-max-x = <4096>;
> -		touchscreen-max-y = <4096>;
> +		touchscreen-size-x = <4096>;
> +		touchscreen-size-y = <4096>;
>  		touchscreen-max-pressure = <2048>;
>  
>  		ti,x-plate-ohms = <280>;

Acked-By: Sebastian Reichel <sre@kernel.org>

max-x/fuzz-x is a leftover from development of the generic bindings
(Initially I used min and max).

If 3eea8b5d68c801fec788b411582b803463834752's behaviour should
become the new behaviour it should be documented, that maximum and
fuzz values are dependent on each other. Instead I suggest to
restore the old behaviour.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150530/2f9f6d69/attachment.sig>