[2/2] Input: edt-ft5x06 - simplify event reporting code
diff mbox series

Message ID 20190623063153.261546-2-dmitry.torokhov@gmail.com
State Mainlined
Commit 17b92927f8531d1ceb7e4f10b75fc4b066ac77fc
Headers show
Series
  • [1/2] Input: edt-ft5x06 - use get_unaligned_be16()
Related show

Commit Message

Dmitry Torokhov June 23, 2019, 6:31 a.m. UTC
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(-)

Comments

Andy Shevchenko June 23, 2019, 7:59 a.m. UTC | #1
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);
Benoit Parrot June 28, 2019, 5:37 p.m. UTC | #2
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
>
Dmitry Torokhov June 30, 2019, 7:05 a.m. UTC | #3
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.
Andy Shevchenko July 1, 2019, 10:41 a.m. UTC | #4
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.

Patch
diff mbox series

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);