Re: [PATCH] ACPI: EC: Set ec_no_wakeup for Lenovo Go S

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

 



On Tue, 1 Apr 2025 at 04:40, Mario Limonciello <superm1@xxxxxxxxxx> wrote:
>
> On 3/31/2025 4:16 PM, Antheas Kapenekakis wrote:
> > On Mon, 31 Mar 2025 at 22:55, Mario Limonciello <superm1@xxxxxxxxxx> wrote:
> >>
> >> On 3/31/2025 3:53 PM, Antheas Kapenekakis wrote:
> >>> On Mon, 31 Mar 2025 at 22:51, Antheas Kapenekakis <lkml@xxxxxxxxxxx> wrote:
> >>>>
> >>>> On Mon, 31 Mar 2025 at 22:44, Mario Limonciello <superm1@xxxxxxxxxx> wrote:
> >>>>>
> >>>>> From: Mario Limonciello <mario.limonciello@xxxxxxx>
> >>>>>
> >>>>> When AC adapter is unplugged or plugged in EC wakes from
> >>>>> HW sleep but APU doesn't enter back into HW sleep.
> >>>>>
> >>>>> The reason this hapens is that when APU exits HW sleep the power
> >>>>> rails the EC controls will power up the TCON.  The TCON has a
> >>>>> GPIO that will be toggled during this time.  The GPIO is not marked
> >>>>> as a wakeup source however GPIO controller still has an unserviced
> >>>>> interrupt and it will block entering HW sleep again. Clearing the
> >>>>> GPIO doesn't help, the TCON raises it again until it's been initialized
> >>>>> by i2c-hid.
> >>>>>
> >>>>> Fixing this would require TCON F/W changes and it's already broken
> >>>>> in the wild on production hardware.
> >>>>>
> >>>>> To avoid triggering this issue add a quirk to avoid letting EC wake
> >>>>> up system at all.  The power button still works properly on this system.
> >>>>
> >>>> Hi Mario,
> >>>> I reported this issue to you early in January, did all the debugging
> >>>> for it, found the cause, made this patch, tested it, and finally
> >>>> deployed it as well. Then sent it to Xino.
> >>>>
> >>>> Then you pushed back for perfectly valid reasons, and we had a
> >>>> multi-week long back and forth trying to find the proper cause for
> >>>> this.
> >>>>
> >>>> So from my side I do not get why I am just a reported-by here. This is
> >>>> my patch. We also had a discussion about this out of band.
> >>>>
> >>>> Antheas
> >>>
> >>> It is interesting you ended up finding the cause. Which makes
> >>> attributing this a bit murkier.
> >>>
> >>> Antheas
> >>
> >> Hi Antheas,
> >>
> >> FWIW - you and I separately created very similar patches.
> >
> > There is only one way to write a no_ec_wakeup patch after I reported
> > to you that it fixes it, here [1] mine is the same, even the DMI order
> > is the same.
>
> You mean the same order as my patch for GoS that applied to acp-6x?
>
> commit b9a8ea185f3f8 ("ASoC: acp: Support microphone from Lenovo Go S")

Yes, which was submitted, accepted, and prior work at the time, so you
do not get to be credited for it as part of this patch as well.

> >
> >> There has been more debugging that is not public (as you can see from
> >> the content of this commit message).
> >>
> >> What tag would you like in this case?
> >
> > I think co-developed-by since you also worked very hard on fixing
> > this. I do not know if b4 picks up co-developed tags.
> >
>
> Here are tags for linking to your patch development to be picked up.
>
> Link:
> https://github.com/bazzite-org/patchwork/commit/95b93b2852718ee1e808c72e6b1836da4a95fc63
> Co-developed-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>

Anyways, you got what my problem here was. You nacked and bikeshed
this patch for 2 months, and that was after I did all the background
research, testing and deployed it [1], so you could find the real
cause, which I let you do as a _professional courtesy_. Then, out of a
sudden you are the primary author on a patch I authored and you nacked
and started testing after it was done [2].

