2023-02-10 16:46:49

by Werner Sembach

[permalink] [raw]
Subject: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
changed the policy such that I2C touchpads may be able to wake up the
system by default if the system is configured as such.

However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
tables that the TP_ATTN# signal connected to GPIO 10 is configured as
ActiveLow and level triggered but connected to a pull up. As soon as the
system suspends the touchpad loses power and then the system wakes up.

To avoid this problem, introduce a quirk for this model that will prevent
the wakeup capability for being set for GPIO 10.

Signed-off-by: Werner Sembach <[email protected]>
Cc: [email protected] # v6.1+
---
drivers/gpio/gpiolib-acpi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index e2ab4d5253bea..82e8e43582eba 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1612,6 +1612,18 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
.ignore_wake = "ELAN0415:00@9",
},
},
+ {
+ /*
+ * Spurious wakeups from TP_ATTN# pin
+ * Found in BIOS 1.7.7
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_NAME, "NH5xAx"),
+ },
+ .driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+ .ignore_wake = "SYNA1202:00@16",
+ },
+ },
{} /* Terminating entry */
};

--
2.34.1



2023-02-10 17:05:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> changed the policy such that I2C touchpads may be able to wake up the
> system by default if the system is configured as such.
>
> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
> ActiveLow and level triggered but connected to a pull up.

I'm not sure I understand the issue here. From what you say here it seems
correct ACPI description.

> As soon as the
> system suspends the touchpad loses power and then the system wakes up.
>
> To avoid this problem, introduce a quirk for this model that will prevent
> the wakeup capability for being set for GPIO 10.

I'm not against fixing this, but wouldn't be better to actually target the root
cause and have a different quirk? Or is it me who didn't get what is the root
cause?

--
With Best Regards,
Andy Shevchenko



2023-02-13 11:30:43

by Werner Sembach

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
> On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
>> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
>> changed the policy such that I2C touchpads may be able to wake up the
>> system by default if the system is configured as such.
>>
>> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
>> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
>> ActiveLow and level triggered but connected to a pull up.
> I'm not sure I understand the issue here. From what you say here it seems
> correct ACPI description.
TBH I copied the commit description from
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
which is for a different device having the exact same problem.
>
>> As soon as the
>> system suspends the touchpad loses power and then the system wakes up.
>>
>> To avoid this problem, introduce a quirk for this model that will prevent
>> the wakeup capability for being set for GPIO 10.
> I'm not against fixing this, but wouldn't be better to actually target the root
> cause and have a different quirk? Or is it me who didn't get what is the root
> cause?
>
I missed to reference the original discussion while copying the description:
https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
it's a somewhat convoluted issue spanning multiple bugs when you scroll up from
that particular linked comment, which are however irrelevant for this patch)

I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
summing it up, as I might have misunderstood parts of it ^^

I added the other ones from there to the cc.


2023-02-13 12:42:05

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 12:30:08PM +0100, Werner Sembach wrote:
> Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
> > On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
> > > commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> > > changed the policy such that I2C touchpads may be able to wake up the
> > > system by default if the system is configured as such.
> > >
> > > However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
> > > tables that the TP_ATTN# signal connected to GPIO 10 is configured as
> > > ActiveLow and level triggered but connected to a pull up.
> > I'm not sure I understand the issue here. From what you say here it seems
> > correct ACPI description.
> TBH I copied the commit description from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
> which is for a different device having the exact same problem.

Yeah, and I reviewed that and seems paid no attention to this detail.

So, ActiveLow + PullUp is the _right_ thing to do in ACPI.
The problem seems somewhere else.

Mario, can we have an access to the schematics of the affected pin to
understand better what's going on?

Or is that description missing some crucial detail?

> > > As soon as the
> > > system suspends the touchpad loses power and then the system wakes up.
> > >
> > > To avoid this problem, introduce a quirk for this model that will prevent
> > > the wakeup capability for being set for GPIO 10.
> > I'm not against fixing this, but wouldn't be better to actually target the root
> > cause and have a different quirk? Or is it me who didn't get what is the root
> > cause?
> >
> I missed to reference the original discussion while copying the description:
> https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
> it's a somewhat convoluted issue spanning multiple bugs when you scroll up
> from that particular linked comment, which are however irrelevant for this
> patch)
>
> I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
> summing it up, as I might have misunderstood parts of it ^^

The GpioIo() and GpioInt() resources have gaps in them, due to this some
additional information is required or some heuristics is used to deduct
the settings.

All this is described in
https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.html

> I added the other ones from there to the cc.

Thank you.

--
With Best Regards,
Andy Shevchenko



2023-02-13 13:22:01

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx


