MIME-Version: 1.0
References: <Y+Z5OSa6hepQBOyc@smile.fi.intel.com> <029b8d80-db28-cdb2-5c39-334be6968fad@tuxedocomputers.com>
 <Y+owDqifuU9nf+1i@smile.fi.intel.com> <86db79fa-5efb-caad-3310-60928907cc58@amd.com>
 <Y+pLLzLDotZQLpdA@smile.fi.intel.com> <97026dc5-e92e-62fe-43ae-33533125d900@tuxedocomputers.com>
 <CAHQZ30Cs+kp82coR10Wat7q3S_8+pFf=5=44kMEMcjBOjmn=6A@mail.gmail.com>
 <Y+p4Sq/WnZ4jAp+F@smile.fi.intel.com> <Y+p6I379g+V4vpIc@smile.fi.intel.com>
 <Y+p6mY+w9POvkBzC@smile.fi.intel.com> <Y+p68FfTYpUP7B1F@smile.fi.intel.com>
In-Reply-To: <Y+p68FfTYpUP7B1F@smile.fi.intel.com>
From:   Raul Rangel <rrangel@chromium.org>
Date:   Mon, 13 Feb 2023 11:20:40 -0700
Message-ID: <CAHQZ30DaNjAREK3TXKtKC-G31NXi1tFTLmRRf8c-Ck1tg4e-YA@mail.gmail.com>
Subject: Re: [PATCH] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NH5xAx
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Werner Sembach <wse@tuxedocomputers.com>,
        Mario Limonciello <mario.limonciello@amd.com>,
        mika.westerberg@linux.intel.com, linus.walleij@linaro.org,
        brgl@bgdev.pl, linux-gpio@vger.kernel.org,
        linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
        Alex Deucher <alexander.deucher@amd.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Feb 13, 2023 at 11:01 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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 <wse@tuxedocomputers.com> 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
>
>