Re: 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

Re: 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

[Dmitry Torokhov]

On Sat, Oct 13, 2018 at 6:32 AM Jaak Ristioja <jaak-89mTbI93R4uuvFJfX82//w@public.gmane.org> wrote:
>
> On 27.08.2018 19:22, Jaak Ristioja wrote:
> > Upgrading Linux from 4.16 to 4.17, a ThinkPad L570 started receiving
> > spurious input events, mostly right mouse button click events, but also
> > cursor jumps.
> >
> > I have not attempted to understand whether these events come from the
> > trackpoint or touchpad or some other driver, but I managed to bisect
> > this issue to commit a986c75a7df0 titled "platform/x86: thinkpad_acpi:
> > Add 2nd Fan Support for Thinkpad P50" by Alexander Kappner.
> >
> > Apparently the quirk mitigation is applied when the BIOS version begins
> > with N1. The BIOS version on the L570 in question is N1XET57W (1.35 )
> > which is probably why this commit causes the described problems on the
> > ThinkPad L570. How exactly? - I don't know.
> >
> > The issue did not reproduce when running some stable 4.17 and 4.18
> > kernels with commit a986c75a7df0 reverted.
> >
> > Please fix this for future kernels. Thanks! :)
>
> Ping. Do you need any additional information?

Sounds like we need tighter check for the quirk, maybe based on
DMI/Board name? Can we revert the offending commit for now?

Thanks.

-- 
Dmitry

Re: 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

[Henrique de Moraes Holschuh]

On Sat, 13 Oct 2018, Jaak Ristioja wrote:
> On 27.08.2018 19:22, Jaak Ristioja wrote:
> > Upgrading Linux from 4.16 to 4.17, a ThinkPad L570 started receiving
> > spurious input events, mostly right mouse button click events, but also
> > cursor jumps.
> > 
> > I have not attempted to understand whether these events come from the
> > trackpoint or touchpad or some other driver, but I managed to bisect
> > this issue to commit a986c75a7df0 titled "platform/x86: thinkpad_acpi:
> > Add 2nd Fan Support for Thinkpad P50" by Alexander Kappner.
> > 
> > Apparently the quirk mitigation is applied when the BIOS version begins
> > with N1. The BIOS version on the L570 in question is N1XET57W (1.35 )
> > which is probably why this commit causes the described problems on the
> > ThinkPad L570. How exactly? - I don't know.
> > 
> > The issue did not reproduce when running some stable 4.17 and 4.18
> > kernels with commit a986c75a7df0 reverted.
> > 
> > Please fix this for future kernels. Thanks! :)
> 
> Ping. Do you need any additional information?

It looks like it is failing to detect N1X and N1 as different models,
the bug is likely on this commit:

	commit 846a416b4630fc960c10500b4194ad504bc22d4b
	platform/x86: thinkpad_acpi: Proper model/release matching

Real life has been **extremely** busy, so I'd appreciate a patch.
Otherwise I will eventually get to it, but it might take a few
weeks(!!).

-- 
  Henrique Holschuh

Re: 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

[Henrique de Moraes Holschuh]

On Mon, 15 Oct 2018, Dmitry Torokhov wrote:
> On Sat, Oct 13, 2018 at 6:32 AM Jaak Ristioja <jaak-89mTbI93R4uuvFJfX82//w@public.gmane.org> wrote:
> > On 27.08.2018 19:22, Jaak Ristioja wrote:
> > > Upgrading Linux from 4.16 to 4.17, a ThinkPad L570 started receiving
> > > spurious input events, mostly right mouse button click events, but also
> > > cursor jumps.
> > >
> > > I have not attempted to understand whether these events come from the
> > > trackpoint or touchpad or some other driver, but I managed to bisect
> > > this issue to commit a986c75a7df0 titled "platform/x86: thinkpad_acpi:
> > > Add 2nd Fan Support for Thinkpad P50" by Alexander Kappner.
> > >
> > > Apparently the quirk mitigation is applied when the BIOS version begins
> > > with N1. The BIOS version on the L570 in question is N1XET57W (1.35 )
> > > which is probably why this commit causes the described problems on the
> > > ThinkPad L570. How exactly? - I don't know.
> > >
> > > The issue did not reproduce when running some stable 4.17 and 4.18
> > > kernels with commit a986c75a7df0 reverted.
> > >
> > > Please fix this for future kernels. Thanks! :)
> >
> > Ping. Do you need any additional information?
> 
> Sounds like we need tighter check for the quirk, maybe based on
> DMI/Board name? Can we revert the offending commit for now?

