drivers/mmc/host/rtsx_pci_sdmmc.c | 41 +++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
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@linux.dev>
---
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
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@linux.dev>
> ---
> 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
On Thu, 15 Jan 2026 at 11:23, Ricky WU <ricky_wu@realtek.com> wrote:
>
> 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@linux.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
> > ---
> > 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
>
> > >
> > > 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@linux.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 All,
Just a gentle ping on the patch below.
This change is intended as a follow-up cleanup on top of Matthew’s
card_busy() patch, which has already been merged. I wanted to check
whether this additional adjustment to the voltage-switch path looks OK,
or if there are any comments or concerns from the maintainers.
We’re happy to rework or rebase the patch as needed.
> 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
> > >
On Wed, 28 Jan 2026 at 04:06, Ricky WU <ricky_wu@realtek.com> wrote:
>
> > > >
> > > > 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@linux.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 All,
>
> Just a gentle ping on the patch below.
> This change is intended as a follow-up cleanup on top of Matthew’s
> card_busy() patch, which has already been merged. I wanted to check
> whether this additional adjustment to the voltage-switch path looks OK,
> or if there are any comments or concerns from the maintainers.
>
> We’re happy to rework or rebase the patch as needed.
I suggest that you re-submit the patch (formatted correctly with a
commit message) separately and ask Matthew if he has the time to
review/test it, then will happily apply it.
[...]
Kind regards
Uffe
> >
> > 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@linux.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
> >
On 1/15/26 2:23 AM, Ricky WU wrote:
> 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.
Hi Ricky,
Let's go with this method.
Thanks!
Matt
>
> 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@linux.dev>
>> ---
>> 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
>
© 2016 - 2026 Red Hat, Inc.