| Message ID | 20150601174730.GA30024@dtor-ws (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 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
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
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.
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
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
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.
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