Re: [PATCH 2/7] Input: psmouse - clean up code
From: Dmitry Torokhov
Date: Mon Jun 25 2018 - 14:52:20 EST
On Mon, Jun 25, 2018 at 08:35:14PM +0200, Jiri Slaby wrote:
> On 01/20/2018, 12:06 AM, Dmitry Torokhov wrote:
> > - switch to using BIT() macros
> > - use u8 instead of unsigned char for byte data
> > - use input_set_capability() instead of manipulating capabilities bits
> > directly
> > - use sign_extend32() when extracting wheel data.
> > - do not abuse -1 as error code, propagate errors from various calls.
> â
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> â
> > @@ -157,39 +159,42 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
> â
> > case 0x00:
> > case 0xC0:
> > - input_report_rel(dev, REL_WHEEL, (int) (packet[3] & 8) - (int) (packet[3] & 7));
> > - input_report_key(dev, BTN_SIDE, (packet[3] >> 4) & 1);
> > - input_report_key(dev, BTN_EXTRA, (packet[3] >> 5) & 1);
> > + input_report_rel(dev, REL_WHEEL,
> > + -sign_extend32(packet[3], 3));
> > + input_report_key(dev, BTN_SIDE, BIT(4));
> > + input_report_key(dev, BTN_EXTRA, BIT(5));
> > break;
> > }
> > break;
> >
> > case PSMOUSE_GENPS:
> > /* Report scroll buttons on NetMice */
> > - input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
> > + input_report_rel(dev, REL_WHEEL, -(s8) packet[3]);
> >
> > /* Extra buttons on Genius NewNet 3D */
> > - input_report_key(dev, BTN_SIDE, (packet[0] >> 6) & 1);
> > - input_report_key(dev, BTN_EXTRA, (packet[0] >> 7) & 1);
> > + input_report_key(dev, BTN_SIDE, BIT(6));
> > + input_report_key(dev, BTN_EXTRA, BIT(7));
> > break;
> >
> > case PSMOUSE_THINKPS:
> > /* Extra button on ThinkingMouse */
> > - input_report_key(dev, BTN_EXTRA, (packet[0] >> 3) & 1);
> > + input_report_key(dev, BTN_EXTRA, BIT(3));
>
> While hunting a 4.17 bug where some openSUSE users lost their ability to
> mouse-click, I came across this commit. Putting aside it's a total mess
> of multiple changes, were these changes above intentional at all? I mean
> changing
> (packet[0] >> 3) & 1
> to hardwired
> BIT(3)
> does not look quite right. And if it is for some to me unknown reason,
> it should have been properly documented in the commit log. I believe
> your intent was:
> packet[0] & BIT(3)
> ?
Yes, that was, I am not sure what I was thinking; I'll fix that up.
Thanks.
--
Dmitry