I guess a nicer way of saying this is that you make it hard to
collaborate on kernel development. When I bring up issues to you, do
the background research, bisect, and general grunt work for them, you
do a minor cleanup which is easy for you as a kernel developer, then
strip the credit for them and I have to hunt you down to get some of
it back. This is not a productive environment, I cannot work like
this.

I think this is the 6th or 8th time this happened but this time it is
particularly egregious, because you had me spend 20 hours debugging
offshoots after my patch was already done in random directions trying
to find a real cause, only to see me get dropped to a normal reported
by, and that is after I told you off very harshly because of [2].
Otherwise the reported-by might have been missing too.

In any case, there is no point in rehashing this over and over.
Authorship in this series is mostly fine now, so it can go through.

And to avoid having this conversation again, there is another Legion
Go S [3] patch you nacked and froze the testing for, so you could go
on the manhunt for the real cause of this one. But it will probably be
needed and you will find that as you get TDP controls going. So if you
want me to prepare that in a timely manner, because that one actually
needs rewriting to be posted, now is the time to say so.

Antheas

[1] https://github.com/bazzite-org/kernel-bazzite/releases/tag/6.12.12-201
[2] https://gitlab.com/evlaV/linux-integration/-/commit/6c5a3a96be9b061f07bf9a1bcc33156c932ddf67
[3] https://gitlab.freedesktop.org/drm/amd/-/issues/3929#note_2764760

> > Not that I am content with it, which you might have noticed with my
> > absence in the amd/drm issue tracker.
> >
> > So, was it the touchscreen after all? Did you verify this by tweaking
> > its firmware?
>
> Yes it's the touchscreen causing this issue.  It was confirmed by a
> hardware rework.
>
> >
> > Antheas
> >
> > [1] https://github.com/bazzite-org/patchwork/commit/95b93b2852718ee1e808c72e6b1836da4a95fc63
> >
> >
> >> Thanks,
> >>
> >>>>
> >>>>> Cc: Xino JS1 Ni <nijs1@xxxxxxxxxx>
> >>>>> Reported-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> >>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3929
> >>>>> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> >>>>> ---
> >>>>>    drivers/acpi/ec.c | 28 ++++++++++++++++++++++++++++
> >>>>>    1 file changed, 28 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> >>>>> index 8db09d81918fb..3c5f34892734e 100644
> >>>>> --- a/drivers/acpi/ec.c
> >>>>> +++ b/drivers/acpi/ec.c
> >>>>> @@ -2301,6 +2301,34 @@ static const struct dmi_system_id acpi_ec_no_wakeup[] = {
> >>>>>                           DMI_MATCH(DMI_PRODUCT_FAMILY, "103C_5336AN HP ZHAN 66 Pro"),
> >>>>>                   },
> >>>>>           },
> >>>>> +       /*
> >>>>> +        * Lenovo Legion Go S; touchscreen blocks HW sleep when woken up from EC
> >>>>> +        * https://gitlab.freedesktop.org/drm/amd/-/issues/3929
> >>>>> +        */
> >>>>> +       {
> >>>>> +               .matches = {
> >>>>> +                       DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
> >>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "83L3"),
> >>>>> +               }
> >>>>> +       },
> >>>>> +       {
> >>>>> +               .matches = {
> >>>>> +                       DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
> >>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "83N6"),
> >>>>> +               }
> >>>>> +       },
> >>>>> +       {
> >>>>> +               .matches = {
> >>>>> +                       DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
> >>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "83Q2"),
> >>>>> +               }
> >>>>> +       },
> >>>>> +       {
> >>>>> +               .matches = {
> >>>>> +                       DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
> >>>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "83Q3"),
> >>>>> +               }
> >>>>> +       },
> >>>>>           { },
> >>>>>    };
> >>>>>
> >>>>> --
> >>>>> 2.43.0
> >>>>>
> >>
>




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux