| Message ID | 20141119183921.GF37989@dtor-ws (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Dmitry, luckily I found some time right now. I haven't reverted the original large patch, but I tested your short patch and it fixes the problem for me. regards Marcus On Wed, Nov 19, 2014 at 7:39 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Wed, Nov 19, 2014 at 07:20:21PM +0100, Marcus Overhagen wrote: >> Hi, >> >> I will try reverting that, but may need until tomorrow or Friday. >> >> Just now I made a warm reboot from rc3 to rc5 and initially scrolling >> behaviour was ok, but after about a minute it went wrong and I got >> this: >> >> [ 179.705362] ------------[ cut here ]------------ >> [ 179.705382] WARNING: CPU: 3 PID: 0 at >> drivers/input/mouse/elantech.c:433 >> elantech_report_trackpoint.isra.5+0x199/0x1b0 [psmouse]() >> [ 179.705385] psmouse serio1: elantech: Unexpected trackpoint message > > Or just this for now: > > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c > index 3fcb6b3..294f613 100644 > --- a/drivers/input/mouse/elantech.c > +++ b/drivers/input/mouse/elantech.c > @@ -793,7 +793,7 @@ static int elantech_packet_check_v4(struct psmouse *psmouse) > unsigned char packet_type = packet[3] & 0x03; > bool sanity_check; > > - if ((packet[3] & 0x0f) == 0x06) > + if ((packet[3] & 0x0f) == 0x06 && etd->tp_dev) > return PACKET_TRACKPOINT; > > /* > > -- > Dmitry -- 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
Hi, Thanks Dmitry, the fix you provided will mitigate the regression. But there might be more that is going on for v4 hardware. The detection of PACKET_TRACKPOINT can be made more strict. Thank you Marcus and Benjamin for the reports. Sorry for the regression, it was not expected that lowest nibble of packet[3] is 0x6 for non-trackpoint packets. So we have two laptops where 1) according to the msb of the capabilities has no trackpoint Benjamin, can you give feedback if the laptop has actually a trackpoint/stick/ how else one calls that strange but convenient thing in the middle of the keyboard ? Marcus, do you even have a trackpoint ? 2) mouse packets are received on serio that smell like a trackpoint packet. When I look at the packet dump from marcus, It would also fail the sanity check when crc_enabled is off, but probably succeeds on marcus laptop ? Marcus, can you send the firmware and capabilities information for your laptop (it is available in the kernel messages). Could you send me a kernel message log for a period of 1 minute when using the touchpad with the debug set to 2 ? I wonder packet[3]==0x46, why the 0x40 and 0x04 bits are set and the current elantech driver does not do anything with that part of the information. The packets from Marcus are probably v4 'MOTION' packets, and since the odd behaviour when they are ignored, they at least are containing the motion information, but maybe some extra information is hidden. There are multiple of those packets in a very short timeframe (milliseconds), I wonder if all of the MOTION packets have this signature. At least now I'm happy that the warning is there, because otherwise it might be more difficult to pinpoint the cause. Thanks, Ulrik On Wed, Nov 19, 2014 at 08:23:32PM +0100, Marcus Overhagen wrote: > Date: Wed, 19 Nov 2014 20:23:32 +0100 > From: Marcus Overhagen <marcus.overhagen@gmail.com> > To: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: Benjamin Tissoires <benjamin.tissoires@gmail.com>, Ulrik De Bie > <ulrik.debie-os@e2big.org>, Hans de Goede <hdegoede@redhat.com>, > "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-input > <linux-input@vger.kernel.org>, Jiri Kosina <jkosina@suse.cz> > Subject: Re: [git pull] Input updates for 3.18-rc4 > > Hi Dmitry, > > luckily I found some time right now. I haven't reverted the original large > patch, but I tested your short patch and it fixes the problem for me. > > regards > Marcus > > On Wed, Nov 19, 2014 at 7:39 PM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > On Wed, Nov 19, 2014 at 07:20:21PM +0100, Marcus Overhagen wrote: > >> Hi, > >> > >> I will try reverting that, but may need until tomorrow or Friday. > >> > >> Just now I made a warm reboot from rc3 to rc5 and initially scrolling > >> behaviour was ok, but after about a minute it went wrong and I got > >> this: > >> > >> [ 179.705362] ------------[ cut here ]------------ > >> [ 179.705382] WARNING: CPU: 3 PID: 0 at > >> drivers/input/mouse/elantech.c:433 > >> elantech_report_trackpoint.isra.5+0x199/0x1b0 [psmouse]() > >> [ 179.705385] psmouse serio1: elantech: Unexpected trackpoint message > > > > Or just this for now: > > > > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c > > index 3fcb6b3..294f613 100644 > > --- a/drivers/input/mouse/elantech.c > > +++ b/drivers/input/mouse/elantech.c > > @@ -793,7 +793,7 @@ static int elantech_packet_check_v4(struct psmouse *psmouse) > > unsigned char packet_type = packet[3] & 0x03; > > bool sanity_check; > > > > - if ((packet[3] & 0x0f) == 0x06) > > + if ((packet[3] & 0x0f) == 0x06 && etd->tp_dev) > > return PACKET_TRACKPOINT; > > > > /* > > > > -- > > Dmitry -- 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
On Wed, Nov 19, 2014 at 10:05 PM, <ulrik.debie-os@e2big.org> wrote: > Marcus, do you even have a trackpoint ? This is a clickpad without buttons and without trackpoint. Looks similar to this image: http://i.imgur.com/5pcCxqa.jpg > Marcus, can you send the firmware and capabilities information for your laptop > (it is available in the kernel messages). Could you send me a kernel message log for a period of 1 minute when using the touchpad with the debug set to 2 ? I sent you an email. hope that helps. regards Marcus -- 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
On Wed, Nov 19, 2014 at 10:49:05PM +0100, Marcus Overhagen wrote: > On Wed, Nov 19, 2014 at 10:05 PM, <ulrik.debie-os@e2big.org> wrote: > > > Marcus, do you even have a trackpoint ? > This is a clickpad without buttons and without trackpoint. > Looks similar to this image: http://i.imgur.com/5pcCxqa.jpg > > > Marcus, can you send the firmware and capabilities information for your laptop > > (it is available in the kernel messages). Could you send me a kernel message log for a period of 1 minute when using the touchpad with the debug set to 2 ? > I sent you an email. hope that helps. > I received it, thanks. It contains both packet[3]==0x42 and packet[3]==0x46, but the touchpad part of the driver does not do anything different for the bit 0x04. I'll anaylze the dump and look for patterns in the coming days. Thanks again, Ulrik -- 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
Hi, when moving a single finger [3] seems to be one of 0x21, 0x25, 0x31, 0x35 moving two fingers [3] seems to be mostly 0x22, 0x26, 0x32, 0x36 but also sometimes it's 0x42, 0x46, 0x52, 0x56. It seems to occationally seems to switch between these two groups after touching the pad with three or more fingers, but not every time. Moving three fingers I see[3] beeing 0x26, 0x36, 0x76, 0x66 (probably more) regards Marcus -- 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
On Wed, Nov 19, 2014 at 4:05 PM, <ulrik.debie-os@e2big.org> wrote: > Hi, > > Thanks Dmitry, the fix you provided will mitigate the regression. But > there might be more that is going on for v4 hardware. The detection > of PACKET_TRACKPOINT can be made more strict. > Thank you Marcus and Benjamin for the reports. > > Sorry for the regression, it was not expected that lowest nibble of packet[3] > is 0x6 for non-trackpoint packets. > > So we have two laptops where > 1) according to the msb of the capabilities has no trackpoint > Benjamin, can you give feedback if the laptop has actually a trackpoint/stick/ > how else one calls that strange but convenient thing in the middle of the > keyboard ? Ulrik, the reporter said that there is no trackstick on his device. The exact reference is Samsung Notebook NP900X3E-A01SE. Cheers, Benjamin -- 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
Hi, On Wed, Nov 19, 2014 at 11:21:31PM +0100, Marcus Overhagen wrote: > Hi, > > when moving a single finger [3] seems to be one of 0x21, 0x25, 0x31, 0x35 > moving two fingers [3] seems to be mostly 0x22, 0x26, 0x32, 0x36 but > also sometimes it's 0x42, 0x46, 0x52, 0x56. > It seems to occationally seems to switch between these two groups > after touching the pad with three or more fingers, but not every time. > > Moving three fingers I see[3] beeing 0x26, 0x36, 0x76, 0x66 (probably more) > > regards > Marcus Ok, after some digging through the packet dump kindly provided by Marcus, it is clear that Documentation/input/elantech.txt is not correctly representing anymore the packets of the v4 hardware. There should be some 0 and 1's replaced by x because they are currently "don't know" and definitely not always 0 or 1. Example: He has 0x26,0x36,0x46,0x56,0x66,0x76 in packet[3], and the documentation had the bits as: id2 id1 id0 1 0 0 1 0 X X The bits marked with X can thus be different. But when those are changed to X==don't care then it is not trivial to differentiate them from the trackpoint that has the following signature for that byte: 0 0 ~sy ~sx 0 1 1 0 I'm considering the following change: The test t = get_unaligned_le32(&packet[0]); switch (t & ~7U) { case 0x06000030U: case 0x16008020U: case 0x26800010U: case 0x36808000U: could be moved to elantech_packet_check_v3/4() instead of the simpler test on the lowest nibble of packet[3] (and keep the etd->tp_dev check): if ((packet[3] & 0x0f) == 0x06 && etd->tp_dev) return PACKET_TRACKPOINT; I'll think a little bit more on it. Based on the packet dump I have this seems to allow a perfect discrimation between trackpoint and touchpad packets. Thanks, Regards, Ulrik -- 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
Hi Ulrik, On Tue, Nov 25, 2014 at 4:23 PM, <ulrik.debie-os@e2big.org> wrote: > Hi, > > On Wed, Nov 19, 2014 at 11:21:31PM +0100, Marcus Overhagen wrote: >> Hi, >> >> when moving a single finger [3] seems to be one of 0x21, 0x25, 0x31, 0x35 >> moving two fingers [3] seems to be mostly 0x22, 0x26, 0x32, 0x36 but >> also sometimes it's 0x42, 0x46, 0x52, 0x56. >> It seems to occationally seems to switch between these two groups >> after touching the pad with three or more fingers, but not every time. >> >> Moving three fingers I see[3] beeing 0x26, 0x36, 0x76, 0x66 (probably more) >> >> regards >> Marcus > > > Ok, after some digging through the packet dump kindly provided by Marcus, > it is clear that Documentation/input/elantech.txt is not correctly > representing anymore the packets of the v4 hardware. There should be some > 0 and 1's replaced by x because they are currently "don't know" and definitely > not always 0 or 1. > > Example: > He has 0x26,0x36,0x46,0x56,0x66,0x76 in packet[3], and the documentation had > the bits as: > id2 id1 id0 1 0 0 1 0 > X X > The bits marked with X can thus be different. But when those are changed to > X==don't care then it is not trivial to differentiate them from the trackpoint > that has the following signature for that byte: > 0 0 ~sy ~sx 0 1 1 0 > > > > I'm considering the following change: > The test > > t = get_unaligned_le32(&packet[0]); > > switch (t & ~7U) { > case 0x06000030U: > case 0x16008020U: > case 0x26800010U: > case 0x36808000U: > > could be moved to elantech_packet_check_v3/4() instead of the > simpler test on the lowest nibble of packet[3] (and keep the etd->tp_dev check): > if ((packet[3] & 0x0f) == 0x06 && etd->tp_dev) > return PACKET_TRACKPOINT; > > I'll think a little bit more on it. Based on the packet dump I have this > seems to allow a perfect discrimation between trackpoint and touchpad packets. > Thanks for the progress on these. Do you think you will be able to get something in shape before the final 3.18? Dmitry, can we either revert the current patches and reschedule them for 3.19 or at least apply one quick fix? I am starting to see a lot of people complaining about it, and I am wondering if adding this functionality in a -rc5 release was not a good idea :-/. Cheers, Benjamin -- 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
Hi Benjamin, On Wed, Nov 26, 2014 at 09:33:26AM -0500, Benjamin Tissoires wrote: > > Hi Ulrik, > > On Tue, Nov 25, 2014 at 4:23 PM, <ulrik.debie-os@e2big.org> wrote: > > Hi, > > > > On Wed, Nov 19, 2014 at 11:21:31PM +0100, Marcus Overhagen wrote: > >> Hi, > >> > >> when moving a single finger [3] seems to be one of 0x21, 0x25, 0x31, 0x35 > >> moving two fingers [3] seems to be mostly 0x22, 0x26, 0x32, 0x36 but > >> also sometimes it's 0x42, 0x46, 0x52, 0x56. > >> It seems to occationally seems to switch between these two groups > >> after touching the pad with three or more fingers, but not every time. > >> > >> Moving three fingers I see[3] beeing 0x26, 0x36, 0x76, 0x66 (probably more) > >> > >> regards > >> Marcus > > > > > > Ok, after some digging through the packet dump kindly provided by Marcus, > > it is clear that Documentation/input/elantech.txt is not correctly > > representing anymore the packets of the v4 hardware. There should be some > > 0 and 1's replaced by x because they are currently "don't know" and definitely > > not always 0 or 1. > > > > Example: > > He has 0x26,0x36,0x46,0x56,0x66,0x76 in packet[3], and the documentation had > > the bits as: > > id2 id1 id0 1 0 0 1 0 > > X X > > The bits marked with X can thus be different. But when those are changed to > > X==don't care then it is not trivial to differentiate them from the trackpoint > > that has the following signature for that byte: > > 0 0 ~sy ~sx 0 1 1 0 > > > > > > > > I'm considering the following change: > > The test > > > > t = get_unaligned_le32(&packet[0]); > > > > switch (t & ~7U) { > > case 0x06000030U: > > case 0x16008020U: > > case 0x26800010U: > > case 0x36808000U: > > > > could be moved to elantech_packet_check_v3/4() instead of the > > simpler test on the lowest nibble of packet[3] (and keep the etd->tp_dev check): > > if ((packet[3] & 0x0f) == 0x06 && etd->tp_dev) > > return PACKET_TRACKPOINT; > > > > I'll think a little bit more on it. Based on the packet dump I have this > > seems to allow a perfect discrimation between trackpoint and touchpad packets. > > > > Thanks for the progress on these. Do you think you will be able to get > something in shape before the final 3.18? > > Dmitry, can we either revert the current patches and reschedule them > for 3.19 or at least apply one quick fix? I am starting to see a lot > of people complaining about it, and I am wondering if adding this > functionality in a -rc5 release was not a good idea :-/. Dmitry has already requested Linus to pull the quick fix b54783f2de0c216115333eca1a8d3d5c3e75b I expect this fix from Dmitry to also help the person that you reported, could you confirm that ? Basically when the hardware reports no trackpoint present, it will then behave like before the trackpoint as enabled on v4. I expect some responses from people soon in order to find out if there is even a better way to fix it. Thanks, Kind regards, Ulrik > Cheers, > Benjamin > -- > 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 -- 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
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index 3fcb6b3..294f613 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -793,7 +793,7 @@ static int elantech_packet_check_v4(struct psmouse *psmouse) unsigned char packet_type = packet[3] & 0x03; bool sanity_check; - if ((packet[3] & 0x0f) == 0x06) + if ((packet[3] & 0x0f) == 0x06 && etd->tp_dev) return PACKET_TRACKPOINT; /*