Re: Regression since commit 92bac83

[Pali RohÃr]

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/