From: pavel@ucw.cz (Pavel Machek)
To: linux-arm-kernel@lists.infradead.org
Subject: Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen on n900.
Date: Tue, 2 Jun 2015 10:25:22 +0200 [thread overview]
Message-ID: <20150602082521.GA14955@amd> (raw)
In-Reply-To: <20150601213213.GB14395@dtor-ws>
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
next prev parent reply other threads:[~2015-06-02 8:25 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <201505241444.41039@pali>
[not found] ` <20150527132545.GA23434@amd>
[not found] ` <20150527133311.GJ30798@pali>
[not found] ` <20150527143722.GA28108@amd>
[not found] ` <20150527145837.GA13223@earth>
[not found] ` <20150529190816.GA20232@amd>
[not found] ` <20150529192505.GA28987@amd>
2015-05-29 19:32 ` [PATCH] fix n900 dts file to work around 4.1 touchscreen regression on n900 Pavel Machek
2015-05-29 19:49 ` Felipe Balbi
2015-05-29 19:56 ` Pavel Machek
2015-05-29 20:17 ` Maxime Ripard
2015-05-29 20:21 ` Felipe Balbi
2015-05-29 20:29 ` Dmitry Torokhov
2015-05-29 20:34 ` Pavel Machek
2015-05-29 20:48 ` Dmitry Torokhov
2015-05-29 21:02 ` Pavel Machek
2015-05-29 21:38 ` Dmitry Torokhov
2015-06-01 9:55 ` Maxime Ripard
[not found] ` <20150601140605.GA26908@amd>
2015-06-01 14:58 ` Please revert 3eea8b5d68c801fec788b411582b803463834752 as it breaks touchscreen " Maxime Ripard
2015-06-01 15:21 ` Pavel Machek
2015-06-01 17:47 ` Dmitry Torokhov
2015-06-01 20:27 ` Pavel Machek
2015-06-01 20:45 ` Dmitry Torokhov
2015-06-01 20:54 ` Tony Lindgren
2015-06-01 21:22 ` Maxime Ripard
2015-06-01 21:32 ` Dmitry Torokhov
2015-06-02 8:25 ` Pavel Machek [this message]
2015-06-02 9:44 ` Maxime Ripard
2015-06-02 17:58 ` Dmitry Torokhov
2015-06-02 18:08 ` Dmitry Torokhov
2015-06-25 20:25 ` Pavel Machek
2015-05-29 20:22 ` [PATCH] fix n900 dts file to work around 4.1 touchscreen regression " Dmitry Torokhov
2015-05-29 20:03 ` Maxime Ripard
2015-05-29 20:18 ` Tony Lindgren
2015-05-29 20:30 ` Pavel Machek
2015-05-30 10:14 ` Pavel Machek
2015-06-01 9:49 ` Maxime Ripard
2015-06-01 9:54 ` Pavel Machek
2015-05-30 2:21 ` Sebastian Reichel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150602082521.GA14955@amd \
--to=pavel@ucw.cz \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).