* [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code
2019-06-23 6:31 [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16() Dmitry Torokhov
@ 2019-06-23 6:31 ` Dmitry Torokhov
2019-06-23 7:59 ` Andy Shevchenko
2019-06-28 17:37 ` Benoit Parrot
2019-06-23 8:00 ` [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16() Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2019-06-23 6:31 UTC (permalink / raw)
To: linux-input; +Cc: Andy Shevchenko, Marco Felsch, Benoit Parrot, linux-kernel
Now that input_mt_report_slot_state() returns true if slot is active we no
longer need a temporary for the slot state.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/edt-ft5x06.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index ec770226e119..3cc4341bbdff 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -229,7 +229,6 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
for (i = 0; i < tsdata->max_support_points; i++) {
u8 *buf = &rdbuf[i * tplen + offset];
- bool down;
type = buf[0] >> 6;
/* ignore Reserved events */
@@ -247,16 +246,12 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
swap(x, y);
id = (buf[2] >> 4) & 0x0f;
- down = type != TOUCH_EVENT_UP;
input_mt_slot(tsdata->input, id);
- input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, down);
-
- if (!down)
- continue;
-
- touchscreen_report_pos(tsdata->input, &tsdata->prop, x, y,
- true);
+ if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,
+ type != TOUCH_EVENT_UP))
+ touchscreen_report_pos(tsdata->input, &tsdata->prop,
+ x, y, true);
}
input_mt_report_pointer_emulation(tsdata->input, true);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code
2019-06-23 6:31 ` [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code Dmitry Torokhov
@ 2019-06-23 7:59 ` Andy Shevchenko
2019-06-30 7:05 ` Dmitry Torokhov
2019-06-28 17:37 ` Benoit Parrot
1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2019-06-23 7:59 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Marco Felsch, Benoit Parrot, Linux Kernel Mailing List
On Sun, Jun 23, 2019 at 9:31 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Now that input_mt_report_slot_state() returns true if slot is active we no
> longer need a temporary for the slot state.
> - down = type != TOUCH_EVENT_UP;
>
> input_mt_slot(tsdata->input, id);
> - input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, down);
> + if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,
> + type != TOUCH_EVENT_UP))
Can't we simple do somethink like
- down = type != TOUCH_EVENT_UP;
+ down = input_mt_report_slot_state(tsdata->input,
MT_TOOL_FINGER, type != TOUCH_EVENT_UP);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code
2019-06-23 7:59 ` Andy Shevchenko
@ 2019-06-30 7:05 ` Dmitry Torokhov
2019-07-01 10:41 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2019-06-30 7:05 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-input, Marco Felsch, Benoit Parrot, Linux Kernel Mailing List
On Sun, Jun 23, 2019 at 10:59:18AM +0300, Andy Shevchenko wrote:
> On Sun, Jun 23, 2019 at 9:31 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Now that input_mt_report_slot_state() returns true if slot is active we no
> > longer need a temporary for the slot state.
>
>
> > - down = type != TOUCH_EVENT_UP;
> >
> > input_mt_slot(tsdata->input, id);
> > - input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, down);
>
> > + if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,
> > + type != TOUCH_EVENT_UP))
>
> Can't we simple do somethink like
> - down = type != TOUCH_EVENT_UP;
> + down = input_mt_report_slot_state(tsdata->input,
> MT_TOOL_FINGER, type != TOUCH_EVENT_UP);
Why though? The temporary was needed so we did not have to repeat the
expression for "contact down" condition, and now we do not need it. The
whole change was done so that we cab remove the temporary...
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code
2019-06-30 7:05 ` Dmitry Torokhov
@ 2019-07-01 10:41 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2019-07-01 10:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Marco Felsch, Benoit Parrot, Linux Kernel Mailing List
On Sun, Jun 30, 2019 at 10:05 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Sun, Jun 23, 2019 at 10:59:18AM +0300, Andy Shevchenko wrote:
> > On Sun, Jun 23, 2019 at 9:31 AM Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > >
> > > Now that input_mt_report_slot_state() returns true if slot is active we no
> > > longer need a temporary for the slot state.
> > > - down = type != TOUCH_EVENT_UP;
> > >
> > > input_mt_slot(tsdata->input, id);
> > > - input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, down);
> >
> > > + if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,
> > > + type != TOUCH_EVENT_UP))
> >
> > Can't we simple do somethink like
> > - down = type != TOUCH_EVENT_UP;
> > + down = input_mt_report_slot_state(tsdata->input,
> > MT_TOOL_FINGER, type != TOUCH_EVENT_UP);
>
> Why though? The temporary was needed so we did not have to repeat the
> expression for "contact down" condition, and now we do not need it. The
> whole change was done so that we cab remove the temporary...
I see. Thanks for explanation.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code
2019-06-23 6:31 ` [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code Dmitry Torokhov
2019-06-23 7:59 ` Andy Shevchenko
@ 2019-06-28 17:37 ` Benoit Parrot
1 sibling, 0 replies; 11+ messages in thread
From: Benoit Parrot @ 2019-06-28 17:37 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Andy Shevchenko, Marco Felsch, linux-kernel
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote on Sat [2019-Jun-22 23:31:53 -0700]:
> Now that input_mt_report_slot_state() returns true if slot is active we no
> longer need a temporary for the slot state.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Appears to work fine in my test:
Tested-by: Benoit Parrot <bparrot@ti.com>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index ec770226e119..3cc4341bbdff 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -229,7 +229,6 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>
> for (i = 0; i < tsdata->max_support_points; i++) {
> u8 *buf = &rdbuf[i * tplen + offset];
> - bool down;
>
> type = buf[0] >> 6;
> /* ignore Reserved events */
> @@ -247,16 +246,12 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> swap(x, y);
>
> id = (buf[2] >> 4) & 0x0f;
> - down = type != TOUCH_EVENT_UP;
>
> input_mt_slot(tsdata->input, id);
> - input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, down);
> -
> - if (!down)
> - continue;
> -
> - touchscreen_report_pos(tsdata->input, &tsdata->prop, x, y,
> - true);
> + if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,
> + type != TOUCH_EVENT_UP))
> + touchscreen_report_pos(tsdata->input, &tsdata->prop,
> + x, y, true);
> }
>
> input_mt_report_pointer_emulation(tsdata->input, true);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16()
2019-06-23 6:31 [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16() Dmitry Torokhov
2019-06-23 6:31 ` [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code Dmitry Torokhov
@ 2019-06-23 8:00 ` Andy Shevchenko
2019-06-25 8:44 ` David Laight
2019-06-28 17:36 ` Benoit Parrot
3 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2019-06-23 8:00 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Marco Felsch, Benoit Parrot, Linux Kernel Mailing List
On Sun, Jun 23, 2019 at 9:31 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Instead of doing conversion by hand, let's use the proper accessors.
>
The code looks fine to me,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
I can test it later next week (Wednesday or so).
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c639ebce914c..ec770226e119 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -27,6 +27,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/input/mt.h>
> #include <linux/input/touchscreen.h>
> +#include <asm/unaligned.h>
>
> #define WORK_REGISTER_THRESHOLD 0x00
> #define WORK_REGISTER_REPORT_RATE 0x08
> @@ -239,8 +240,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> if (tsdata->version == EDT_M06 && type == TOUCH_EVENT_DOWN)
> continue;
>
> - x = ((buf[0] << 8) | buf[1]) & 0x0fff;
> - y = ((buf[2] << 8) | buf[3]) & 0x0fff;
> + x = get_unaligned_be16(buf) & 0x0fff;
> + y = get_unaligned_be16(buf + 2) & 0x0fff;
> /* The FT5x26 send the y coordinate first */
> if (tsdata->version == EV_FT)
> swap(x, y);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16()
2019-06-23 6:31 [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16() Dmitry Torokhov
2019-06-23 6:31 ` [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code Dmitry Torokhov
2019-06-23 8:00 ` [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16() Andy Shevchenko
@ 2019-06-25 8:44 ` David Laight
2019-06-25 10:50 ` Andy Shevchenko
2019-06-28 17:36 ` Benoit Parrot
3 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2019-06-25 8:44 UTC (permalink / raw)
To: 'Dmitry Torokhov', linux-input
Cc: Andy Shevchenko, Marco Felsch, Benoit Parrot, linux-kernel
From: Dmitry Torokhov
> Sent: 23 June 2019 07:32
>
> Instead of doing conversion by hand, let's use the proper accessors.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c639ebce914c..ec770226e119 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -27,6 +27,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/input/mt.h>
> #include <linux/input/touchscreen.h>
> +#include <asm/unaligned.h>
>
> #define WORK_REGISTER_THRESHOLD 0x00
> #define WORK_REGISTER_REPORT_RATE 0x08
> @@ -239,8 +240,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> if (tsdata->version == EDT_M06 && type == TOUCH_EVENT_DOWN)
> continue;
>
> - x = ((buf[0] << 8) | buf[1]) & 0x0fff;
> - y = ((buf[2] << 8) | buf[3]) & 0x0fff;
> + x = get_unaligned_be16(buf) & 0x0fff;
> + y = get_unaligned_be16(buf + 2) & 0x0fff;
You might as well delete the pointless masking with 0xff.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16()
2019-06-25 8:44 ` David Laight
@ 2019-06-25 10:50 ` Andy Shevchenko
2019-06-25 13:00 ` David Laight
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2019-06-25 10:50 UTC (permalink / raw)
To: David Laight
Cc: Dmitry Torokhov, linux-input, Marco Felsch, Benoit Parrot, linux-kernel
On Tue, Jun 25, 2019 at 11:44 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Dmitry Torokhov
> > Sent: 23 June 2019 07:32
> >
> > Instead of doing conversion by hand, let's use the proper accessors.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/touchscreen/edt-ft5x06.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > index c639ebce914c..ec770226e119 100644
> > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > @@ -27,6 +27,7 @@
> > #include <linux/gpio/consumer.h>
> > #include <linux/input/mt.h>
> > #include <linux/input/touchscreen.h>
> > +#include <asm/unaligned.h>
> >
> > #define WORK_REGISTER_THRESHOLD 0x00
> > #define WORK_REGISTER_REPORT_RATE 0x08
> > @@ -239,8 +240,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> > if (tsdata->version == EDT_M06 && type == TOUCH_EVENT_DOWN)
> > continue;
> >
> > - x = ((buf[0] << 8) | buf[1]) & 0x0fff;
> > - y = ((buf[2] << 8) | buf[3]) & 0x0fff;
> > + x = get_unaligned_be16(buf) & 0x0fff;
> > + y = get_unaligned_be16(buf + 2) & 0x0fff;
>
> You might as well delete the pointless masking with 0xff.
Hmm... Does it guarantee the most significant nibble to be always 0?
(Note 16-bit value and three f:s in the mask)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16()
2019-06-25 10:50 ` Andy Shevchenko
@ 2019-06-25 13:00 ` David Laight
0 siblings, 0 replies; 11+ messages in thread
From: David Laight @ 2019-06-25 13:00 UTC (permalink / raw)
To: 'Andy Shevchenko'
Cc: Dmitry Torokhov, linux-input, Marco Felsch, Benoit Parrot, linux-kernel
From: Andy Shevchenko
> Sent: 25 June 2019 11:50
> To: David Laight
> Cc: Dmitry Torokhov; linux-input@vger.kernel.org; Marco Felsch; Benoit Parrot; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16()
>
> On Tue, Jun 25, 2019 at 11:44 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Dmitry Torokhov
> > > Sent: 23 June 2019 07:32
> > >
> > > Instead of doing conversion by hand, let's use the proper accessors.
> > >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > > drivers/input/touchscreen/edt-ft5x06.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > > index c639ebce914c..ec770226e119 100644
> > > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > > @@ -27,6 +27,7 @@
> > > #include <linux/gpio/consumer.h>
> > > #include <linux/input/mt.h>
> > > #include <linux/input/touchscreen.h>
> > > +#include <asm/unaligned.h>
> > >
> > > #define WORK_REGISTER_THRESHOLD 0x00
> > > #define WORK_REGISTER_REPORT_RATE 0x08
> > > @@ -239,8 +240,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> > > if (tsdata->version == EDT_M06 && type == TOUCH_EVENT_DOWN)
> > > continue;
> > >
> > > - x = ((buf[0] << 8) | buf[1]) & 0x0fff;
> > > - y = ((buf[2] << 8) | buf[3]) & 0x0fff;
> > > + x = get_unaligned_be16(buf) & 0x0fff;
> > > + y = get_unaligned_be16(buf + 2) & 0x0fff;
> >
> > You might as well delete the pointless masking with 0xff.
>
> Hmm... Does it guarantee the most significant nibble to be always 0?
> (Note 16-bit value and three f:s in the mask)
Sorry, I misread it :-(
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16()
2019-06-23 6:31 [PATCH 1/2] Input: edt-ft5x06 - use get_unaligned_be16() Dmitry Torokhov
` (2 preceding siblings ...)
2019-06-25 8:44 ` David Laight
@ 2019-06-28 17:36 ` Benoit Parrot
3 siblings, 0 replies; 11+ messages in thread
From: Benoit Parrot @ 2019-06-28 17:36 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Andy Shevchenko, Marco Felsch, linux-kernel
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote on Sat [2019-Jun-22 23:31:52 -0700]:
> Instead of doing conversion by hand, let's use the proper accessors.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c639ebce914c..ec770226e119 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -27,6 +27,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/input/mt.h>
> #include <linux/input/touchscreen.h>
> +#include <asm/unaligned.h>
>
> #define WORK_REGISTER_THRESHOLD 0x00
> #define WORK_REGISTER_REPORT_RATE 0x08
> @@ -239,8 +240,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> if (tsdata->version == EDT_M06 && type == TOUCH_EVENT_DOWN)
> continue;
>
> - x = ((buf[0] << 8) | buf[1]) & 0x0fff;
> - y = ((buf[2] << 8) | buf[3]) & 0x0fff;
> + x = get_unaligned_be16(buf) & 0x0fff;
> + y = get_unaligned_be16(buf + 2) & 0x0fff;
Appears to work fine in my test:
Tested-by: Benoit Parrot <bparrot@ti.com>
> /* The FT5x26 send the y coordinate first */
> if (tsdata->version == EV_FT)
> swap(x, y);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread