Re: ALPS DualPoint double click bug

["Pali Rohár" <pali.rohar@gmail.com>]

On Thursday 30 July 2015 17:00:56 Hans de Goede wrote:
> Hi,
> 
> On 30-07-15 16:46, Pali Rohár wrote:
> >What about introducing new flag ALPS_<something> instead calling
> >dmi_name_in_vendors() function every time when we need to process
> >packet?
> 
> That is a good idea. Douglas can you test the attached version
> instead of the previous one please ?
> 
> Thanks & Regards,
> 
> Hans

> @@ -251,9 +253,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>  		return;
>  	}
>  
> -	/* Non interleaved V2 dualpoint has separate stick button bits */
> +	/* Dell non interleaved V2 dualpoint has separate stick button bits */
>  	if (priv->proto_version == ALPS_PROTO_V2 &&
> -	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> +	    priv->flags == (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) {

Hi again. Now I'm trying to understand what this condition means and you
probably wanted to write... priv->flags is field and so == comparator is
hard to decode and understood. Now it means that priv->flags must have
set ALPS_DELL, ALPS_PASS and ALPS_DUALPOINT and must not set ALPS_WHEEL,
ALPS_FW_BK_1, ALPS_FW_BK_2, ALPS_FOUR_BUTTONS, ALPS_PS2_INTERLEAVED,
ALPS_BUTTONPAD and all other future flags! With future flags this code
is fragile and can be easy broken in future (by introducing new flags).
Because of "Non interleaved" in description you probably wanted
something like this?

 if (priv->proto_version == ALPS_PROTO_V2 &&
     (priv->flags & (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) &&
     !(priv->flags & ALPS_PS2_INTERLEAVED))

(flags must contains ALPS_DELL, ALPS_PASS, ALPS_DUALPOINT and must not
ALPS_PS2_INTERLEAVED)

-- 
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html