On 2/13/23 06:41, Andy Shevchenko wrote:
> On Mon, Feb 13, 2023 at 12:30:08PM +0100, Werner Sembach wrote:
>> Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
>>> On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
>>>> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
>>>> changed the policy such that I2C touchpads may be able to wake up the
>>>> system by default if the system is configured as such.
>>>>
>>>> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
>>>> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
>>>> ActiveLow and level triggered but connected to a pull up.
>>> I'm not sure I understand the issue here. From what you say here it seems
>>> correct ACPI description.
>> TBH I copied the commit description from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
>> which is for a different device having the exact same problem.
> Yeah, and I reviewed that and seems paid no attention to this detail.
>
> So, ActiveLow + PullUp is the _right_ thing to do in ACPI.
> The problem seems somewhere else.
>
> Mario, can we have an access to the schematics of the affected pin to
> understand better what's going on?
>
> Or is that description missing some crucial detail?

The schematics were shared by the reporter for the original issue which
is how we reached the conclusion there was a mistake.

As they're both Clevo designs it's certainly possible they have the same
mistake in two systems.

>>>> As soon as the
>>>> system suspends the touchpad loses power and then the system wakes up.
>>>>
>>>> To avoid this problem, introduce a quirk for this model that will prevent
>>>> the wakeup capability for being set for GPIO 10.
>>> I'm not against fixing this, but wouldn't be better to actually target the root
>>> cause and have a different quirk? Or is it me who didn't get what is the root
>>> cause?
>>>
>> I missed to reference the original discussion while copying the description:
>> https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
>> it's a somewhat convoluted issue spanning multiple bugs when you scroll up
>> from that particular linked comment, which are however irrelevant for this
>> patch)
>>
>> I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
>> summing it up, as I might have misunderstood parts of it ^^
> The GpioIo() and GpioInt() resources have gaps in them, due to this some
> additional information is required or some heuristics is used to deduct
> the settings.
>
> All this is described in
> https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.html
>
>> I added the other ones from there to the cc.
> Thank you.
>

2023-02-13 14:38:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 07:20:48AM -0600, Mario Limonciello wrote:
> On 2/13/23 06:41, Andy Shevchenko wrote:
> > On Mon, Feb 13, 2023 at 12:30:08PM +0100, Werner Sembach wrote:
> > > Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
> > > > On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
> > > > > commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> > > > > changed the policy such that I2C touchpads may be able to wake up the
> > > > > system by default if the system is configured as such.
> > > > >
> > > > > However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
> > > > > tables that the TP_ATTN# signal connected to GPIO 10 is configured as
> > > > > ActiveLow and level triggered but connected to a pull up.
> > > > I'm not sure I understand the issue here. From what you say here it seems
> > > > correct ACPI description.
> > > TBH I copied the commit description from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
> > > which is for a different device having the exact same problem.
> > Yeah, and I reviewed that and seems paid no attention to this detail.
> >
> > So, ActiveLow + PullUp is the _right_ thing to do in ACPI.
> > The problem seems somewhere else.
> >
> > Mario, can we have an access to the schematics of the affected pin to
> > understand better what's going on?
> >
> > Or is that description missing some crucial detail?
>
> The schematics were shared by the reporter for the original issue which is
> how we reached the conclusion there was a mistake.
>
> As they're both Clevo designs it's certainly possible they have the same
> mistake in two systems.

Thank you!
I have looked at the schematics and read discussion.

So, the conclusion that this is a BIOS bug is incorrect in my opinion.
The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
signal for a while or on the PCB level, so that pull up should be connected
to another power source that stays on.

This means the description on the initial patch with the same issue is
incorrect.

Do we know the power sequence on the suspend to see which and how on the
time line the power sources are off/on?

> > > > > As soon as the
> > > > > system suspends the touchpad loses power and then the system wakes up.
> > > > >
> > > > > To avoid this problem, introduce a quirk for this model that will prevent
> > > > > the wakeup capability for being set for GPIO 10.
> > > > I'm not against fixing this, but wouldn't be better to actually target the root
> > > > cause and have a different quirk? Or is it me who didn't get what is the root
> > > > cause?
> > > >
> > > I missed to reference the original discussion while copying the description:
> > > https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
> > > it's a somewhat convoluted issue spanning multiple bugs when you scroll up
> > > from that particular linked comment, which are however irrelevant for this
> > > patch)
> > >
> > > I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
> > > summing it up, as I might have misunderstood parts of it ^^
> > The GpioIo() and GpioInt() resources have gaps in them, due to this some
> > additional information is required or some heuristics is used to deduct
> > the settings.
> >
> > All this is described in
> > https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.html
> >
> > > I added the other ones from there to the cc.
> > Thank you.

--
With Best Regards,
Andy Shevchenko



2023-02-13 14:48:07

by Werner Sembach

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

Am 13.02.23 um 15:37 schrieb Andy Shevchenko:
> On Mon, Feb 13, 2023 at 07:20:48AM -0600, Mario Limonciello wrote:
>> On 2/13/23 06:41, Andy Shevchenko wrote:
>>> On Mon, Feb 13, 2023 at 12:30:08PM +0100, Werner Sembach wrote:
>>>> Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
>>>>> On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
>>>>>> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
>>>>>> changed the policy such that I2C touchpads may be able to wake up the
>>>>>> system by default if the system is configured as such.
>>>>>>
>>>>>> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
>>>>>> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
>>>>>> ActiveLow and level triggered but connected to a pull up.
>>>>> I'm not sure I understand the issue here. From what you say here it seems
>>>>> correct ACPI description.
>>>> TBH I copied the commit description from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
>>>> which is for a different device having the exact same problem.
>>> Yeah, and I reviewed that and seems paid no attention to this detail.
>>>
>>> So, ActiveLow + PullUp is the _right_ thing to do in ACPI.
>>> The problem seems somewhere else.
>>>
>>> Mario, can we have an access to the schematics of the affected pin to
>>> understand better what's going on?
>>>
>>> Or is that description missing some crucial detail?

Schematics for the NH5xAx can also be found on this unofficial clevo mirror
(service manuals, scroll to end for schematics):

http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip

http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip

User: repo

PW: repo

>> The schematics were shared by the reporter for the original issue which is
>> how we reached the conclusion there was a mistake.
>>
>> As they're both Clevo designs it's certainly possible they have the same
>> mistake in two systems.
> Thank you!
> I have looked at the schematics and read discussion.
>
> So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> signal for a while or on the PCB level, so that pull up should be connected
> to another power source that stays on.
>
> This means the description on the initial patch with the same issue is
> incorrect.
>
> Do we know the power sequence on the suspend to see which and how on the
> time line the power sources are off/on?
>
>>>>>> As soon as the
>>>>>> system suspends the touchpad loses power and then the system wakes up.
>>>>>>
>>>>>> To avoid this problem, introduce a quirk for this model that will prevent
>>>>>> the wakeup capability for being set for GPIO 10.
>>>>> I'm not against fixing this, but wouldn't be better to actually target the root
>>>>> cause and have a different quirk? Or is it me who didn't get what is the root
>>>>> cause?
>>>>>
>>>> I missed to reference the original discussion while copying the description:
>>>> https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
>>>> it's a somewhat convoluted issue spanning multiple bugs when you scroll up
>>>> from that particular linked comment, which are however irrelevant for this
>>>> patch)
>>>>
>>>> I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
>>>> summing it up, as I might have misunderstood parts of it ^^
>>> The GpioIo() and GpioInt() resources have gaps in them, due to this some
>>> additional information is required or some heuristics is used to deduct
>>> the settings.
>>>
>>> All this is described in
>>> https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.html
>>>
>>>> I added the other ones from there to the cc.
>>> Thank you.

2023-02-13 17:21:07

by Raul Rangel

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

SUSB#_EN

On Mon, Feb 13, 2023 at 7:47 AM Werner Sembach <[email protected]> wrote:
>
> Am 13.02.23 um 15:37 schrieb Andy Shevchenko:
> > On Mon, Feb 13, 2023 at 07:20:48AM -0600, Mario Limonciello wrote:
> >> On 2/13/23 06:41, Andy Shevchenko wrote:
> >>> On Mon, Feb 13, 2023 at 12:30:08PM +0100, Werner Sembach wrote:
> >>>> Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
> >>>>> On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
> >>>>>> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> >>>>>> changed the policy such that I2C touchpads may be able to wake up the
> >>>>>> system by default if the system is configured as such.
> >>>>>>
> >>>>>> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
> >>>>>> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
> >>>>>> ActiveLow and level triggered but connected to a pull up.
> >>>>> I'm not sure I understand the issue here. From what you say here it seems
> >>>>> correct ACPI description.
> >>>> TBH I copied the commit description from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
> >>>> which is for a different device having the exact same problem.
> >>> Yeah, and I reviewed that and seems paid no attention to this detail.
> >>>
> >>> So, ActiveLow + PullUp is the _right_ thing to do in ACPI.
> >>> The problem seems somewhere else.
> >>>
> >>> Mario, can we have an access to the schematics of the affected pin to
> >>> understand better what's going on?
> >>>
> >>> Or is that description missing some crucial detail?
>
> Schematics for the NH5xAx can also be found on this unofficial clevo mirror
> (service manuals, scroll to end for schematics):
>
> http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
>
> http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip
>
> User: repo
>
> PW: repo
>
> >> The schematics were shared by the reporter for the original issue which is
> >> how we reached the conclusion there was a mistake.
> >>
> >> As they're both Clevo designs it's certainly possible they have the same
> >> mistake in two systems.

> > Thank you!
> > I have looked at the schematics and read discussion.
> >
> > So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> > The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> > signal for a while or on the PCB level, so that pull up should be connected
> > to another power source that stays on.
> >
> > This means the description on the initial patch with the same issue is
> > incorrect.
> >
> > Do we know the power sequence on the suspend to see which and how on the
> > time line the power sources are off/on?
> >

If you look at the load switch for 3.3VS, its EN2 pin is connected to
SUSB#_EN which is connected to SUSB# which is connected to
AND(SUSB#_PCH -> SLP_S3_L, PM_SLP_S0 -> S0A3_GPIO). So there is no
PMIC/EC firmware that is incharge of this. I guess I'm not quite sure
how they have S0A3_GPIO configured, so maybe I have an invert wrong.

The EC does control DD_ON which controls the 3.3V and 5V rails.

> >>>>>> As soon as the
> >>>>>> system suspends the touchpad loses power and then the system wakes up.
> >>>>>>
> >>>>>> To avoid this problem, introduce a quirk for this model that will prevent
> >>>>>> the wakeup capability for being set for GPIO 10.
> >>>>> I'm not against fixing this, but wouldn't be better to actually target the root
> >>>>> cause and have a different quirk? Or is it me who didn't get what is the root
> >>>>> cause?
> >>>>>
> >>>> I missed to reference the original discussion while copying the description:
> >>>> https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
> >>>> it's a somewhat convoluted issue spanning multiple bugs when you scroll up
> >>>> from that particular linked comment, which are however irrelevant for this
> >>>> patch)
> >>>>
> >>>> I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
> >>>> summing it up, as I might have misunderstood parts of it ^^
> >>> The GpioIo() and GpioInt() resources have gaps in them, due to this some
> >>> additional information is required or some heuristics is used to deduct
> >>> the settings.
> >>>
> >>> All this is described in
> >>> https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.html
> >>>
> >>>> I added the other ones from there to the cc.
> >>> Thank you.

2023-02-13 17:50:19

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 10:20:41AM -0700, Raul Rangel wrote:
> On Mon, Feb 13, 2023 at 7:47 AM Werner Sembach <[email protected]> wrote:
> > Am 13.02.23 um 15:37 schrieb Andy Shevchenko:
> > > On Mon, Feb 13, 2023 at 07:20:48AM -0600, Mario Limonciello wrote:
> > >> On 2/13/23 06:41, Andy Shevchenko wrote:
> > >>> On Mon, Feb 13, 2023 at 12:30:08PM +0100, Werner Sembach wrote:
> > >>>> Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
> > >>>>> On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
> > >>>>>> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> > >>>>>> changed the policy such that I2C touchpads may be able to wake up the
> > >>>>>> system by default if the system is configured as such.
> > >>>>>>
> > >>>>>> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
> > >>>>>> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
> > >>>>>> ActiveLow and level triggered but connected to a pull up.
> > >>>>> I'm not sure I understand the issue here. From what you say here it seems
> > >>>>> correct ACPI description.
> > >>>> TBH I copied the commit description from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
> > >>>> which is for a different device having the exact same problem.
> > >>> Yeah, and I reviewed that and seems paid no attention to this detail.
> > >>>
> > >>> So, ActiveLow + PullUp is the _right_ thing to do in ACPI.
> > >>> The problem seems somewhere else.
> > >>>
> > >>> Mario, can we have an access to the schematics of the affected pin to
> > >>> understand better what's going on?
> > >>>
> > >>> Or is that description missing some crucial detail?
> >
> > Schematics for the NH5xAx can also be found on this unofficial clevo mirror
> > (service manuals, scroll to end for schematics):
> >
> > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> >
> > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip
> >
> > User: repo
> >
> > PW: repo
> >
> > >> The schematics were shared by the reporter for the original issue which is
> > >> how we reached the conclusion there was a mistake.
> > >>
> > >> As they're both Clevo designs it's certainly possible they have the same
> > >> mistake in two systems.
>
> > > Thank you!
> > > I have looked at the schematics and read discussion.
> > >
> > > So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> > > The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> > > signal for a while or on the PCB level, so that pull up should be connected
> > > to another power source that stays on.
> > >
> > > This means the description on the initial patch with the same issue is
> > > incorrect.
> > >
> > > Do we know the power sequence on the suspend to see which and how on the
> > > time line the power sources are off/on?
>
> If you look at the load switch for 3.3VS, its EN2 pin is connected to
> SUSB#_EN which is connected to SUSB# which is connected to
> AND(SUSB#_PCH -> SLP_S3_L, PM_SLP_S0 -> S0A3_GPIO). So there is no
> PMIC/EC firmware that is incharge of this. I guess I'm not quite sure
> how they have S0A3_GPIO configured, so maybe I have an invert wrong.
>
> The EC does control DD_ON which controls the 3.3V and 5V rails.

On page 6 of the schematics I see the U7 that forms SUSB# from SUSB#_APU
(which corresponds to what you said) _and_ EC_EN, which is GPIO from IT5570,
which is EC.

Are you using different schematics? I'm using the one from FDO bug report.

> > >>>>>> As soon as the
> > >>>>>> system suspends the touchpad loses power and then the system wakes up.
> > >>>>>>
> > >>>>>> To avoid this problem, introduce a quirk for this model that will prevent
> > >>>>>> the wakeup capability for being set for GPIO 10.
> > >>>>> I'm not against fixing this, but wouldn't be better to actually target the root
> > >>>>> cause and have a different quirk? Or is it me who didn't get what is the root
> > >>>>> cause?
> > >>>>>
> > >>>> I missed to reference the original discussion while copying the description:
> > >>>> https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
> > >>>> it's a somewhat convoluted issue spanning multiple bugs when you scroll up
> > >>>> from that particular linked comment, which are however irrelevant for this
> > >>>> patch)
> > >>>>
> > >>>> I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
> > >>>> summing it up, as I might have misunderstood parts of it ^^
> > >>> The GpioIo() and GpioInt() resources have gaps in them, due to this some
> > >>> additional information is required or some heuristics is used to deduct
> > >>> the settings.
> > >>>
> > >>> All this is described in
> > >>> https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.html
> > >>>
> > >>>> I added the other ones from there to the cc.
> > >>> Thank you.

--
With Best Regards,
Andy Shevchenko



2023-02-13 17:58:06

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 07:50:02PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 13, 2023 at 10:20:41AM -0700, Raul Rangel wrote:
> > On Mon, Feb 13, 2023 at 7:47 AM Werner Sembach <[email protected]> wrote:
> > > Am 13.02.23 um 15:37 schrieb Andy Shevchenko:
> > > > On Mon, Feb 13, 2023 at 07:20:48AM -0600, Mario Limonciello wrote:
> > > >> On 2/13/23 06:41, Andy Shevchenko wrote:
> > > >>> On Mon, Feb 13, 2023 at 12:30:08PM +0100, Werner Sembach wrote:
> > > >>>> Am 10.02.23 um 18:04 schrieb Andy Shevchenko:
> > > >>>>> On Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach wrote:
> > > >>>>>> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> > > >>>>>> changed the policy such that I2C touchpads may be able to wake up the
> > > >>>>>> system by default if the system is configured as such.
> > > >>>>>>
> > > >>>>>> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
> > > >>>>>> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
> > > >>>>>> ActiveLow and level triggered but connected to a pull up.
> > > >>>>> I'm not sure I understand the issue here. From what you say here it seems
> > > >>>>> correct ACPI description.
> > > >>>> TBH I copied the commit description from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cb786180dfb5258ff3111181b5e4ecb1d4a297b
> > > >>>> which is for a different device having the exact same problem.
> > > >>> Yeah, and I reviewed that and seems paid no attention to this detail.
> > > >>>
> > > >>> So, ActiveLow + PullUp is the _right_ thing to do in ACPI.
> > > >>> The problem seems somewhere else.
> > > >>>
> > > >>> Mario, can we have an access to the schematics of the affected pin to
> > > >>> understand better what's going on?
> > > >>>
> > > >>> Or is that description missing some crucial detail?
> > >
> > > Schematics for the NH5xAx can also be found on this unofficial clevo mirror
> > > (service manuals, scroll to end for schematics):
> > >
> > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> > >
> > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip
> > >
> > > User: repo
> > >
> > > PW: repo
> > >
> > > >> The schematics were shared by the reporter for the original issue which is
> > > >> how we reached the conclusion there was a mistake.
> > > >>
> > > >> As they're both Clevo designs it's certainly possible they have the same
> > > >> mistake in two systems.
> >
> > > > Thank you!
> > > > I have looked at the schematics and read discussion.
> > > >
> > > > So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> > > > The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> > > > signal for a while or on the PCB level, so that pull up should be connected
> > > > to another power source that stays on.
> > > >
> > > > This means the description on the initial patch with the same issue is
> > > > incorrect.
> > > >
> > > > Do we know the power sequence on the suspend to see which and how on the
> > > > time line the power sources are off/on?
> >
> > If you look at the load switch for 3.3VS, its EN2 pin is connected to
> > SUSB#_EN which is connected to SUSB# which is connected to
> > AND(SUSB#_PCH -> SLP_S3_L, PM_SLP_S0 -> S0A3_GPIO). So there is no
> > PMIC/EC firmware that is incharge of this. I guess I'm not quite sure
> > how they have S0A3_GPIO configured, so maybe I have an invert wrong.
> >
> > The EC does control DD_ON which controls the 3.3V and 5V rails.
>
> On page 6 of the schematics I see the U7 that forms SUSB# from SUSB#_APU
> (which corresponds to what you said) _and_ EC_EN, which is GPIO from IT5570,
> which is EC.
>
> Are you using different schematics? I'm using the one from FDO bug report.

Just checked this one:
http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip

Also uses EC (SUSB_EC#).

> > > >>>>>> As soon as the
> > > >>>>>> system suspends the touchpad loses power and then the system wakes up.
> > > >>>>>>
> > > >>>>>> To avoid this problem, introduce a quirk for this model that will prevent
> > > >>>>>> the wakeup capability for being set for GPIO 10.
> > > >>>>> I'm not against fixing this, but wouldn't be better to actually target the root
> > > >>>>> cause and have a different quirk? Or is it me who didn't get what is the root
> > > >>>>> cause?
> > > >>>>>
> > > >>>> I missed to reference the original discussion while copying the description:
> > > >>>> https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627 (Note that
> > > >>>> it's a somewhat convoluted issue spanning multiple bugs when you scroll up
> > > >>>> from that particular linked comment, which are however irrelevant for this
> > > >>>> patch)
> > > >>>>
> > > >>>> I'm not deep into how ACPI defined IRQ work so maybe not a good idea for me
> > > >>>> summing it up, as I might have misunderstood parts of it ^^
> > > >>> The GpioIo() and GpioInt() resources have gaps in them, due to this some
> > > >>> additional information is required or some heuristics is used to deduct
> > > >>> the settings.
> > > >>>
> > > >>> All this is described in
> > > >>> https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.html
> > > >>>
> > > >>>> I added the other ones from there to the cc.
> > > >>> Thank you.

--
With Best Regards,
Andy Shevchenko



2023-02-13 18:00:13

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 07:57:55PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 13, 2023 at 07:50:02PM +0200, Andy Shevchenko wrote:
> > On Mon, Feb 13, 2023 at 10:20:41AM -0700, Raul Rangel wrote:
> > > On Mon, Feb 13, 2023 at 7:47 AM Werner Sembach <[email protected]> wrote:
> > > > Am 13.02.23 um 15:37 schrieb Andy Shevchenko:

...

> > > > Schematics for the NH5xAx can also be found on this unofficial clevo mirror
> > > > (service manuals, scroll to end for schematics):
> > > >
> > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> > > >
> > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip
> > > >
> > > > User: repo
> > > >
> > > > PW: repo
> > > >
> > > > >> The schematics were shared by the reporter for the original issue which is
> > > > >> how we reached the conclusion there was a mistake.
> > > > >>
> > > > >> As they're both Clevo designs it's certainly possible they have the same
> > > > >> mistake in two systems.
> > >
> > > > > Thank you!
> > > > > I have looked at the schematics and read discussion.
> > > > >
> > > > > So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> > > > > The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> > > > > signal for a while or on the PCB level, so that pull up should be connected
> > > > > to another power source that stays on.
> > > > >
> > > > > This means the description on the initial patch with the same issue is
> > > > > incorrect.
> > > > >
> > > > > Do we know the power sequence on the suspend to see which and how on the
> > > > > time line the power sources are off/on?
> > >
> > > If you look at the load switch for 3.3VS, its EN2 pin is connected to
> > > SUSB#_EN which is connected to SUSB# which is connected to
> > > AND(SUSB#_PCH -> SLP_S3_L, PM_SLP_S0 -> S0A3_GPIO). So there is no
> > > PMIC/EC firmware that is incharge of this. I guess I'm not quite sure
> > > how they have S0A3_GPIO configured, so maybe I have an invert wrong.
> > >
> > > The EC does control DD_ON which controls the 3.3V and 5V rails.
> >
> > On page 6 of the schematics I see the U7 that forms SUSB# from SUSB#_APU
> > (which corresponds to what you said) _and_ EC_EN, which is GPIO from IT5570,
> > which is EC.
> >
> > Are you using different schematics? I'm using the one from FDO bug report.
>
> Just checked this one:
> http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
>
> Also uses EC (SUSB_EC#).

So this all makes me thing that either EC firmware is buggy or we have ACPI EC
code in the kernel to fix.

--
With Best Regards,
Andy Shevchenko



2023-02-13 18:01:33

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 07:59:53PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 13, 2023 at 07:57:55PM +0200, Andy Shevchenko wrote:
> > On Mon, Feb 13, 2023 at 07:50:02PM +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 13, 2023 at 10:20:41AM -0700, Raul Rangel wrote:
> > > > On Mon, Feb 13, 2023 at 7:47 AM Werner Sembach <[email protected]> wrote:
> > > > > Am 13.02.23 um 15:37 schrieb Andy Shevchenko:

...

> > > > > Schematics for the NH5xAx can also be found on this unofficial clevo mirror
> > > > > (service manuals, scroll to end for schematics):
> > > > >
> > > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> > > > >
> > > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip
> > > > >
> > > > > User: repo
> > > > >
> > > > > PW: repo
> > > > >
> > > > > >> The schematics were shared by the reporter for the original issue which is
> > > > > >> how we reached the conclusion there was a mistake.
> > > > > >>
> > > > > >> As they're both Clevo designs it's certainly possible they have the same
> > > > > >> mistake in two systems.
> > > >
> > > > > > Thank you!
> > > > > > I have looked at the schematics and read discussion.
> > > > > >
> > > > > > So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> > > > > > The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> > > > > > signal for a while or on the PCB level, so that pull up should be connected
> > > > > > to another power source that stays on.
> > > > > >
> > > > > > This means the description on the initial patch with the same issue is
> > > > > > incorrect.
> > > > > >
> > > > > > Do we know the power sequence on the suspend to see which and how on the
> > > > > > time line the power sources are off/on?
> > > >
> > > > If you look at the load switch for 3.3VS, its EN2 pin is connected to
> > > > SUSB#_EN which is connected to SUSB# which is connected to
> > > > AND(SUSB#_PCH -> SLP_S3_L, PM_SLP_S0 -> S0A3_GPIO). So there is no
> > > > PMIC/EC firmware that is incharge of this. I guess I'm not quite sure
> > > > how they have S0A3_GPIO configured, so maybe I have an invert wrong.
> > > >
> > > > The EC does control DD_ON which controls the 3.3V and 5V rails.
> > >
> > > On page 6 of the schematics I see the U7 that forms SUSB# from SUSB#_APU
> > > (which corresponds to what you said) _and_ EC_EN, which is GPIO from IT5570,
> > > which is EC.
> > >
> > > Are you using different schematics? I'm using the one from FDO bug report.
> >
> > Just checked this one:
> > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> >
> > Also uses EC (SUSB_EC#).

Sorry, this has to be read as SUSBC_EC#.

> So this all makes me thing that either EC firmware is buggy or we have ACPI EC
> code in the kernel to fix.

--
With Best Regards,
Andy Shevchenko



2023-02-13 18:20:59

by Raul Rangel

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 11:01 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Mon, Feb 13, 2023 at 07:59:53PM +0200, Andy Shevchenko wrote:
> > On Mon, Feb 13, 2023 at 07:57:55PM +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 13, 2023 at 07:50:02PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Feb 13, 2023 at 10:20:41AM -0700, Raul Rangel wrote:
> > > > > On Mon, Feb 13, 2023 at 7:47 AM Werner Sembach <[email protected]> wrote:
> > > > > > Am 13.02.23 um 15:37 schrieb Andy Shevchenko:
>
> ...
>
> > > > > > Schematics for the NH5xAx can also be found on this unofficial clevo mirror
> > > > > > (service manuals, scroll to end for schematics):
> > > > > >
> > > > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> > > > > >
> > > > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip
> > > > > >
> > > > > > User: repo
> > > > > >
> > > > > > PW: repo
> > > > > >
> > > > > > >> The schematics were shared by the reporter for the original issue which is
> > > > > > >> how we reached the conclusion there was a mistake.
> > > > > > >>
> > > > > > >> As they're both Clevo designs it's certainly possible they have the same
> > > > > > >> mistake in two systems.
> > > > >
> > > > > > > Thank you!
> > > > > > > I have looked at the schematics and read discussion.
> > > > > > >
> > > > > > > So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> > > > > > > The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> > > > > > > signal for a while or on the PCB level, so that pull up should be connected
> > > > > > > to another power source that stays on.
> > > > > > >
> > > > > > > This means the description on the initial patch with the same issue is
> > > > > > > incorrect.
> > > > > > >
> > > > > > > Do we know the power sequence on the suspend to see which and how on the
> > > > > > > time line the power sources are off/on?
> > > > >
> > > > > If you look at the load switch for 3.3VS, its EN2 pin is connected to
> > > > > SUSB#_EN which is connected to SUSB# which is connected to
> > > > > AND(SUSB#_PCH -> SLP_S3_L, PM_SLP_S0 -> S0A3_GPIO). So there is no
> > > > > PMIC/EC firmware that is incharge of this. I guess I'm not quite sure
> > > > > how they have S0A3_GPIO configured, so maybe I have an invert wrong.
> > > > >
> > > > > The EC does control DD_ON which controls the 3.3V and 5V rails.
> > > >
> > > > On page 6 of the schematics I see the U7 that forms SUSB# from SUSB#_APU
> > > > (which corresponds to what you said) _and_ EC_EN, which is GPIO from IT5570,
> > > > which is EC.
> > > >
> > > > Are you using different schematics? I'm using the one from FDO bug report.
> > >
> > > Just checked this one:
> > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> > >
> > > Also uses EC (SUSB_EC#).
>
> Sorry, this has to be read as SUSBC_EC#.

It looks like SUSBC_EC# has to stay high during S3/S0i3 otherwise it's
going to shut down the S5 power domain. So I'm guessing U7 is there to
prevent the S3 domain from being powered on while the S5 domain is
powered off.

Sheet 59 of 73 VDDCR_SOC_S5, VDDCR_ALW seems to have a helpful table
that describes all the power states. I'm confused where SLP_SUS# comes
from though. I'm also not sure about S5_MUX_CTRL since that seems to
be connected to a testpoint.

>
> > So this all makes me thing that either EC firmware is buggy or we have ACPI EC
> > code in the kernel to fix.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

2023-02-13 19:01:55

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

On Mon, Feb 13, 2023 at 11:20:40AM -0700, Raul Rangel wrote:
> On Mon, Feb 13, 2023 at 11:01 AM Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Mon, Feb 13, 2023 at 07:59:53PM +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 13, 2023 at 07:57:55PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Feb 13, 2023 at 07:50:02PM +0200, Andy Shevchenko wrote:
> > > > > On Mon, Feb 13, 2023 at 10:20:41AM -0700, Raul Rangel wrote:
> > > > > > On Mon, Feb 13, 2023 at 7:47 AM Werner Sembach <[email protected]> wrote:
> > > > > > > Am 13.02.23 um 15:37 schrieb Andy Shevchenko:

...

> > > > > > > Schematics for the NH5xAx can also be found on this unofficial clevo mirror
> > > > > > > (service manuals, scroll to end for schematics):
> > > > > > >
> > > > > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> > > > > > >
> > > > > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AF1.zip
> > > > > > >
> > > > > > > User: repo
> > > > > > >
> > > > > > > PW: repo
> > > > > > >
> > > > > > > >> The schematics were shared by the reporter for the original issue which is
> > > > > > > >> how we reached the conclusion there was a mistake.
> > > > > > > >>
> > > > > > > >> As they're both Clevo designs it's certainly possible they have the same
> > > > > > > >> mistake in two systems.
> > > > > >
> > > > > > > > Thank you!
> > > > > > > > I have looked at the schematics and read discussion.
> > > > > > > >
> > > > > > > > So, the conclusion that this is a BIOS bug is incorrect in my opinion.
> > > > > > > > The problem is either in the PMIC/EC firmware that shouldn't shut down 3.3VS
> > > > > > > > signal for a while or on the PCB level, so that pull up should be connected
> > > > > > > > to another power source that stays on.
> > > > > > > >
> > > > > > > > This means the description on the initial patch with the same issue is
> > > > > > > > incorrect.
> > > > > > > >
> > > > > > > > Do we know the power sequence on the suspend to see which and how on the
> > > > > > > > time line the power sources are off/on?
> > > > > >
> > > > > > If you look at the load switch for 3.3VS, its EN2 pin is connected to
> > > > > > SUSB#_EN which is connected to SUSB# which is connected to
> > > > > > AND(SUSB#_PCH -> SLP_S3_L, PM_SLP_S0 -> S0A3_GPIO). So there is no
> > > > > > PMIC/EC firmware that is incharge of this. I guess I'm not quite sure
> > > > > > how they have S0A3_GPIO configured, so maybe I have an invert wrong.
> > > > > >
> > > > > > The EC does control DD_ON which controls the 3.3V and 5V rails.
> > > > >
> > > > > On page 6 of the schematics I see the U7 that forms SUSB# from SUSB#_APU
> > > > > (which corresponds to what you said) _and_ EC_EN, which is GPIO from IT5570,
> > > > > which is EC.
> > > > >
> > > > > Are you using different schematics? I'm using the one from FDO bug report.
> > > >
> > > > Just checked this one:
> > > > http://repo.palkeo.com/clevo-mirror/NH5xACx_AFx_ADx/NH50AC.zip
> > > >
> > > > Also uses EC (SUSB_EC#).
> >
> > Sorry, this has to be read as SUSBC_EC#.
>
> It looks like SUSBC_EC# has to stay high during S3/S0i3 otherwise it's
> going to shut down the S5 power domain. So I'm guessing U7 is there to
> prevent the S3 domain from being powered on while the S5 domain is
> powered off.
>
> Sheet 59 of 73 VDDCR_SOC_S5, VDDCR_ALW seems to have a helpful table
> that describes all the power states. I'm confused where SLP_SUS# comes
> from though. I'm also not sure about S5_MUX_CTRL since that seems to
> be connected to a testpoint.

I see, thanks for looking into this.

So, if EC has no business with this, whose responsibility to assert that
signal? I'm trying to get if it's PCB issue (wrong power rail for pull up)
or something else.

Btw, is anybody can actually boot Windows and check the signals of the pin on a
time line in comparison to Linux to see if the behaviour of them is the same?
(If Windows and Linux got the same timeline of the signals it would mean that
the issue in the Linux kernel.)

> > > So this all makes me thing that either EC firmware is buggy or we have ACPI EC
> > > code in the kernel to fix.

--
With Best Regards,
Andy Shevchenko



2023-02-13 21:58:01

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach kirjoitti:
> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
> changed the policy such that I2C touchpads may be able to wake up the
> system by default if the system is configured as such.
>
> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
> ActiveLow and level triggered but connected to a pull up. As soon as the
> system suspends the touchpad loses power and then the system wakes up.
>
> To avoid this problem, introduce a quirk for this model that will prevent
> the wakeup capability for being set for GPIO 10.

Please, update the commit message and send v2.

We would like to have this fix nevertheless, because users will suffer w/o it.

--
With Best Regards,
Andy Shevchenko



2023-02-14 12:31:46

by Werner Sembach

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx

Am 13.02.23 um 22:56 schrieb [email protected]:
> Fri, Feb 10, 2023 at 05:46:36PM +0100, Werner Sembach kirjoitti:
>> commit 1796f808e4bb ("HID: i2c-hid: acpi: Stop setting wakeup_capable")
>> changed the policy such that I2C touchpads may be able to wake up the
>> system by default if the system is configured as such.
>>
>> However on Clevo NH5xAx/TUXEDO XA15 Gen10 there is a mistake in the ACPI
>> tables that the TP_ATTN# signal connected to GPIO 10 is configured as
>> ActiveLow and level triggered but connected to a pull up. As soon as the
>> system suspends the touchpad loses power and then the system wakes up.
>>
>> To avoid this problem, introduce a quirk for this model that will prevent
>> the wakeup capability for being set for GPIO 10.
> Please, update the commit message and send v2.
>
> We would like to have this fix nevertheless, because users will suffer w/o it.
>
OK I'm on it.