[PATCH] mmc: sdhci-pci-gli: Fail runtime resume on PLL timeout

[PATCH] mmc: sdhci-pci-gli: Fail runtime resume on PLL timeout

[Pengpeng Hou]

gl9763e_runtime_resume() logs when the internal clock never becomes
stable but still enables the card clock and reports runtime-resume
success.

Return the poll timeout before enabling the card clock. Clear the
PLL-enable bit so a failed resume leaves the clock state matching runtime
suspend. Low-power negotiation remains enabled for the suspended device.

The issue was found by our static-analysis tool and manually reviewed.

Fixes: d607667bb8fa ("mmc: sdhci-pci-gli: Add runtime PM for GL9763E")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
 drivers/mmc/host/sdhci-pci-gli.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
index b55618566d65..17717b54e08c 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -1888,6 +1888,7 @@ static int gl9763e_runtime_resume(struct sdhci_pci_chip *chip)
 	struct sdhci_pci_slot *slot = chip->slots[0];
 	struct sdhci_host *host = slot->host;
 	u16 clock;
+	int ret;
 
 	if (host->mmc->ios.power_mode != MMC_POWER_ON)
 		return 0;
@@ -1899,11 +1900,18 @@ static int gl9763e_runtime_resume(struct sdhci_pci_chip *chip)
 	sdhci_writew(host, clock, SDHCI_CLOCK_CONTROL);
 
 	/* Wait max 150 ms */
