Re: [PATCH v2 1/2] Input: psmouse - add a custom serio protocol to send extra information
From: Benjamin Tissoires
Date: Wed Feb 08 2017 - 12:53:51 EST
On Feb 08 2017 or thereabouts, Dmitry Torokhov wrote:
> On Tue, Feb 07, 2017 at 05:25:38PM +0100, Benjamin Tissoires wrote:
> > The tracksticks on the Lenovo thinkpads have their buttons connected
> > through the touchpad device. We already fixed that in synaptics.c, but
> > when we switch the device into RMI4 mode to have proper support, the
> > pass-through functionality can't deal with them easily.
> >
> > We add a new PS/2 flag and protocol designed for psmouse.
> > The RMI4 F03 pass-through can then emit a special set of commands
> > to notify psmouse the state of the buttons.
> >
> > This patch implements the protocol in psmouse, while an other will
> > do the same for rmi4-f03.
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
>
> Looks mostly good, but how about this one?
Thanks for the fix. There is one missing input_sync() in the OOB_DATA
case:
>
> Thanks.
>
> --
> Dmitry
>
>
> Input: psmouse - add a custom serio protocol to send extra information
>
> From: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
>
> The tracksticks on the Lenovo thinkpads have their buttons connected
> through the touchpad device. We already fixed that in synaptics.c, but
> when we switch the device into RMI4 mode to have proper support, the
> pass-through functionality can't deal with them easily.
>
> We add a new PS/2 flag and protocol designed for psmouse. The RMI4 F03
> pass-through can then emit a special set of commands to notify psmouse the
> state of the buttons.
>
> This patch implements the protocol in psmouse, while an other will
> do the same for rmi4-f03.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
> Patchwork-Id: 9560567
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
> drivers/input/mouse/psmouse-base.c | 39 +++++++++++++++++++++++++++++++++---
> drivers/input/mouse/psmouse.h | 5 +++++
> include/uapi/linux/serio.h | 7 ++++--
> 3 files changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index bee267424972..98a34105d5b0 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -127,6 +127,13 @@ struct psmouse_protocol {
> int (*init)(struct psmouse *);
> };
>
> +static void psmouse_report_standard_buttons(struct input_dev *dev, u8 buttons)
> +{
> + input_report_key(dev, BTN_LEFT, buttons & BIT(0));
> + input_report_key(dev, BTN_MIDDLE, buttons & BIT(2));
> + input_report_key(dev, BTN_RIGHT, buttons & BIT(1));
> +}
> +
> /*
> * psmouse_process_byte() analyzes the PS/2 data stream and reports
> * relevant events to the input module once full packet has arrived.
> @@ -199,9 +206,8 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
> }
>
> /* Generic PS/2 Mouse */
> - input_report_key(dev, BTN_LEFT, packet[0] & 1);
> - input_report_key(dev, BTN_MIDDLE, (packet[0] >> 2) & 1);
> - input_report_key(dev, BTN_RIGHT, (packet[0] >> 1) & 1);
> + psmouse_report_standard_buttons(dev,
> + packet[0] | psmouse->extra_buttons);
>
> input_report_rel(dev, REL_X, packet[1] ? (int) packet[1] - (int) ((packet[0] << 4) & 0x100) : 0);
> input_report_rel(dev, REL_Y, packet[2] ? (int) ((packet[0] << 3) & 0x100) - (int) packet[2] : 0);
> @@ -282,6 +288,28 @@ static int psmouse_handle_byte(struct psmouse *psmouse)
> return 0;
> }
>
> +static void psmouse_handle_oob_data(struct psmouse *psmouse, u8 data)
> +{
> + switch (psmouse->oob_data_type) {
> + case PSMOUSE_OOB_NONE:
> + psmouse->oob_data_type = data;
> + break;
> +
> + case PSMOUSE_OOB_EXTRA_BTNS:
> + psmouse_report_standard_buttons(psmouse->dev, data);
We should probably call input_sync(psmouse->dev) here or the buttons are
not forwarded until the trackstick gets touched.
With that fix:
Tested-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
Cheers,
Benjamin
PS: I won't have the time to work on the other issues of the series this
week unfortunately :(
> + psmouse->extra_buttons = data;
> + psmouse->oob_data_type = PSMOUSE_OOB_NONE;
> + break;
> +
> + default:
> + psmouse_warn(psmouse,
> + "unknown OOB_DATA type: 0x%02x\n",
> + psmouse->oob_data_type);
> + psmouse->oob_data_type = PSMOUSE_OOB_NONE;
> + break;
> + }
> +}
> +
> /*
> * psmouse_interrupt() handles incoming characters, either passing them
> * for normal processing or gathering them as command response.
> @@ -306,6 +334,11 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
> goto out;
> }
>
> + if (flags & SERIO_OOB_DATA) {
> + psmouse_handle_oob_data(psmouse, data);
> + goto out;
> + }
> +
> if (unlikely(psmouse->ps2dev.flags & PS2_FLAG_ACK))
> if (ps2_handle_ack(&psmouse->ps2dev, data))
> goto out;
> diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
> index e0ca6cda3d16..8c83b8e2505c 100644
> --- a/drivers/input/mouse/psmouse.h
> +++ b/drivers/input/mouse/psmouse.h
> @@ -1,6 +1,9 @@
> #ifndef _PSMOUSE_H
> #define _PSMOUSE_H
>
> +#define PSMOUSE_OOB_NONE 0x00
> +#define PSMOUSE_OOB_EXTRA_BTNS 0x01
> +
> #define PSMOUSE_CMD_SETSCALE11 0x00e6
> #define PSMOUSE_CMD_SETSCALE21 0x00e7
> #define PSMOUSE_CMD_SETRES 0x10e8
> @@ -53,6 +56,8 @@ struct psmouse {
> unsigned char pktcnt;
> unsigned char pktsize;
> unsigned char type;
> + unsigned char oob_data_type;
> + unsigned char extra_buttons;
> bool ignore_parity;
> bool acks_disable_command;
> unsigned int model;
> diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h
> index f2447a83ac8d..ccd0ccd00f47 100644
> --- a/include/uapi/linux/serio.h
> +++ b/include/uapi/linux/serio.h
> @@ -17,9 +17,10 @@
> /*
> * bit masks for use in "interrupt" flags (3rd argument)
> */
> -#define SERIO_TIMEOUT 1
> -#define SERIO_PARITY 2
> -#define SERIO_FRAME 4
> +#define SERIO_TIMEOUT BIT(0)
> +#define SERIO_PARITY BIT(1)
> +#define SERIO_FRAME BIT(2)
> +#define SERIO_OOB_DATA BIT(3)
>
> /*
> * Serio types