[PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve sd_set_power_mode()

[PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve sd_set_power_mode()

[Ulf Hansson]

The code in sd_set_power_mode() is a bit obfuscated and also has some minor
issue in its error-path. This small series addresses these problems.

Ulf Hansson (4):
  mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode()
  mmc: rtsx_usb_sdmmc: Print debug-messages at power-on/off errors
  mmc: rtsx_usb_sdmmc: Convert sd_set_power_mode() into void
  mmc: rtsx_usb_sdmmc: Re-work the code in sd_set_power_mode()

 drivers/mmc/host/rtsx_usb_sdmmc.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH 1/4] mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode()

[Ulf Hansson]

In the error path of sd_set_power_mode() we don't update host->power_mode,
which could lead to an imbalance of the runtime PM usage count. Fix this by
always updating host->power_mode.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index d229c2b83ea9..8c35cb85a9c0 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1029,9 +1029,7 @@ static int sd_set_power_mode(struct rtsx_usb_sdmmc *host,
 		err = sd_power_on(host);
 	}
 
-	if (!err)
-		host->power_mode = power_mode;
-
+	host->power_mode = power_mode;
 	return err;
 }
 
-- 
2.43.0

[PATCH 2/4] mmc: rtsx_usb_sdmmc: Print debug-messages at power-on/off errors

[Ulf Hansson]

It should be useful to know when we fail to power-on/off a card. Let's
therefore print debug-messages when this happens.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 8c35cb85a9c0..c204cdeaee3e 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1023,10 +1023,14 @@ static int sd_set_power_mode(struct rtsx_usb_sdmmc *host,
 
 	if (power_mode == MMC_POWER_OFF) {
 		err = sd_power_off(host);
+		if (err)
+			dev_dbg(sdmmc_dev(host), "power-off (err = %d)\n", err);
 		pm_runtime_put_noidle(sdmmc_dev(host));
 	} else {
 		pm_runtime_get_noresume(sdmmc_dev(host));
 		err = sd_power_on(host);
+		if (err)
+			dev_dbg(sdmmc_dev(host), "power-on (err = %d)\n", err);
 	}
 
 	host->power_mode = power_mode;
-- 
2.43.0

[PATCH 3/4] mmc: rtsx_usb_sdmmc: Convert sd_set_power_mode() into void

[Ulf Hansson]

The sdmmc_set_ios() is the only caller of sd_set_power_mode() and it
ignores the return code. Let's therefore convert sd_set_power_mode() into a
void function instead.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index c204cdeaee3e..af45bac019d2 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1010,7 +1010,7 @@ static int sd_power_off(struct rtsx_usb_sdmmc *host)
 	return sd_pull_ctl_disable_qfn24(ucr);
 }
 
