| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <62b33623756d497d88bbdb9deccc0766@realtek.com>
Date: Thu, 22 Jan 2026 08:47:49 +0000
From: Ricky WU <ricky_wu@...ltek.com>
To: Ulf Hansson <ulf.hansson@...aro.org>,
Matthew Schwartz
<matthew.schwartz@...ux.dev>
CC: "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] mmc: rtsx_pci_sdmmc: implement sdmmc_card_busy function
> >
> > Hi Matthew,
> >
> > Thanks for working on this patch.
> >
> > We’ve tested this change on our platforms and can confirm that adding the
> > card_busy() callback does resolve the
> > “cannot verify signal voltage switch” issue for us 👍.
> >
> > That said, while reviewing the change we noticed a potential redundancy in
> > the existing driver logic. In sdmmc_switch_voltage() we already perform
> > explicit DAT line stabilization checks via
> > sd_wait_voltage_stable_1() and sd_wait_voltage_stable_2().
> >
> > Once card_busy() is implemented and used by the MMC core during the
> > voltage-switch verification phase, these two stabilization steps appear to
> > be partially overlapping with what the core now validates via
> > card_busy(). In our testing, with card_busy() present, the stable_1 /
> > stable_2 logic no longer seems strictly necessary and could likely be
> > simplified or removed with some adjustment.
> >
> > From a process point of view, we’re not sure which approach you’d prefer:
> >
> > Land your patch as-is first, and then we can follow up with a separate
> > cleanup/modification patch to adjust sdmmc_switch_voltage(), or
> >
> > We can prepare an additional patch that builds on top of yours and share
> > it with you for review, so the changes can be aligned together.
> >
> > Please let us know which option you think makes more sense for upstream,
> or
> > if you’d prefer a different approach.
> >
> > Thanks again for the fix and for looking into this driver.
> >
> > Best regards,
> > Ricky
> >
> > > rtsx_pci_sdmmc does not have an sdmmc_card_busy function, so any
> voltage
> > > switches cause a kernel warning, "mmc0: cannot verify signal voltage
> switch."
> > >
> > > Copy the sdmmc_card_busy function from rtsx_pci_usb to rtsx_pci_sdmmc
> to
> > > fix this.
> > >
> > > Fixes: ff984e57d36e ("mmc: Add realtek pcie sdmmc host driver")
> > > Signed-off-by: Matthew Schwartz <matthew.schwartz@...ux.dev>
>
> I have applied this for fixes and by adding a stable tag. I am also
> adding Ricky's reviewed/tested-by tag, according to the above.
>
> Let's deal with the potential improvement on-top, as agreed.
>
> Kind regards
> Uffe
>
>
Hi Matthew Ulf,
based on card_busy() patch, we’ve prepared a small follow-up change
that simplifies the voltage-switch handling in
sdmmc_switch_voltage().
sd_wait_voltage_stable_1() and sd_wait_voltage_stable_2() become largely
redundant.
In our testing, removing these two helpers and keeping only the minimal
clock-control handling (forcing the SD clock to stop before switching and
cleaning up on error) works correctly and still conforms to the expected
SD voltage switch behavior. The core-level checks via card_busy() now cover
the DAT line verification that was previously duplicated in the driver.
The attached diff removes sd_wait_voltage_stable_1() and
sd_wait_voltage_stable_2(), and simplifies sdmmc_switch_voltage()
accordingly.
Ricky
---
drivers/mmc/host/rtsx_pci_sdmmc.c | 88 +++----------------------------
1 file changed, 6 insertions(+), 82 deletions(-)
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 4db3328f46df..8dfbc62f165b 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -1181,79 +1181,6 @@ static int sdmmc_get_cd(struct mmc_host *mmc)
return cd;
}
-static int sd_wait_voltage_stable_1(struct realtek_pci_sdmmc *host)
-{
- struct rtsx_pcr *pcr = host->pcr;
- int err;
- u8 stat;
-
- /* Reference to Signal Voltage Switch Sequence in SD spec.
- * Wait for a period of time so that the card can drive SD_CMD and
- * SD_DAT[3:0] to low after sending back CMD11 response.
- */
- mdelay(1);
-
- /* SD_CMD, SD_DAT[3:0] should be driven to low by card;
- * If either one of SD_CMD,SD_DAT[3:0] is not low,
- * abort the voltage switch sequence;
- */
- err = rtsx_pci_read_register(pcr, SD_BUS_STAT, &stat);
- if (err < 0)
- return err;
-
- if (stat & (SD_CMD_STATUS | SD_DAT3_STATUS | SD_DAT2_STATUS |
- SD_DAT1_STATUS | SD_DAT0_STATUS))
- return -EINVAL;
-
- /* Stop toggle SD clock */
- err = rtsx_pci_write_register(pcr, SD_BUS_STAT,
- 0xFF, SD_CLK_FORCE_STOP);
- if (err < 0)
- return err;
-
- return 0;
-}
-
-static int sd_wait_voltage_stable_2(struct realtek_pci_sdmmc *host)
-{
- struct rtsx_pcr *pcr = host->pcr;
- int err;
- u8 stat, mask, val;
-
- /* Wait 1.8V output of voltage regulator in card stable */
- msleep(50);
-
- /* Toggle SD clock again */
- err = rtsx_pci_write_register(pcr, SD_BUS_STAT, 0xFF, SD_CLK_TOGGLE_EN);
- if (err < 0)
- return err;
-
- /* Wait for a period of time so that the card can drive
- * SD_DAT[3:0] to high at 1.8V
- */
- msleep(20);
-
- /* SD_CMD, SD_DAT[3:0] should be pulled high by host */
- err = rtsx_pci_read_register(pcr, SD_BUS_STAT, &stat);
- if (err < 0)
- return err;
-
- mask = SD_CMD_STATUS | SD_DAT3_STATUS | SD_DAT2_STATUS |
- SD_DAT1_STATUS | SD_DAT0_STATUS;
- val = SD_CMD_STATUS | SD_DAT3_STATUS | SD_DAT2_STATUS |
- SD_DAT1_STATUS | SD_DAT0_STATUS;
- if ((stat & mask) != val) {
- dev_dbg(sdmmc_dev(host),
- "%s: SD_BUS_STAT = 0x%x\n", __func__, stat);
- rtsx_pci_write_register(pcr, SD_BUS_STAT,
- SD_CLK_TOGGLE_EN | SD_CLK_FORCE_STOP, 0);
- rtsx_pci_write_register(pcr, CARD_CLK_EN, 0xFF, 0);
- return -EINVAL;
- }
-
- return 0;
-}
-
static int sdmmc_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct realtek_pci_sdmmc *host = mmc_priv(mmc);
@@ -1281,7 +1208,9 @@ static int sdmmc_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
voltage = OUTPUT_1V8;
if (voltage == OUTPUT_1V8) {
- err = sd_wait_voltage_stable_1(host);
+ /* Stop toggle SD clock */
+ err = rtsx_pci_write_register(pcr, SD_BUS_STAT,
+ 0xFF, SD_CLK_FORCE_STOP);
if (err < 0)
goto out;
}
@@ -1290,16 +1219,11 @@ static int sdmmc_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
if (err < 0)
goto out;
- if (voltage == OUTPUT_1V8) {
- err = sd_wait_voltage_stable_2(host);
- if (err < 0)
- goto out;
- }
-
out:
/* Stop toggle SD clock in idle */
- err = rtsx_pci_write_register(pcr, SD_BUS_STAT,
- SD_CLK_TOGGLE_EN | SD_CLK_FORCE_STOP, 0);
+ if (err < 0)
+ rtsx_pci_write_register(pcr, SD_BUS_STAT,
+ SD_CLK_TOGGLE_EN | SD_CLK_FORCE_STOP, 0);
mutex_unlock(&pcr->pcr_mutex);
--
2.34.1
> > > ---
> > > drivers/mmc/host/rtsx_pci_sdmmc.c | 41
> > > +++++++++++++++++++++++++++++++
> > > 1 file changed, 41 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c
> > > b/drivers/mmc/host/rtsx_pci_sdmmc.c
> > > index dc2587ff8519..4db3328f46df 100644
> > > --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> > > +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> > > @@ -1306,6 +1306,46 @@ static int sdmmc_switch_voltage(struct
> mmc_host
> > > *mmc, struct mmc_ios *ios)
> > > return err;
> > > }
> > >
> > > +static int sdmmc_card_busy(struct mmc_host *mmc) {
> > > + struct realtek_pci_sdmmc *host = mmc_priv(mmc);
> > > + struct rtsx_pcr *pcr = host->pcr;
> > > + int err;
> > > + u8 stat;
> > > + u8 mask = SD_DAT3_STATUS | SD_DAT2_STATUS |
> SD_DAT1_STATUS
> > > + | SD_DAT0_STATUS;
> > > +
> > > + mutex_lock(&pcr->pcr_mutex);
> > > +
> > > + rtsx_pci_start_run(pcr);
> > > +
> > > + err = rtsx_pci_write_register(pcr, SD_BUS_STAT,
> > > + SD_CLK_TOGGLE_EN |
> > > SD_CLK_FORCE_STOP,
> > > + SD_CLK_TOGGLE_EN);
> > > + if (err)
> > > + goto out;
> > > +
> > > + mdelay(1);
> > > +
> > > + err = rtsx_pci_read_register(pcr, SD_BUS_STAT, &stat);
> > > + if (err)
> > > + goto out;
> > > +
> > > + err = rtsx_pci_write_register(pcr, SD_BUS_STAT,
> > > + SD_CLK_TOGGLE_EN |
> > > +SD_CLK_FORCE_STOP, 0);
> > > +out:
> > > + mutex_unlock(&pcr->pcr_mutex);
> > > +
> > > + if (err)
> > > + return err;
> > > +
> > > + /* check if any pin between dat[0:3] is low */
> > > + if ((stat & mask) != mask)
> > > + return 1;
> > > + else
> > > + return 0;
> > > +}
> > > +
> > > static int sdmmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> {
> > > struct realtek_pci_sdmmc *host = mmc_priv(mmc); @@ -1418,6
> > > +1458,7 @@ static const struct mmc_host_ops realtek_pci_sdmmc_ops = {
> > > .get_ro = sdmmc_get_ro,
> > > .get_cd = sdmmc_get_cd,
> > > .start_signal_voltage_switch = sdmmc_switch_voltage,
> > > + .card_busy = sdmmc_card_busy,
> > > .execute_tuning = sdmmc_execute_tuning,
> > > .init_sd_express = sdmmc_init_sd_express, };
> > > --
> > > 2.52.0
> >
Powered by blists - more mailing lists