* [PATCH 1/3] Input: of_touchscreen - always issue warning if axis is not set up @ 2015-07-07 0:27 Dmitry Torokhov 2015-07-07 0:27 ` [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis Dmitry Torokhov 2015-07-07 0:27 ` [PATCH 3/3] Input: of_touchscreen - switch to using device properties Dmitry Torokhov 0 siblings, 2 replies; 14+ messages in thread From: Dmitry Torokhov @ 2015-07-07 0:27 UTC (permalink / raw) To: linux-input, Maxime Ripard, Sebastian Reichel Cc: Pavel Machek, Roger Quadros, linux-kernel Do issue warning about axis that is present in device tree but not specified by the driver even in case of multi-touch axis as callers now tell us if they expect multi-touch data or not. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/touchscreen/of_touchscreen.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c index 806cd0a..759cf4b 100644 --- a/drivers/input/touchscreen/of_touchscreen.c +++ b/drivers/input/touchscreen/of_touchscreen.c @@ -39,13 +39,9 @@ static void touchscreen_set_params(struct input_dev *dev, struct input_absinfo *absinfo; 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"); + dev_warn(&dev->dev, + "DT specifies parameters but the axis %lu is not set up\n", + axis); return; } -- 2.4.3.573.g4eafbef ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-07 0:27 [PATCH 1/3] Input: of_touchscreen - always issue warning if axis is not set up Dmitry Torokhov @ 2015-07-07 0:27 ` Dmitry Torokhov 2015-07-07 9:37 ` Roger Quadros 2015-07-07 0:27 ` [PATCH 3/3] Input: of_touchscreen - switch to using device properties Dmitry Torokhov 1 sibling, 1 reply; 14+ messages in thread From: Dmitry Torokhov @ 2015-07-07 0:27 UTC (permalink / raw) To: linux-input, Maxime Ripard, Sebastian Reichel Cc: Pavel Machek, Roger Quadros, linux-kernel The binding specification says that "touchscreen-size-x" and "-y" specify horizontal and vertical resolution of the touchscreen and therefore maximum absolute coordinates should be reduced by 1 since we are starting with 0. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c index 759cf4b..50bc0f2 100644 --- a/drivers/input/touchscreen/of_touchscreen.c +++ b/drivers/input/touchscreen/of_touchscreen.c @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) axis = multitouch ? ABS_MT_POSITION_X : ABS_X; data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", - input_abs_get_max(dev, axis), + input_abs_get_max(dev, + axis) + 1, &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); + touchscreen_set_params(dev, axis, maximum - 1, 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), + input_abs_get_max(dev, + axis) + 1, &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); + touchscreen_set_params(dev, axis, maximum - 1, fuzz); axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE; data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure", -- 2.4.3.573.g4eafbef ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-07 0:27 ` [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis Dmitry Torokhov @ 2015-07-07 9:37 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-07 9:37 UTC (permalink / raw) To: Dmitry Torokhov, linux-input, Maxime Ripard, Sebastian Reichel Cc: Pavel Machek, linux-kernel Hi Dmitry, On 07/07/15 03:27, Dmitry Torokhov wrote: > The binding specification says that "touchscreen-size-x" and "-y" specify > horizontal and vertical resolution of the touchscreen and therefore maximum > absolute coordinates should be reduced by 1 since we are starting with 0. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c > index 759cf4b..50bc0f2 100644 > --- a/drivers/input/touchscreen/of_touchscreen.c > +++ b/drivers/input/touchscreen/of_touchscreen.c > @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) > > axis = multitouch ? ABS_MT_POSITION_X : ABS_X; > data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", > - input_abs_get_max(dev, axis), > + input_abs_get_max(dev, > + axis) + 1, Why do we need to pass default_value to touchscreen_get_prop_u32()? If the property doesn't exist we are not updating the parameter anyway right? > &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); > + touchscreen_set_params(dev, axis, maximum - 1, 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), > + input_abs_get_max(dev, > + axis) + 1, > &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); > + touchscreen_set_params(dev, axis, maximum - 1, fuzz); > > axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE; > data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure", > cheers, -roger ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis @ 2015-07-07 9:37 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-07 9:37 UTC (permalink / raw) To: Dmitry Torokhov, linux-input, Maxime Ripard, Sebastian Reichel Cc: Pavel Machek, linux-kernel Hi Dmitry, On 07/07/15 03:27, Dmitry Torokhov wrote: > The binding specification says that "touchscreen-size-x" and "-y" specify > horizontal and vertical resolution of the touchscreen and therefore maximum > absolute coordinates should be reduced by 1 since we are starting with 0. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c > index 759cf4b..50bc0f2 100644 > --- a/drivers/input/touchscreen/of_touchscreen.c > +++ b/drivers/input/touchscreen/of_touchscreen.c > @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) > > axis = multitouch ? ABS_MT_POSITION_X : ABS_X; > data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", > - input_abs_get_max(dev, axis), > + input_abs_get_max(dev, > + axis) + 1, Why do we need to pass default_value to touchscreen_get_prop_u32()? If the property doesn't exist we are not updating the parameter anyway right? > &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); > + touchscreen_set_params(dev, axis, maximum - 1, 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), > + input_abs_get_max(dev, > + axis) + 1, > &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); > + touchscreen_set_params(dev, axis, maximum - 1, fuzz); > > axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE; > data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure", > cheers, -roger ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-07 9:37 ` Roger Quadros (?) @ 2015-07-07 16:25 ` Dmitry Torokhov 2015-07-08 7:59 ` Roger Quadros -1 siblings, 1 reply; 14+ messages in thread From: Dmitry Torokhov @ 2015-07-07 16:25 UTC (permalink / raw) To: Roger Quadros Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel Hi Roger, On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: > Hi Dmitry, > > On 07/07/15 03:27, Dmitry Torokhov wrote: > >The binding specification says that "touchscreen-size-x" and "-y" specify > >horizontal and vertical resolution of the touchscreen and therefore maximum > >absolute coordinates should be reduced by 1 since we are starting with 0. > > > >Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > >--- > > drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > >diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c > >index 759cf4b..50bc0f2 100644 > >--- a/drivers/input/touchscreen/of_touchscreen.c > >+++ b/drivers/input/touchscreen/of_touchscreen.c > >@@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) > > > > axis = multitouch ? ABS_MT_POSITION_X : ABS_X; > > data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", > >- input_abs_get_max(dev, axis), > >+ input_abs_get_max(dev, > >+ axis) + 1, > > Why do we need to pass default_value to touchscreen_get_prop_u32()? > If the property doesn't exist we are not updating the parameter > anyway right? The binding can specify max, fuzz, both, or neither. If only one is specified we do not want to "undo" whatever the driver did (for example tsc2005 sets up the default maximums before trying to parse OF), so we fetch the current value and pass it on as default one. > > > &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); > >+ touchscreen_set_params(dev, axis, maximum - 1, 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), > >+ input_abs_get_max(dev, > >+ axis) + 1, > > &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); > >+ touchscreen_set_params(dev, axis, maximum - 1, fuzz); > > > > axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE; > > data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure", > > > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-07 16:25 ` Dmitry Torokhov @ 2015-07-08 7:59 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-08 7:59 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel Dmitry, On 07/07/15 19:25, Dmitry Torokhov wrote: > Hi Roger, > > On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: >> Hi Dmitry, >> >> On 07/07/15 03:27, Dmitry Torokhov wrote: >>> The binding specification says that "touchscreen-size-x" and "-y" specify >>> horizontal and vertical resolution of the touchscreen and therefore maximum >>> absolute coordinates should be reduced by 1 since we are starting with 0. >>> >>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >>> --- >>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c >>> index 759cf4b..50bc0f2 100644 >>> --- a/drivers/input/touchscreen/of_touchscreen.c >>> +++ b/drivers/input/touchscreen/of_touchscreen.c >>> @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) >>> >>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; >>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", >>> - input_abs_get_max(dev, axis), >>> + input_abs_get_max(dev, >>> + axis) + 1, >> >> Why do we need to pass default_value to touchscreen_get_prop_u32()? >> If the property doesn't exist we are not updating the parameter >> anyway right? > > The binding can specify max, fuzz, both, or neither. If only one is > specified we do not want to "undo" whatever the driver did (for example why not? At least that is not what touchscreen_parse_properties() does. It will update the touchscreen params if any one of the DT property is present. > tsc2005 sets up the default maximums before trying to parse OF), so we > fetch the current value and pass it on as default one. This is what would happen in tsc2005 case -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. -calls touchscreen_parse_properties() -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", 0xfff+1, &maximum) -if DT properties are missing, touchscreen_get_prop_u32() changes ABS_X_max to 0x1000 and returns false. - as data_present is false we don't do a -1 and so we have ABS_X_max changed to 0x1000. This doesn't look right. So IMO default_value parameter can be removed from touchscreen_get_prop_u32() to make things simpler. If the property doesn't exist it will not change *value and return false. cheers, -roger - > >> >>> &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); >>> + touchscreen_set_params(dev, axis, maximum - 1, 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), >>> + input_abs_get_max(dev, >>> + axis) + 1, >>> &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); >>> + touchscreen_set_params(dev, axis, maximum - 1, fuzz); >>> >>> axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE; >>> data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure", >>> >> > > Thanks. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis @ 2015-07-08 7:59 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-08 7:59 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel Dmitry, On 07/07/15 19:25, Dmitry Torokhov wrote: > Hi Roger, > > On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: >> Hi Dmitry, >> >> On 07/07/15 03:27, Dmitry Torokhov wrote: >>> The binding specification says that "touchscreen-size-x" and "-y" specify >>> horizontal and vertical resolution of the touchscreen and therefore maximum >>> absolute coordinates should be reduced by 1 since we are starting with 0. >>> >>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >>> --- >>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- >>> 1 file changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c >>> index 759cf4b..50bc0f2 100644 >>> --- a/drivers/input/touchscreen/of_touchscreen.c >>> +++ b/drivers/input/touchscreen/of_touchscreen.c >>> @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) >>> >>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; >>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", >>> - input_abs_get_max(dev, axis), >>> + input_abs_get_max(dev, >>> + axis) + 1, >> >> Why do we need to pass default_value to touchscreen_get_prop_u32()? >> If the property doesn't exist we are not updating the parameter >> anyway right? > > The binding can specify max, fuzz, both, or neither. If only one is > specified we do not want to "undo" whatever the driver did (for example why not? At least that is not what touchscreen_parse_properties() does. It will update the touchscreen params if any one of the DT property is present. > tsc2005 sets up the default maximums before trying to parse OF), so we > fetch the current value and pass it on as default one. This is what would happen in tsc2005 case -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. -calls touchscreen_parse_properties() -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", 0xfff+1, &maximum) -if DT properties are missing, touchscreen_get_prop_u32() changes ABS_X_max to 0x1000 and returns false. - as data_present is false we don't do a -1 and so we have ABS_X_max changed to 0x1000. This doesn't look right. So IMO default_value parameter can be removed from touchscreen_get_prop_u32() to make things simpler. If the property doesn't exist it will not change *value and return false. cheers, -roger - > >> >>> &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); >>> + touchscreen_set_params(dev, axis, maximum - 1, 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), >>> + input_abs_get_max(dev, >>> + axis) + 1, >>> &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); >>> + touchscreen_set_params(dev, axis, maximum - 1, fuzz); >>> >>> axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE; >>> data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure", >>> >> > > Thanks. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-08 7:59 ` Roger Quadros (?) @ 2015-07-08 15:08 ` Dmitry Torokhov 2015-07-09 8:32 ` Roger Quadros -1 siblings, 1 reply; 14+ messages in thread From: Dmitry Torokhov @ 2015-07-08 15:08 UTC (permalink / raw) To: Roger Quadros Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel On Wed, Jul 08, 2015 at 10:59:04AM +0300, Roger Quadros wrote: > Dmitry, > > On 07/07/15 19:25, Dmitry Torokhov wrote: > >Hi Roger, > > > >On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: > >>Hi Dmitry, > >> > >>On 07/07/15 03:27, Dmitry Torokhov wrote: > >>>The binding specification says that "touchscreen-size-x" and "-y" specify > >>>horizontal and vertical resolution of the touchscreen and therefore maximum > >>>absolute coordinates should be reduced by 1 since we are starting with 0. > >>> > >>>Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > >>>--- > >>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- > >>> 1 file changed, 6 insertions(+), 4 deletions(-) > >>> > >>>diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c > >>>index 759cf4b..50bc0f2 100644 > >>>--- a/drivers/input/touchscreen/of_touchscreen.c > >>>+++ b/drivers/input/touchscreen/of_touchscreen.c > >>>@@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) > >>> > >>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; > >>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", > >>>- input_abs_get_max(dev, axis), > >>>+ input_abs_get_max(dev, > >>>+ axis) + 1, > >> > >>Why do we need to pass default_value to touchscreen_get_prop_u32()? > >>If the property doesn't exist we are not updating the parameter > >>anyway right? > > > >The binding can specify max, fuzz, both, or neither. If only one is > >specified we do not want to "undo" whatever the driver did (for example > > why not? At least that is not what touchscreen_parse_properties() does. > It will update the touchscreen params if any one of the DT property > is present. Yes, but while keeping the old properties (max, fuzz) intact if they have not been specified in dts. That is why we are passing default value to touchscreen_get_prop_u32. > > >tsc2005 sets up the default maximums before trying to parse OF), so we > >fetch the current value and pass it on as default one. > > This is what would happen in tsc2005 case > > -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. > -calls touchscreen_parse_properties() > -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", > 0xfff+1, &maximum) > -if DT properties are missing, touchscreen_get_prop_u32() changes > ABS_X_max to 0x1000 and returns false. > - as data_present is false we don't do a -1 and so we have ABS_X_max > changed to 0x1000. If data_present is false we do not call input_set_abs_params() at all ans so nothing changes in input device settings. Now, if fuzz would be set up (but size was not) we would be calling input_set_abs_params(), but given that we subtract 1 from maximum returned by touchscreen_get_prop_u32() we'd be setting input_set_abs_params(input, ABS_X, 0, 0x1000 - 1, fuzz) thus preserving 0xfff value set by the driver. > > This doesn't look right. > > So IMO default_value parameter can be removed from > touchscreen_get_prop_u32() to make things simpler. > > If the property doesn't exist it will not change *value and return false. But it will change if one of the properties set. For example, if you set fuzz but not size. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-08 15:08 ` Dmitry Torokhov @ 2015-07-09 8:32 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-09 8:32 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel Dmitry, On 08/07/15 18:08, Dmitry Torokhov wrote: > On Wed, Jul 08, 2015 at 10:59:04AM +0300, Roger Quadros wrote: >> Dmitry, >> >> On 07/07/15 19:25, Dmitry Torokhov wrote: >>> Hi Roger, >>> >>> On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: >>>> Hi Dmitry, >>>> >>>> On 07/07/15 03:27, Dmitry Torokhov wrote: >>>>> The binding specification says that "touchscreen-size-x" and "-y" specify >>>>> horizontal and vertical resolution of the touchscreen and therefore maximum >>>>> absolute coordinates should be reduced by 1 since we are starting with 0. >>>>> >>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >>>>> --- >>>>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- >>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c >>>>> index 759cf4b..50bc0f2 100644 >>>>> --- a/drivers/input/touchscreen/of_touchscreen.c >>>>> +++ b/drivers/input/touchscreen/of_touchscreen.c >>>>> @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) >>>>> >>>>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; >>>>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", >>>>> - input_abs_get_max(dev, axis), >>>>> + input_abs_get_max(dev, >>>>> + axis) + 1, >>>> >>>> Why do we need to pass default_value to touchscreen_get_prop_u32()? >>>> If the property doesn't exist we are not updating the parameter >>>> anyway right? >>> >>> The binding can specify max, fuzz, both, or neither. If only one is >>> specified we do not want to "undo" whatever the driver did (for example >> >> why not? At least that is not what touchscreen_parse_properties() does. >> It will update the touchscreen params if any one of the DT property >> is present. > > Yes, but while keeping the old properties (max, fuzz) intact if they > have not been specified in dts. That is why we are passing default value > to touchscreen_get_prop_u32. > >> >>> tsc2005 sets up the default maximums before trying to parse OF), so we >>> fetch the current value and pass it on as default one. >> >> This is what would happen in tsc2005 case >> >> -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. >> -calls touchscreen_parse_properties() >> -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", >> 0xfff+1, &maximum) >> -if DT properties are missing, touchscreen_get_prop_u32() changes >> ABS_X_max to 0x1000 and returns false. >> - as data_present is false we don't do a -1 and so we have ABS_X_max >> changed to 0x1000. > > If data_present is false we do not call input_set_abs_params() at all > ans so nothing changes in input device settings. Now, if fuzz would be You are right. I missed the fact that we don't call input_set_abs_params() in data_present is false case hence my confusion. > set up (but size was not) we would be calling input_set_abs_params(), > but given that we subtract 1 from maximum returned by > touchscreen_get_prop_u32() we'd be setting input_set_abs_params(input, > ABS_X, 0, 0x1000 - 1, fuzz) thus preserving 0xfff value set by the > driver. > >> >> This doesn't look right. >> >> So IMO default_value parameter can be removed from >> touchscreen_get_prop_u32() to make things simpler. >> >> If the property doesn't exist it will not change *value and return false. > > But it will change if one of the properties set. For example, if you set > fuzz but not size. Yes. Thanks for explaining :). cheers, -roger ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis @ 2015-07-09 8:32 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-09 8:32 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel Dmitry, On 08/07/15 18:08, Dmitry Torokhov wrote: > On Wed, Jul 08, 2015 at 10:59:04AM +0300, Roger Quadros wrote: >> Dmitry, >> >> On 07/07/15 19:25, Dmitry Torokhov wrote: >>> Hi Roger, >>> >>> On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: >>>> Hi Dmitry, >>>> >>>> On 07/07/15 03:27, Dmitry Torokhov wrote: >>>>> The binding specification says that "touchscreen-size-x" and "-y" specify >>>>> horizontal and vertical resolution of the touchscreen and therefore maximum >>>>> absolute coordinates should be reduced by 1 since we are starting with 0. >>>>> >>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >>>>> --- >>>>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- >>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c >>>>> index 759cf4b..50bc0f2 100644 >>>>> --- a/drivers/input/touchscreen/of_touchscreen.c >>>>> +++ b/drivers/input/touchscreen/of_touchscreen.c >>>>> @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) >>>>> >>>>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; >>>>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", >>>>> - input_abs_get_max(dev, axis), >>>>> + input_abs_get_max(dev, >>>>> + axis) + 1, >>>> >>>> Why do we need to pass default_value to touchscreen_get_prop_u32()? >>>> If the property doesn't exist we are not updating the parameter >>>> anyway right? >>> >>> The binding can specify max, fuzz, both, or neither. If only one is >>> specified we do not want to "undo" whatever the driver did (for example >> >> why not? At least that is not what touchscreen_parse_properties() does. >> It will update the touchscreen params if any one of the DT property >> is present. > > Yes, but while keeping the old properties (max, fuzz) intact if they > have not been specified in dts. That is why we are passing default value > to touchscreen_get_prop_u32. > >> >>> tsc2005 sets up the default maximums before trying to parse OF), so we >>> fetch the current value and pass it on as default one. >> >> This is what would happen in tsc2005 case >> >> -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. >> -calls touchscreen_parse_properties() >> -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", >> 0xfff+1, &maximum) >> -if DT properties are missing, touchscreen_get_prop_u32() changes >> ABS_X_max to 0x1000 and returns false. >> - as data_present is false we don't do a -1 and so we have ABS_X_max >> changed to 0x1000. > > If data_present is false we do not call input_set_abs_params() at all > ans so nothing changes in input device settings. Now, if fuzz would be You are right. I missed the fact that we don't call input_set_abs_params() in data_present is false case hence my confusion. > set up (but size was not) we would be calling input_set_abs_params(), > but given that we subtract 1 from maximum returned by > touchscreen_get_prop_u32() we'd be setting input_set_abs_params(input, > ABS_X, 0, 0x1000 - 1, fuzz) thus preserving 0xfff value set by the > driver. > >> >> This doesn't look right. >> >> So IMO default_value parameter can be removed from >> touchscreen_get_prop_u32() to make things simpler. >> >> If the property doesn't exist it will not change *value and return false. > > But it will change if one of the properties set. For example, if you set > fuzz but not size. Yes. Thanks for explaining :). cheers, -roger ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-09 8:32 ` Roger Quadros (?) @ 2015-07-09 18:16 ` Dmitry Torokhov 2015-07-10 8:32 ` Roger Quadros -1 siblings, 1 reply; 14+ messages in thread From: Dmitry Torokhov @ 2015-07-09 18:16 UTC (permalink / raw) To: Roger Quadros Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel On Thu, Jul 09, 2015 at 11:32:11AM +0300, Roger Quadros wrote: > Dmitry, > > On 08/07/15 18:08, Dmitry Torokhov wrote: > > On Wed, Jul 08, 2015 at 10:59:04AM +0300, Roger Quadros wrote: > >> Dmitry, > >> > >> On 07/07/15 19:25, Dmitry Torokhov wrote: > >>> Hi Roger, > >>> > >>> On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: > >>>> Hi Dmitry, > >>>> > >>>> On 07/07/15 03:27, Dmitry Torokhov wrote: > >>>>> The binding specification says that "touchscreen-size-x" and "-y" specify > >>>>> horizontal and vertical resolution of the touchscreen and therefore maximum > >>>>> absolute coordinates should be reduced by 1 since we are starting with 0. > >>>>> > >>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > >>>>> --- > >>>>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- > >>>>> 1 file changed, 6 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c > >>>>> index 759cf4b..50bc0f2 100644 > >>>>> --- a/drivers/input/touchscreen/of_touchscreen.c > >>>>> +++ b/drivers/input/touchscreen/of_touchscreen.c > >>>>> @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) > >>>>> > >>>>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; > >>>>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", > >>>>> - input_abs_get_max(dev, axis), > >>>>> + input_abs_get_max(dev, > >>>>> + axis) + 1, > >>>> > >>>> Why do we need to pass default_value to touchscreen_get_prop_u32()? > >>>> If the property doesn't exist we are not updating the parameter > >>>> anyway right? > >>> > >>> The binding can specify max, fuzz, both, or neither. If only one is > >>> specified we do not want to "undo" whatever the driver did (for example > >> > >> why not? At least that is not what touchscreen_parse_properties() does. > >> It will update the touchscreen params if any one of the DT property > >> is present. > > > > Yes, but while keeping the old properties (max, fuzz) intact if they > > have not been specified in dts. That is why we are passing default value > > to touchscreen_get_prop_u32. > > > >> > >>> tsc2005 sets up the default maximums before trying to parse OF), so we > >>> fetch the current value and pass it on as default one. > >> > >> This is what would happen in tsc2005 case > >> > >> -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. > >> -calls touchscreen_parse_properties() > >> -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", > >> 0xfff+1, &maximum) > >> -if DT properties are missing, touchscreen_get_prop_u32() changes > >> ABS_X_max to 0x1000 and returns false. > >> - as data_present is false we don't do a -1 and so we have ABS_X_max > >> changed to 0x1000. > > > > If data_present is false we do not call input_set_abs_params() at all > > ans so nothing changes in input device settings. Now, if fuzz would be > > You are right. I missed the fact that we don't call input_set_abs_params() > in data_present is false case hence my confusion. > > > set up (but size was not) we would be calling input_set_abs_params(), > > but given that we subtract 1 from maximum returned by > > touchscreen_get_prop_u32() we'd be setting input_set_abs_params(input, > > ABS_X, 0, 0x1000 - 1, fuzz) thus preserving 0xfff value set by the > > driver. > > > >> > >> This doesn't look right. > >> > >> So IMO default_value parameter can be removed from > >> touchscreen_get_prop_u32() to make things simpler. > >> > >> If the property doesn't exist it will not change *value and return false. > > > > But it will change if one of the properties set. For example, if you set > > fuzz but not size. > > Yes. Thanks for explaining :). No problem. Can I put you down as "reviewed-by" on the of_touchscreen changes then? Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis 2015-07-09 18:16 ` Dmitry Torokhov @ 2015-07-10 8:32 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-10 8:32 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel On 09/07/15 21:16, Dmitry Torokhov wrote: > On Thu, Jul 09, 2015 at 11:32:11AM +0300, Roger Quadros wrote: >> Dmitry, >> >> On 08/07/15 18:08, Dmitry Torokhov wrote: >>> On Wed, Jul 08, 2015 at 10:59:04AM +0300, Roger Quadros wrote: >>>> Dmitry, >>>> >>>> On 07/07/15 19:25, Dmitry Torokhov wrote: >>>>> Hi Roger, >>>>> >>>>> On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: >>>>>> Hi Dmitry, >>>>>> >>>>>> On 07/07/15 03:27, Dmitry Torokhov wrote: >>>>>>> The binding specification says that "touchscreen-size-x" and "-y" specify >>>>>>> horizontal and vertical resolution of the touchscreen and therefore maximum >>>>>>> absolute coordinates should be reduced by 1 since we are starting with 0. >>>>>>> >>>>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >>>>>>> --- >>>>>>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- >>>>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c >>>>>>> index 759cf4b..50bc0f2 100644 >>>>>>> --- a/drivers/input/touchscreen/of_touchscreen.c >>>>>>> +++ b/drivers/input/touchscreen/of_touchscreen.c >>>>>>> @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) >>>>>>> >>>>>>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; >>>>>>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", >>>>>>> - input_abs_get_max(dev, axis), >>>>>>> + input_abs_get_max(dev, >>>>>>> + axis) + 1, >>>>>> >>>>>> Why do we need to pass default_value to touchscreen_get_prop_u32()? >>>>>> If the property doesn't exist we are not updating the parameter >>>>>> anyway right? >>>>> >>>>> The binding can specify max, fuzz, both, or neither. If only one is >>>>> specified we do not want to "undo" whatever the driver did (for example >>>> >>>> why not? At least that is not what touchscreen_parse_properties() does. >>>> It will update the touchscreen params if any one of the DT property >>>> is present. >>> >>> Yes, but while keeping the old properties (max, fuzz) intact if they >>> have not been specified in dts. That is why we are passing default value >>> to touchscreen_get_prop_u32. >>> >>>> >>>>> tsc2005 sets up the default maximums before trying to parse OF), so we >>>>> fetch the current value and pass it on as default one. >>>> >>>> This is what would happen in tsc2005 case >>>> >>>> -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. >>>> -calls touchscreen_parse_properties() >>>> -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", >>>> 0xfff+1, &maximum) >>>> -if DT properties are missing, touchscreen_get_prop_u32() changes >>>> ABS_X_max to 0x1000 and returns false. >>>> - as data_present is false we don't do a -1 and so we have ABS_X_max >>>> changed to 0x1000. >>> >>> If data_present is false we do not call input_set_abs_params() at all >>> ans so nothing changes in input device settings. Now, if fuzz would be >> >> You are right. I missed the fact that we don't call input_set_abs_params() >> in data_present is false case hence my confusion. >> >>> set up (but size was not) we would be calling input_set_abs_params(), >>> but given that we subtract 1 from maximum returned by >>> touchscreen_get_prop_u32() we'd be setting input_set_abs_params(input, >>> ABS_X, 0, 0x1000 - 1, fuzz) thus preserving 0xfff value set by the >>> driver. >>> >>>> >>>> This doesn't look right. >>>> >>>> So IMO default_value parameter can be removed from >>>> touchscreen_get_prop_u32() to make things simpler. >>>> >>>> If the property doesn't exist it will not change *value and return false. >>> >>> But it will change if one of the properties set. For example, if you set >>> fuzz but not size. >> >> Yes. Thanks for explaining :). > > No problem. Can I put you down as "reviewed-by" on the of_touchscreen > changes then? Yes please. For all 3 patches. Reviewed-by: Roger Quadros <rogerq@ti.com> cheers, -roger ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis @ 2015-07-10 8:32 ` Roger Quadros 0 siblings, 0 replies; 14+ messages in thread From: Roger Quadros @ 2015-07-10 8:32 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Maxime Ripard, Sebastian Reichel, Pavel Machek, linux-kernel On 09/07/15 21:16, Dmitry Torokhov wrote: > On Thu, Jul 09, 2015 at 11:32:11AM +0300, Roger Quadros wrote: >> Dmitry, >> >> On 08/07/15 18:08, Dmitry Torokhov wrote: >>> On Wed, Jul 08, 2015 at 10:59:04AM +0300, Roger Quadros wrote: >>>> Dmitry, >>>> >>>> On 07/07/15 19:25, Dmitry Torokhov wrote: >>>>> Hi Roger, >>>>> >>>>> On Tue, Jul 07, 2015 at 12:37:31PM +0300, Roger Quadros wrote: >>>>>> Hi Dmitry, >>>>>> >>>>>> On 07/07/15 03:27, Dmitry Torokhov wrote: >>>>>>> The binding specification says that "touchscreen-size-x" and "-y" specify >>>>>>> horizontal and vertical resolution of the touchscreen and therefore maximum >>>>>>> absolute coordinates should be reduced by 1 since we are starting with 0. >>>>>>> >>>>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> >>>>>>> --- >>>>>>> drivers/input/touchscreen/of_touchscreen.c | 10 ++++++---- >>>>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c >>>>>>> index 759cf4b..50bc0f2 100644 >>>>>>> --- a/drivers/input/touchscreen/of_touchscreen.c >>>>>>> +++ b/drivers/input/touchscreen/of_touchscreen.c >>>>>>> @@ -71,23 +71,25 @@ void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) >>>>>>> >>>>>>> axis = multitouch ? ABS_MT_POSITION_X : ABS_X; >>>>>>> data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", >>>>>>> - input_abs_get_max(dev, axis), >>>>>>> + input_abs_get_max(dev, >>>>>>> + axis) + 1, >>>>>> >>>>>> Why do we need to pass default_value to touchscreen_get_prop_u32()? >>>>>> If the property doesn't exist we are not updating the parameter >>>>>> anyway right? >>>>> >>>>> The binding can specify max, fuzz, both, or neither. If only one is >>>>> specified we do not want to "undo" whatever the driver did (for example >>>> >>>> why not? At least that is not what touchscreen_parse_properties() does. >>>> It will update the touchscreen params if any one of the DT property >>>> is present. >>> >>> Yes, but while keeping the old properties (max, fuzz) intact if they >>> have not been specified in dts. That is why we are passing default value >>> to touchscreen_get_prop_u32. >>> >>>> >>>>> tsc2005 sets up the default maximums before trying to parse OF), so we >>>>> fetch the current value and pass it on as default one. >>>> >>>> This is what would happen in tsc2005 case >>>> >>>> -driver first sets ABS_X_max to 0xfff and ABS_Y_max to 0xfff. >>>> -calls touchscreen_parse_properties() >>>> -which calls touchscreen_get_prop_u32(dev, "touchscreen-size-x", >>>> 0xfff+1, &maximum) >>>> -if DT properties are missing, touchscreen_get_prop_u32() changes >>>> ABS_X_max to 0x1000 and returns false. >>>> - as data_present is false we don't do a -1 and so we have ABS_X_max >>>> changed to 0x1000. >>> >>> If data_present is false we do not call input_set_abs_params() at all >>> ans so nothing changes in input device settings. Now, if fuzz would be >> >> You are right. I missed the fact that we don't call input_set_abs_params() >> in data_present is false case hence my confusion. >> >>> set up (but size was not) we would be calling input_set_abs_params(), >>> but given that we subtract 1 from maximum returned by >>> touchscreen_get_prop_u32() we'd be setting input_set_abs_params(input, >>> ABS_X, 0, 0x1000 - 1, fuzz) thus preserving 0xfff value set by the >>> driver. >>> >>>> >>>> This doesn't look right. >>>> >>>> So IMO default_value parameter can be removed from >>>> touchscreen_get_prop_u32() to make things simpler. >>>> >>>> If the property doesn't exist it will not change *value and return false. >>> >>> But it will change if one of the properties set. For example, if you set >>> fuzz but not size. >> >> Yes. Thanks for explaining :). > > No problem. Can I put you down as "reviewed-by" on the of_touchscreen > changes then? Yes please. For all 3 patches. Reviewed-by: Roger Quadros <rogerq@ti.com> cheers, -roger ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] Input: of_touchscreen - switch to using device properties 2015-07-07 0:27 [PATCH 1/3] Input: of_touchscreen - always issue warning if axis is not set up Dmitry Torokhov 2015-07-07 0:27 ` [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis Dmitry Torokhov @ 2015-07-07 0:27 ` Dmitry Torokhov 1 sibling, 0 replies; 14+ messages in thread From: Dmitry Torokhov @ 2015-07-07 0:27 UTC (permalink / raw) To: linux-input, Maxime Ripard, Sebastian Reichel Cc: Pavel Machek, Roger Quadros, linux-kernel Let's switch form OF to device properties so that common parsing code could work not only on device tree but also on ACPI-based platforms. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/touchscreen/Kconfig | 4 +-- drivers/input/touchscreen/Makefile | 2 +- drivers/input/touchscreen/edt-ft5x06.c | 2 +- drivers/input/touchscreen/of_touchscreen.c | 56 ++++++++++++++++-------------- drivers/input/touchscreen/tsc2005.c | 2 +- include/linux/input/touchscreen.h | 11 ++---- 6 files changed, 37 insertions(+), 40 deletions(-) diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 2dc9d62..38d0f99 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -11,9 +11,9 @@ menuconfig INPUT_TOUCHSCREEN if INPUT_TOUCHSCREEN -config OF_TOUCHSCREEN +config TOUCHSCREEN_PROPERTIES def_tristate INPUT - depends on INPUT && OF + depends on INPUT config TOUCHSCREEN_88PM860X tristate "Marvell 88PM860x touchscreen" diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile index fa3d33b..c85aae2 100644 --- a/drivers/input/touchscreen/Makefile +++ b/drivers/input/touchscreen/Makefile @@ -6,7 +6,7 @@ wm97xx-ts-y := wm97xx-core.o -obj-$(CONFIG_OF_TOUCHSCREEN) += of_touchscreen.o +obj-$(CONFIG_TOUCHSCREEN_PROPERTIES) += of_touchscreen.o obj-$(CONFIG_TOUCHSCREEN_88PM860X) += 88pm860x-ts.o obj-$(CONFIG_TOUCHSCREEN_AD7877) += ad7877.o obj-$(CONFIG_TOUCHSCREEN_AD7879) += ad7879.o diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index 394b1de..8f8f319 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, true); + touchscreen_parse_properties(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 50bc0f2..bb6f2fe 100644 --- a/drivers/input/touchscreen/of_touchscreen.c +++ b/drivers/input/touchscreen/of_touchscreen.c @@ -9,12 +9,12 @@ * */ -#include <linux/of.h> +#include <linux/property.h> #include <linux/input.h> #include <linux/input/mt.h> #include <linux/input/touchscreen.h> -static bool touchscreen_get_prop_u32(struct device_node *np, +static bool touchscreen_get_prop_u32(struct device *dev, const char *property, unsigned int default_value, unsigned int *value) @@ -22,7 +22,7 @@ static bool touchscreen_get_prop_u32(struct device_node *np, u32 val; int error; - error = of_property_read_u32(np, property, &val); + error = device_property_read_u32(dev, property, &val); if (error) { *value = default_value; return false; @@ -51,54 +51,58 @@ static void touchscreen_set_params(struct input_dev *dev, } /** - * touchscreen_parse_of_params - parse common touchscreen DT properties - * @dev: device that should be parsed + * touchscreen_parse_properties - parse common touchscreen DT properties + * @input: input device that should be parsed + * @multitouch: specifies whether parsed properties should be applied to + * single-touch or multi-touch axes * * This function parses common DT properties for touchscreens and setups the - * input device accordingly. The function keeps previously setuped default + * input device accordingly. The function keeps previously set up default * values if no value is specified via DT. */ -void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch) +void touchscreen_parse_properties(struct input_dev *input, bool multitouch) { - struct device_node *np = dev->dev.parent->of_node; + struct device *dev = input->dev.parent; unsigned int axis; unsigned int maximum, fuzz; bool data_present; - input_alloc_absinfo(dev); - if (!dev->absinfo) + input_alloc_absinfo(input); + if (!input->absinfo) return; axis = multitouch ? ABS_MT_POSITION_X : ABS_X; - data_present = touchscreen_get_prop_u32(np, "touchscreen-size-x", - input_abs_get_max(dev, + data_present = touchscreen_get_prop_u32(dev, "touchscreen-size-x", + input_abs_get_max(input, axis) + 1, &maximum) | - touchscreen_get_prop_u32(np, "touchscreen-fuzz-x", - input_abs_get_fuzz(dev, axis), + touchscreen_get_prop_u32(dev, "touchscreen-fuzz-x", + input_abs_get_fuzz(input, axis), &fuzz); if (data_present) - touchscreen_set_params(dev, axis, maximum - 1, fuzz); + touchscreen_set_params(input, axis, maximum - 1, fuzz); axis = multitouch ? ABS_MT_POSITION_Y : ABS_Y; - data_present = touchscreen_get_prop_u32(np, "touchscreen-size-y", - input_abs_get_max(dev, + data_present = touchscreen_get_prop_u32(dev, "touchscreen-size-y", + input_abs_get_max(input, axis) + 1, &maximum) | - touchscreen_get_prop_u32(np, "touchscreen-fuzz-y", - input_abs_get_fuzz(dev, axis), + touchscreen_get_prop_u32(dev, "touchscreen-fuzz-y", + input_abs_get_fuzz(input, axis), &fuzz); if (data_present) - touchscreen_set_params(dev, axis, maximum - 1, fuzz); + touchscreen_set_params(input, axis, maximum - 1, fuzz); axis = multitouch ? ABS_MT_PRESSURE : ABS_PRESSURE; - data_present = touchscreen_get_prop_u32(np, "touchscreen-max-pressure", - input_abs_get_max(dev, axis), + data_present = touchscreen_get_prop_u32(dev, + "touchscreen-max-pressure", + input_abs_get_max(input, axis), &maximum) | - touchscreen_get_prop_u32(np, "touchscreen-fuzz-pressure", - input_abs_get_fuzz(dev, axis), + touchscreen_get_prop_u32(dev, + "touchscreen-fuzz-pressure", + input_abs_get_fuzz(input, axis), &fuzz); if (data_present) - touchscreen_set_params(dev, axis, maximum, fuzz); + touchscreen_set_params(input, axis, maximum, fuzz); } -EXPORT_SYMBOL(touchscreen_parse_of_params); +EXPORT_SYMBOL(touchscreen_parse_properties); diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c index d8c025b..aaf94752 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, false); + touchscreen_parse_properties(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 eecc9ea..c91e137 100644 --- a/include/linux/input/touchscreen.h +++ b/include/linux/input/touchscreen.h @@ -9,15 +9,8 @@ #ifndef _TOUCHSCREEN_H #define _TOUCHSCREEN_H -#include <linux/input.h> +struct input_dev; -#ifdef CONFIG_OF -void touchscreen_parse_of_params(struct input_dev *dev, bool multitouch); -#else -static inline void touchscreen_parse_of_params(struct input_dev *dev, - bool multitouch) -{ -} -#endif +void touchscreen_parse_properties(struct input_dev *dev, bool multitouch); #endif -- 2.4.3.573.g4eafbef ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-07-10 8:33 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-07 0:27 [PATCH 1/3] Input: of_touchscreen - always issue warning if axis is not set up Dmitry Torokhov 2015-07-07 0:27 ` [PATCH 2/3] Input: of_touchscreen - fix setting max values on X/Y axis Dmitry Torokhov 2015-07-07 9:37 ` Roger Quadros 2015-07-07 9:37 ` Roger Quadros 2015-07-07 16:25 ` Dmitry Torokhov 2015-07-08 7:59 ` Roger Quadros 2015-07-08 7:59 ` Roger Quadros 2015-07-08 15:08 ` Dmitry Torokhov 2015-07-09 8:32 ` Roger Quadros 2015-07-09 8:32 ` Roger Quadros 2015-07-09 18:16 ` Dmitry Torokhov 2015-07-10 8:32 ` Roger Quadros 2015-07-10 8:32 ` Roger Quadros 2015-07-07 0:27 ` [PATCH 3/3] Input: of_touchscreen - switch to using device properties Dmitry Torokhov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.