-static int sd_set_power_mode(struct rtsx_usb_sdmmc *host,
+static void sd_set_power_mode(struct rtsx_usb_sdmmc *host,
 		unsigned char power_mode)
 {
 	int err;
@@ -1019,7 +1019,7 @@ static int sd_set_power_mode(struct rtsx_usb_sdmmc *host,
 		power_mode = MMC_POWER_ON;
 
 	if (power_mode == host->power_mode)
-		return 0;
+		return;
 
 	if (power_mode == MMC_POWER_OFF) {
 		err = sd_power_off(host);
@@ -1034,7 +1034,6 @@ static int sd_set_power_mode(struct rtsx_usb_sdmmc *host,
 	}
 
 	host->power_mode = power_mode;
-	return err;
 }
 
 static int sd_set_timing(struct rtsx_usb_sdmmc *host,
-- 
2.43.0

[PATCH 4/4] mmc: rtsx_usb_sdmmc: Re-work the code in sd_set_power_mode()

[Ulf Hansson]

It's only at MMC_POWER_OFF and at MMC_POWER_UP when some operations must be
carried out in sd_set_power_mode(). The code is a bit obfuscated in this
regards. Let's convert it into a switch-case-clause to make this clear.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/rtsx_usb_sdmmc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index af45bac019d2..c1fdcc334c8f 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -1015,22 +1015,30 @@ static void sd_set_power_mode(struct rtsx_usb_sdmmc *host,
 {
 	int err;
 
-	if (power_mode != MMC_POWER_OFF)
-		power_mode = MMC_POWER_ON;
-
 	if (power_mode == host->power_mode)
 		return;
 
-	if (power_mode == MMC_POWER_OFF) {
+	switch (power_mode) {
+	case MMC_POWER_OFF:
 		err = sd_power_off(host);
 		if (err)
 			dev_dbg(sdmmc_dev(host), "power-off (err = %d)\n", err);
 		pm_runtime_put_noidle(sdmmc_dev(host));
-	} else {
+		break;
+
+	case MMC_POWER_UP:
 		pm_runtime_get_noresume(sdmmc_dev(host));
 		err = sd_power_on(host);
 		if (err)
 			dev_dbg(sdmmc_dev(host), "power-on (err = %d)\n", err);
+		break;
+
+	case MMC_POWER_ON:
+	case MMC_POWER_UNDEFINED:
+		break;
+
+	default:
+		break;
 	}
 
 	host->power_mode = power_mode;
-- 
2.43.0

RE: [PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve sd_set_power_mode()

[Avri Altman]

> The code in sd_set_power_mode() is a bit obfuscated and also has some
> minor issue in its error-path. This small series addresses these problems.
> 
> Ulf Hansson (4):
>   mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode()
>   mmc: rtsx_usb_sdmmc: Print debug-messages at power-on/off errors
>   mmc: rtsx_usb_sdmmc: Convert sd_set_power_mode() into void
>   mmc: rtsx_usb_sdmmc: Re-work the code in sd_set_power_mode()
> 
>  drivers/mmc/host/rtsx_usb_sdmmc.c | 31 ++++++++++++++++++++---------
For the whole series:
Reviewed-by: Avri Altman <avri.altman@sandisk.com>

> --
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> --
> 2.43.0
> 

Re: [PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve sd_set_power_mode()

[Ulf Hansson]

On Tue, 10 Jun 2025 at 13:16, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> The code in sd_set_power_mode() is a bit obfuscated and also has some minor
> issue in its error-path. This small series addresses these problems.
>
> Ulf Hansson (4):
>   mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode()
>   mmc: rtsx_usb_sdmmc: Print debug-messages at power-on/off errors
>   mmc: rtsx_usb_sdmmc: Convert sd_set_power_mode() into void
>   mmc: rtsx_usb_sdmmc: Re-work the code in sd_set_power_mode()
>
>  drivers/mmc/host/rtsx_usb_sdmmc.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>
> --
> 2.43.0
>

Ricky, I would appreciate your feedback on these too. Or at least an ack.

Kind regards
Uffe

RE: [PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve sd_set_power_mode()

[Ricky WU]

> On Tue, 10 Jun 2025 at 13:16, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > The code in sd_set_power_mode() is a bit obfuscated and also has some
> > minor issue in its error-path. This small series addresses these problems.
> >
> > Ulf Hansson (4):
> >   mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode()
> >   mmc: rtsx_usb_sdmmc: Print debug-messages at power-on/off errors
> >   mmc: rtsx_usb_sdmmc: Convert sd_set_power_mode() into void
> >   mmc: rtsx_usb_sdmmc: Re-work the code in sd_set_power_mode()
> >
> >  drivers/mmc/host/rtsx_usb_sdmmc.c | 31
> > ++++++++++++++++++++-----------
> >  1 file changed, 20 insertions(+), 11 deletions(-)
> >
> > --
> > 2.43.0
> >
> 
> Ricky, I would appreciate your feedback on these too. Or at least an ack.

Hi Ulf,
Sorry, I forgot to reply this patch,
This patch is fine for me, and this also work well
Thank you


> 
> Kind regards
> Uffe

RE: [PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve sd_set_power_mode()

[Ricky WU]

> -----Original Message-----
> From: Ricky WU
> Sent: Wednesday, June 18, 2025 10:48 AM
> To: 'Ulf Hansson' <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve
> sd_set_power_mode()
> 
> > On Tue, 10 Jun 2025 at 13:16, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > The code in sd_set_power_mode() is a bit obfuscated and also has
> > > some minor issue in its error-path. This small series addresses these
> problems.
> > >
> > > Ulf Hansson (4):
> > >   mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode()
> > >   mmc: rtsx_usb_sdmmc: Print debug-messages at power-on/off errors
> > >   mmc: rtsx_usb_sdmmc: Convert sd_set_power_mode() into void
> > >   mmc: rtsx_usb_sdmmc: Re-work the code in sd_set_power_mode()
> > >
> > >  drivers/mmc/host/rtsx_usb_sdmmc.c | 31
> > > ++++++++++++++++++++-----------
> > >  1 file changed, 20 insertions(+), 11 deletions(-)
> > >
> > > --
> > > 2.43.0
> > >
> >
> > Ricky, I would appreciate your feedback on these too. Or at least an ack.
> 
> Hi Ulf,
> Sorry, I forgot to reply this patch,
> This patch is fine for me, and this also work well Thank you
> 
Acked-by: Ricky Wu <ricky_wu@realtek.com>
> 
> >
> > Kind regards
> > Uffe

Re: [PATCH 0/4] mmc: rtsx_usb_sdmmc: Improve sd_set_power_mode()

[Ulf Hansson]

On Tue, 10 Jun 2025 at 13:16, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> The code in sd_set_power_mode() is a bit obfuscated and also has some minor
> issue in its error-path. This small series addresses these problems.
>
> Ulf Hansson (4):
>   mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode()
>   mmc: rtsx_usb_sdmmc: Print debug-messages at power-on/off errors
>   mmc: rtsx_usb_sdmmc: Convert sd_set_power_mode() into void
>   mmc: rtsx_usb_sdmmc: Re-work the code in sd_set_power_mode()
>
>  drivers/mmc/host/rtsx_usb_sdmmc.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>

Applied for next!

Kind regards
Uffe