Apparently it has to be fixed properly, because if I understood the
issue correctly, every two-character quirk will trigger on *any*
three-character model that starts with those two characters.

We could revert the three-character quirk support, but this would cause
regressions as well.

-- 
  Henrique Holschuh

Re: 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

[Jouke Witteveen]

On Tue, Oct 23, 2018 at 6:06 AM Henrique de Moraes Holschuh
<hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote:
>
> On Mon, 15 Oct 2018, Dmitry Torokhov wrote:
> > On Sat, Oct 13, 2018 at 6:32 AM Jaak Ristioja <jaak-89mTbI93R4uuvFJfX82//w@public.gmane.org> wrote:
> > > On 27.08.2018 19:22, Jaak Ristioja wrote:
> > > > Upgrading Linux from 4.16 to 4.17, a ThinkPad L570 started receiving
> > > > spurious input events, mostly right mouse button click events, but also
> > > > cursor jumps.
> > > >
> > > > I have not attempted to understand whether these events come from the
> > > > trackpoint or touchpad or some other driver, but I managed to bisect
> > > > this issue to commit a986c75a7df0 titled "platform/x86: thinkpad_acpi:
> > > > Add 2nd Fan Support for Thinkpad P50" by Alexander Kappner.
> > > >
> > > > Apparently the quirk mitigation is applied when the BIOS version begins
> > > > with N1. The BIOS version on the L570 in question is N1XET57W (1.35 )
> > > > which is probably why this commit causes the described problems on the
> > > > ThinkPad L570. How exactly? - I don't know.
> > > >
> > > > The issue did not reproduce when running some stable 4.17 and 4.18
> > > > kernels with commit a986c75a7df0 reverted.
> > > >
> > > > Please fix this for future kernels. Thanks! :)
> > >
> > > Ping. Do you need any additional information?
> >
> > Sounds like we need tighter check for the quirk, maybe based on
> > DMI/Board name? Can we revert the offending commit for now?
>
> Apparently it has to be fixed properly, because if I understood the
> issue correctly, every two-character quirk will trigger on *any*
> three-character model that starts with those two characters.

This should be (and hopefully is) incorrect.
In the tpacpi_check_quirks function, equality between the quirk bios
and the actual model bios is tested. For two-character models there
are three nonzero bytes, for two character models there are only two.
Therefore, a two-character quirk should not trigger on any
three-character model.

> We could revert the three-character quirk support, but this would cause
> regressions as well.

I doubt this would fix anything. Maybe the bios id for the L570 that
is used by the driver is not the one Jaak Ristioja reported? The
detected string is reported in the dmesg output.

That said, I do wonder why the fan quirks redefine all matching macros
(TPACPI_FAN_Q*) and don't use the generic ones (TPACPI_Q*). This looks
like an easy thing to clean up.

Regards,
- Jouke

> --
>   Henrique Holschuh
>
>
> _______________________________________________
> ibm-acpi-devel mailing list
> ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

[Henrique de Moraes Holschuh]

On Tue, 23 Oct 2018, Jouke Witteveen wrote:
> > Apparently it has to be fixed properly, because if I understood the
> > issue correctly, every two-character quirk will trigger on *any*
> > three-character model that starts with those two characters.
> 
> This should be (and hopefully is) incorrect.
> In the tpacpi_check_quirks function, equality between the quirk bios
> and the actual model bios is tested. For two-character models there
> are three nonzero bytes, for two character models there are only two.

Hmm, good.  Need to check the parser that is feeding it, though.

One actually has to parse a DMI string like aaETxxWW (old models) and
aaaET????  (need to actually verify how the ending goes on three-char
models).  This for BIOS.  For EC, it used to be HT instead of ET for
H8S-based embedded controllers, maybe it changed for the new ARC-based
embedded controllers.

> That said, I do wonder why the fan quirks redefine all matching macros
> (TPACPI_FAN_Q*) and don't use the generic ones (TPACPI_Q*). This looks
> like an easy thing to clean up.

They used to have some minor differences.  A cleanup patch is likely
fine, as long as it is tested.

-- 
  Henrique Holschuh