-	if (read_poll_timeout(sdhci_readw, clock, (clock & SDHCI_CLOCK_INT_STABLE),
-			      1000, 150000, false, host, SDHCI_CLOCK_CONTROL)) {
+	ret = read_poll_timeout(sdhci_readw, clock,
+				(clock & SDHCI_CLOCK_INT_STABLE),
+				1000, 150000, false, host,
+				SDHCI_CLOCK_CONTROL);
+	if (ret) {
 		pr_err("%s: PLL clock never stabilised.\n",
 		       mmc_hostname(host->mmc));
 		sdhci_dumpregs(host);
+
+		clock &= ~SDHCI_CLOCK_PLL_EN;
+		sdhci_writew(host, clock, SDHCI_CLOCK_CONTROL);
+		return ret;
 	}
 
 	clock |= SDHCI_CLOCK_CARD_EN;
-- 
2.50.1 (Apple Git-155)

RE: [PATCH] mmc: sdhci-pci-gli: Fail runtime resume on PLL timeout

[BenChuang[莊智量]]

> -----Original Message-----
> From: Pengpeng Hou <hppiscas@163.com>
> Sent: Sunday, September 6, 2026 11:40 AM
> To: Adrian Hunter <adrian.hunter@intel.com>; Ulf Hansson <ulfh@kernel.org>
> Cc: Pengpeng Hou <hppiscas@163.com>; linux-mmc@vger.kernel.org; BenChuang[莊智量]
> <Ben.Chuang@genesyslogic.com.tw>; linux-kernel@vger.kernel.org
> Subject: [PATCH] mmc: sdhci-pci-gli: Fail runtime resume on PLL timeout
>
> [You don't often get email from hppiscas@163.com. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> gl9763e_runtime_resume() logs when the internal clock never becomes
> stable but still enables the card clock and reports runtime-resume
> success.
>
> Return the poll timeout before enabling the card clock. Clear the
> PLL-enable bit so a failed resume leaves the clock state matching runtime
> suspend. Low-power negotiation remains enabled for the suspended device.
>
> The issue was found by our static-analysis tool and manually reviewed.
>

Hi Pengpeng,

Thanks for the patch.

Recently, no new Chromebooks use the GL9763E chip. This driver is old and we only want to keep it stable.

This modification comes from a static-analysis tool, not a real bug report from a user. On our side, we only
have the generic EVB board and we do not have the real Chromebook machines to test this change.
Changing the return value from 0 to a non-zero error like -ETIMEOUT might bring unknown risks to old machines,
so we think it is safer to keep the original code.

At this stage, we want to see what the maintainers think about this. Thanks!

Best regards,
Ben Chuang

> Fixes: d607667bb8fa ("mmc: sdhci-pci-gli: Add runtime PM for GL9763E")
> Assisted-by: gpt 5
> Signed-off-by: Pengpeng Hou <hppiscas@163.com>
> ---
>  drivers/mmc/host/sdhci-pci-gli.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
> index b55618566d65..17717b54e08c 100644
> --- a/drivers/mmc/host/sdhci-pci-gli.c
> +++ b/drivers/mmc/host/sdhci-pci-gli.c
> @@ -1888,6 +1888,7 @@ static int gl9763e_runtime_resume(struct sdhci_pci_chip *chip)
>         struct sdhci_pci_slot *slot = chip->slots[0];
>         struct sdhci_host *host = slot->host;
>         u16 clock;
> +       int ret;
>
>         if (host->mmc->ios.power_mode != MMC_POWER_ON)
>                 return 0;
> @@ -1899,11 +1900,18 @@ static int gl9763e_runtime_resume(struct sdhci_pci_chip *chip)
>         sdhci_writew(host, clock, SDHCI_CLOCK_CONTROL);
>
>         /* Wait max 150 ms */
> -       if (read_poll_timeout(sdhci_readw, clock, (clock & SDHCI_CLOCK_INT_STABLE),
> -                             1000, 150000, false, host, SDHCI_CLOCK_CONTROL)) {
> +       ret = read_poll_timeout(sdhci_readw, clock,
> +                               (clock & SDHCI_CLOCK_INT_STABLE),
> +                               1000, 150000, false, host,
> +                               SDHCI_CLOCK_CONTROL);
> +       if (ret) {
>                 pr_err("%s: PLL clock never stabilised.\n",
>                        mmc_hostname(host->mmc));
>                 sdhci_dumpregs(host);
> +
> +               clock &= ~SDHCI_CLOCK_PLL_EN;
> +               sdhci_writew(host, clock, SDHCI_CLOCK_CONTROL);
> +               return ret;
>         }
>
>         clock |= SDHCI_CLOCK_CARD_EN;
> --
> 2.50.1 (Apple Git-155)

________________________________

Genesys Logic Email Confidentiality Notice:
This mail and any attachments may contain information that is confidential, proprietary, privileged or otherwise protected by law. The mail is intended solely for the named addressee (or a person responsible for delivering it to the addressee). If you are not the intended recipient of this mail, you are not authorized to read, print, copy or disseminate this mail.

If you have received this email in error, please notify us immediately by reply email and immediately delete this message and any attachments from your system. Please be noted that any unauthorized use, dissemination, distribution or copying of this email is strictly prohibited.
________________________________

Re: [PATCH] mmc: sdhci-pci-gli: Fail runtime resume on PLL timeout

[Adrian Hunter]

On 07/09/2026 12:16, BenChuang[莊智量] wrote:
> 
>> -----Original Message-----
>> From: Pengpeng Hou <hppiscas@163.com>
>> Sent: Sunday, September 6, 2026 11:40 AM
>> To: Adrian Hunter <adrian.hunter@intel.com>; Ulf Hansson <ulfh@kernel.org>
>> Cc: Pengpeng Hou <hppiscas@163.com>; linux-mmc@vger.kernel.org; BenChuang[莊智量]
>> <Ben.Chuang@genesyslogic.com.tw>; linux-kernel@vger.kernel.org
>> Subject: [PATCH] mmc: sdhci-pci-gli: Fail runtime resume on PLL timeout
>>
>> [You don't often get email from hppiscas@163.com. Learn why this is important at
>> https://aka.ms/LearnAboutSenderIdentification ]
>>
>> gl9763e_runtime_resume() logs when the internal clock never becomes
>> stable but still enables the card clock and reports runtime-resume
>> success.
>>
>> Return the poll timeout before enabling the card clock. Clear the
>> PLL-enable bit so a failed resume leaves the clock state matching runtime
>> suspend. Low-power negotiation remains enabled for the suspended device.
>>
>> The issue was found by our static-analysis tool and manually reviewed.
>>
> 
> Hi Pengpeng,
> 
> Thanks for the patch.
> 
> Recently, no new Chromebooks use the GL9763E chip. This driver is old and we only want to keep it stable.
> 
> This modification comes from a static-analysis tool, not a real bug report from a user. On our side, we only
> have the generic EVB board and we do not have the real Chromebook machines to test this change.
> Changing the return value from 0 to a non-zero error like -ETIMEOUT might bring unknown risks to old machines,
> so we think it is safer to keep the original code.
> 
> At this stage, we want to see what the maintainers think about this. Thanks!

Currently, the mmc subsystem does not handle runtime resume
errors, so returning an error isn't helpful.  In this case,
returning 0 is a better option, so the existing code is better.

> 
> Best regards,
> Ben Chuang
> 
>> Fixes: d607667bb8fa ("mmc: sdhci-pci-gli: Add runtime PM for GL9763E")
>> Assisted-by: gpt 5
>> Signed-off-by: Pengpeng Hou <hppiscas@163.com>
>> ---
>>  drivers/mmc/host/sdhci-pci-gli.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
>> index b55618566d65..17717b54e08c 100644
>> --- a/drivers/mmc/host/sdhci-pci-gli.c
>> +++ b/drivers/mmc/host/sdhci-pci-gli.c
>> @@ -1888,6 +1888,7 @@ static int gl9763e_runtime_resume(struct sdhci_pci_chip *chip)
>>         struct sdhci_pci_slot *slot = chip->slots[0];
>>         struct sdhci_host *host = slot->host;
>>         u16 clock;
>> +       int ret;
>>
>>         if (host->mmc->ios.power_mode != MMC_POWER_ON)
>>                 return 0;
>> @@ -1899,11 +1900,18 @@ static int gl9763e_runtime_resume(struct sdhci_pci_chip *chip)
>>         sdhci_writew(host, clock, SDHCI_CLOCK_CONTROL);
>>
>>         /* Wait max 150 ms */
>> -       if (read_poll_timeout(sdhci_readw, clock, (clock & SDHCI_CLOCK_INT_STABLE),
>> -                             1000, 150000, false, host, SDHCI_CLOCK_CONTROL)) {
>> +       ret = read_poll_timeout(sdhci_readw, clock,
>> +                               (clock & SDHCI_CLOCK_INT_STABLE),
>> +                               1000, 150000, false, host,
>> +                               SDHCI_CLOCK_CONTROL);
>> +       if (ret) {
>>                 pr_err("%s: PLL clock never stabilised.\n",
>>                        mmc_hostname(host->mmc));
>>                 sdhci_dumpregs(host);
>> +
>> +               clock &= ~SDHCI_CLOCK_PLL_EN;
>> +               sdhci_writew(host, clock, SDHCI_CLOCK_CONTROL);
>> +               return ret;
>>         }
>>
>>         clock |= SDHCI_CLOCK_CARD_EN;
>> --
>> 2.50.1 (Apple Git-155)