| 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
| ||
|
Message-ID: <20150529211722.GB26866@amd>
Date: Fri, 29 May 2015 23:17:22 +0200
From: Pavel Machek <pavel@....cz>
To: Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc: Sebastian Reichel <sre@...nel.org>,
kernel list <linux-kernel@...r.kernel.org>,
dmitry.torokhov@...il.com,
Pali Rohár <pali.rohar@...il.com>,
Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>
Subject: Re: 4.1 touchscreen regression on n900 -- pinpointed [was Re:
linux-n900 v4.1-rc4]
On Fri 2015-05-29 21:57:40, Maxime Ripard wrote:
> On Fri, May 29, 2015 at 09:25:05PM +0200, Pavel Machek wrote:
> > On Fri 2015-05-29 21:08:16, Pavel Machek wrote:
> > > Hi!
> > >
> > > > mh I remember having problems with tsc2005 before. It helped to
> > > > reset the controller (should actually happen automatically when it
> > > > hangs, but I'm not sure, that it actually works).
> > >
> > > Ok, I did some more testing, and found out rather bogus values in
> > > evtest:
> > >
> > > Input device name: "TSC2005 touchscreen"
> > > Supported events:
> > > Event type 0 (EV_SYN)
> > > Event type 1 (EV_KEY)
> > > Event code 330 (BTN_TOUCH)
> > > Event type 3 (EV_ABS)
> > > Event code 0 (ABS_X)
> > > Value 2514
> > > Min 0
> > > Max 0
> > > Fuzz 4
> > >
> > > Which made me go through the git logs, and these patches looked
> > > suspicious. After a revert... yes, touchscreen works as well as it
> > > worked before.
> > >
> > > 0a363a380954e10fece7cd9931b66056eeb07d56
> > > 3eea8b5d68c801fec788b411582b803463834752
> > >
> > > (It is impossible to revert just 3eea..)
> >
> > Hmm, I see:
> >
> > touchscreen-max-x = <4096>;
> > touchscreen-max-y = <4096>;
> > ...that's n900 dts.. this should be size-x/size-y... so we have a bug
> > in dts.
> >
> > But the 3eea8b5d68c801fec788b411582b803463834752 is buggy, it should
> > not overwrite ->maximum for axis it has no devicetree data for.
>
> What do you mean? touchscreen-max-* _is_ device tree data for an axis.
>
> > Maybe replacing
> >
> > + if (maximum || fuzz)
> >
> > in 3eea to (maximum && fuzz)... would help, but it is late in the
> > cycle now, so I'd suggest just reverting 3eea8b.
>
> No, both maximum and fuzz are optional. You can perfectly have one
> without another.
Yes, which is something you got wrong.
Apply this on top of 3eea8b5d68c801fec788b411582b803463834752, and
you'll get simpler code, that works like it did before.
Maxime, since you caused the regression, can I ask you to put the
other patches on top of it, test it still works for you and submit it
properly?
[If you want to return in !test_bit case, and are sure it does not
break anything, feel free to do that.]
Thanks,
Pavel
Signed-off-by: Pavel Machek <pavel@....cz>
diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index b82b520..e626c8a 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -11,39 +11,23 @@
#include <linux/of.h>
#include <linux/input.h>
-#include <linux/input/mt.h>
#include <linux/input/touchscreen.h>
-static u32 of_get_optional_u32(struct device_node *np,
- const char *property)
-{
- u32 val = 0;
-
- of_property_read_u32(np, property, &val);
-
- return val;
-}
-
static void touchscreen_set_params(struct input_dev *dev,
unsigned long axis,
- int max, int fuzz)
+ char *max, char *fuzz)
{
struct input_absinfo *absinfo;
+ struct device_node *np = dev->dev.parent->of_node;
if (!test_bit(axis, dev->absbit)) {
- /*
- * Emit a warning only if the axis is not a multitouch
- * axis, which might not be set by the driver.
- */
- if (!input_is_mt_axis(axis))
- dev_warn(&dev->dev,
- "DT specifies parameters but the axis is not set up\n");
- return;
+ dev_warn(&dev->dev,
+ "DT specifies parameters but the axis is not set up\n");
}
absinfo = &dev->absinfo[axis];
- absinfo->maximum = max;
- absinfo->fuzz = fuzz;
+ of_property_read_u32(np, max, &absinfo->maximum);
+ of_property_read_u32(np, fuzz, &absinfo->fuzz);
}
/**
@@ -56,32 +40,14 @@ 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;
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);
- }
-
- 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);
- }
-
- 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);
- }
+ touchscreen_set_params(dev, ABS_X, "touchscreen-size-x", "touchscreen-fuzz-x");
+ touchscreen_set_params(dev, ABS_Y, "touchscreen-size-y", "touchscreen-fuzz-y");
+ touchscreen_set_params(dev, ABS_PRESSURE, "touchscreen-max-pressure", "touchscreen-fuzz-pressure");
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists