Re: ALPS DualPoint double click bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 30-07-15 17:49, Pali Rohár wrote:
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.

The == operator was already being used before this patch, and it does the job
just fine IMHO.

Regards,

Hans


 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)

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux