Re: Regression since commit 92bac83

From: Pali RohÃr
Date: Wed Oct 21 2015 - 04:19:50 EST


On Tuesday 20 October 2015 13:39:05 Hans de Goede wrote:
> From 5d21a8004260c3e6287bde81c2a9e8f80144e77c Mon Sep 17 00:00:00 2001
> From: Hans de Goede <hdegoede@xxxxxxxxxx>
> Date: Tue, 20 Oct 2015 11:12:07 +0200
> Subject: [PATCH] alps: Only the Dell Latitude D420/430/620/630 have separate
> stick button bits
>
> commit 92bac83dd79e ("Input: alps - non interleaved V2 dualpoint has
> separate stick button bits") assumes that all alps v2 non-interleaved
> dual point setups have the separate stick button bits.
>
> Later we limited this to Dell laptops only because of reports that this
> broke things on non Dell laptops. Now it turns out that this breaks things
> on the Dell Latitude D600 too. So it seems that only the Dell Latitude
> D420/430/620/630, which all share the same touchpad / stick combo,
> have these separate bits.
>
> This patch limits the checking of the separate bits to only these models
> fixing regressions with other models.
>
> Reported-and-tested-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Tested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/input/mouse/alps.c | 48 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 4d24686..41e6cb5 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -100,7 +100,7 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
> #define ALPS_FOUR_BUTTONS 0x40 /* 4 direction button present */
> #define ALPS_PS2_INTERLEAVED 0x80 /* 3-byte PS/2 packet interleaved with
> 6-byte ALPS packet */
> -#define ALPS_DELL 0x100 /* device is a Dell laptop */
> +#define ALPS_STICK_BITS 0x100 /* separate stick button bits */
> #define ALPS_BUTTONPAD 0x200 /* device is a clickpad */
>
> static const struct alps_model_info alps_model_data[] = {
> @@ -159,6 +159,43 @@ static const struct alps_protocol_info alps_v8_protocol_data = {
> ALPS_PROTO_V8, 0x18, 0x18, 0
> };
>
> +/*
> + * Some v2 models report the stick buttons in separate bits
> + */
> +static const struct dmi_system_id alps_dmi_has_separate_stick_buttons[] = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> + {
> + /* Extrapolated from other entries */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D420"),
> + },
> + },
> + {
> + /* Reported-by: Hans de Bruin <jmdebruin@xxxxxxxxx> */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D430"),
> + },
> + },
> + {
> + /* Reported-by: Hans de Goede <hdegoede@xxxxxxxxxx> */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D620"),
> + },
> + },
> + {
> + /* Extrapolated from other entries */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude D630"),
> + },
> + },
> +#endif
> + { }
> +};
> +

Hi! Are you sure that above machines do not have variants without
ALPS_DUALPOINT or without ALPS_PASS? Because if yes, then we could see
another break.

> static void alps_set_abs_params_st(struct alps_data *priv,
> struct input_dev *dev1);
> static void alps_set_abs_params_semi_mt(struct alps_data *priv,
> @@ -253,9 +290,8 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
> return;
> }
>
> - /* Dell non interleaved V2 dualpoint has separate stick button bits */
> - if (priv->proto_version == ALPS_PROTO_V2 &&
> - priv->flags == (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) {
> + /* Some models have separate stick button bits */
> + if (priv->flags & ALPS_STICK_BITS) {

Previous code checked at this place if device has also flags ALPS_PASS
and ALPS_DUALPOINT. Now ALPS_STICK_BITS is defined only if ALPS_PROTO_V2
and DMI has specific Dell machine.

> left |= packet[0] & 1;
> right |= packet[0] & 2;
> middle |= packet[0] & 4;
> @@ -2552,8 +2588,6 @@ static int alps_set_protocol(struct psmouse *psmouse,
> priv->byte0 = protocol->byte0;
> priv->mask0 = protocol->mask0;
> priv->flags = protocol->flags;
> - if (dmi_name_in_vendors("Dell"))
> - priv->flags |= ALPS_DELL;
>
> priv->x_max = 2000;
> priv->y_max = 1400;
> @@ -2568,6 +2602,8 @@ static int alps_set_protocol(struct psmouse *psmouse,
> priv->set_abs_params = alps_set_abs_params_st;
> priv->x_max = 1023;
> priv->y_max = 767;
> + if (dmi_check_system(alps_dmi_has_separate_stick_buttons))
> + priv->flags |= ALPS_STICK_BITS;
> break;
>
> case ALPS_PROTO_V3:

--
Pali RohÃr
pali.rohar@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/