[PATCH] mmc: rtsx: add 74 Clocks in power on flow

[PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Christian Löhle]

commit 1f311c94aabd ("mmc: rtsx: add 74 Clocks in power on flow") upstream.
backport note: removed unavailable power_delay_ms 5ms as 10ms is fine, too.

SD spec definition:
"Host provides at least 74 Clocks before issuing first command"
After 1ms for the voltage stable then start issuing the Clock signals

if POWER STATE is
MMC_POWER_OFF to MMC_POWER_UP to issue Clock signal to card
MMC_POWER_UP to MMC_POWER_ON to stop issuing signal to card

Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
 drivers/mmc/host/rtsx_pci_sdmmc.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 9de6a32f0c9f..0b1d4f2b5862 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -49,10 +49,7 @@ struct realtek_pci_sdmmc {
 	bool			double_clk;
 	bool			eject;
 	bool			initial_mode;
-	int			power_state;
-#define SDMMC_POWER_ON		1
-#define SDMMC_POWER_OFF		0
-
+	int			prev_power_state;
 	int			sg_count;
 	s32			cookie;
 	int			cookie_sg_count;
@@ -913,14 +910,21 @@ static int sd_set_bus_width(struct realtek_pci_sdmmc *host,
 	return err;
 }
 
-static int sd_power_on(struct realtek_pci_sdmmc *host)
+static int sd_power_on(struct realtek_pci_sdmmc *host, unsigned char power_mode)
 {
 	struct rtsx_pcr *pcr = host->pcr;
 	int err;
 
-	if (host->power_state == SDMMC_POWER_ON)
+	if (host->prev_power_state == MMC_POWER_ON)
 		return 0;
 
+	if (host->prev_power_state == MMC_POWER_UP) {
+		rtsx_pci_write_register(pcr, SD_BUS_STAT, SD_CLK_TOGGLE_EN, 0);
+		goto finish;
+	}
+
+	msleep(100);
+
 	rtsx_pci_init_cmd(pcr);
 	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SELECT, 0x07, SD_MOD_SEL);
 	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SHARE_MODE,
@@ -939,11 +943,17 @@ static int sd_power_on(struct realtek_pci_sdmmc *host)
 	if (err < 0)
 		return err;
 
+	mdelay(1);
+
 	err = rtsx_pci_write_register(pcr, CARD_OE, SD_OUTPUT_EN, SD_OUTPUT_EN);
 	if (err < 0)
 		return err;
 
-	host->power_state = SDMMC_POWER_ON;
+	/* send at least 74 clocks */
+	rtsx_pci_write_register(pcr, SD_BUS_STAT, SD_CLK_TOGGLE_EN, SD_CLK_TOGGLE_EN);
+
+finish:
+	host->prev_power_state = power_mode;
 	return 0;
 }
 
@@ -952,7 +962,7 @@ static int sd_power_off(struct realtek_pci_sdmmc *host)
 	struct rtsx_pcr *pcr = host->pcr;
 	int err;
 
-	host->power_state = SDMMC_POWER_OFF;
+	host->prev_power_state = MMC_POWER_OFF;
 
 	rtsx_pci_init_cmd(pcr);
 
@@ -978,7 +988,7 @@ static int sd_set_power_mode(struct realtek_pci_sdmmc *host,
 	if (power_mode == MMC_POWER_OFF)
 		err = sd_power_off(host);
 	else
-		err = sd_power_on(host);
+		err = sd_power_on(host, power_mode);
 
 	return err;
 }
@@ -1416,7 +1426,7 @@ static int rtsx_pci_sdmmc_drv_probe(struct platform_device *pdev)
 	host->mmc = mmc;
 	host->pdev = pdev;
 	host->cookie = -1;
-	host->power_state = SDMMC_POWER_OFF;
+	host->prev_power_state = MMC_POWER_OFF;
 	INIT_WORK(&host->work, sd_request);
 	platform_set_drvdata(pdev, host);
 	pcr->slots[RTSX_SD_CARD].p_dev = pdev;
-- 
2.34.1=
Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Christian Löhle]

>commit 1f311c94aabd ("mmc: rtsx: add 74 Clocks in power on flow") upstream.
>backport note: removed unavailable power_delay_ms 5ms as 10ms is fine, too.
>
>SD spec definition:
>"Host provides at least 74 Clocks before issuing first command"
>After 1ms for the voltage stable then start issuing the Clock signals
>
>if POWER STATE is
>MMC_POWER_OFF to MMC_POWER_UP to issue Clock signal to card
>MMC_POWER_UP to MMC_POWER_ON to stop issuing signal to card
>
>Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
>---

Now for 4.9.y and 4.14.y, my bad and thanks a lot.

Regards,
Christian=
Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Greg Kroah-Hartman]

On Tue, May 10, 2022 at 01:02:24PM +0000, Christian Löhle wrote:
> >commit 1f311c94aabd ("mmc: rtsx: add 74 Clocks in power on flow") upstream.
> >backport note: removed unavailable power_delay_ms 5ms as 10ms is fine, too.
> >
> >SD spec definition:
> >"Host provides at least 74 Clocks before issuing first command"
> >After 1ms for the voltage stable then start issuing the Clock signals
> >
> >if POWER STATE is
> >MMC_POWER_OFF to MMC_POWER_UP to issue Clock signal to card
> >MMC_POWER_UP to MMC_POWER_ON to stop issuing signal to card
> >
> >Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> >Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> >---
> 
> Now for 4.9.y and 4.14.y, my bad and thanks a lot.

Now queued up, thanks.

greg k-h

[PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Christian Löhle]

commit 1f311c94aabd ("mmc: rtsx: add 74 Clocks in power on flow") upstream.

SD spec definition:
"Host provides at least 74 Clocks before issuing first command"
After 1ms for the voltage stable then start issuing the Clock signals

if POWER STATE is
MMC_POWER_OFF to MMC_POWER_UP to issue Clock signal to card
MMC_POWER_UP to MMC_POWER_ON to stop issuing signal to card

Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
 drivers/mmc/host/rtsx_pci_sdmmc.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index e00167bcfaf6..b5cb83bb9b85 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -37,10 +37,7 @@ struct realtek_pci_sdmmc {
 	bool			double_clk;
 	bool			eject;
 	bool			initial_mode;
-	int			power_state;
-#define SDMMC_POWER_ON		1
-#define SDMMC_POWER_OFF		0
-
+	int			prev_power_state;
 	int			sg_count;
 	s32			cookie;
 	int			cookie_sg_count;
@@ -902,14 +899,21 @@ static int sd_set_bus_width(struct realtek_pci_sdmmc *host,
 	return err;
 }
 
-static int sd_power_on(struct realtek_pci_sdmmc *host)
+static int sd_power_on(struct realtek_pci_sdmmc *host, unsigned char power_mode)
 {
 	struct rtsx_pcr *pcr = host->pcr;
 	int err;
 
-	if (host->power_state == SDMMC_POWER_ON)
+	if (host->prev_power_state == MMC_POWER_ON)
 		return 0;
 
+	if (host->prev_power_state == MMC_POWER_UP) {
+		rtsx_pci_write_register(pcr, SD_BUS_STAT, SD_CLK_TOGGLE_EN, 0);
+		goto finish;
+	}
+
+	msleep(100);
+
 	rtsx_pci_init_cmd(pcr);
 	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SELECT, 0x07, SD_MOD_SEL);
 	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, CARD_SHARE_MODE,
@@ -928,11 +932,17 @@ static int sd_power_on(struct realtek_pci_sdmmc *host)
 	if (err < 0)
 		return err;
 
+	mdelay(1);
+
 	err = rtsx_pci_write_register(pcr, CARD_OE, SD_OUTPUT_EN, SD_OUTPUT_EN);
 	if (err < 0)
 		return err;
 
-	host->power_state = SDMMC_POWER_ON;
+	/* send at least 74 clocks */
+	rtsx_pci_write_register(pcr, SD_BUS_STAT, SD_CLK_TOGGLE_EN, SD_CLK_TOGGLE_EN);
+
+finish:
+	host->prev_power_state = power_mode;
 	return 0;
 }
 
@@ -941,7 +951,7 @@ static int sd_power_off(struct realtek_pci_sdmmc *host)
 	struct rtsx_pcr *pcr = host->pcr;
 	int err;
 
-	host->power_state = SDMMC_POWER_OFF;
+	host->prev_power_state = MMC_POWER_OFF;
 
 	rtsx_pci_init_cmd(pcr);
 
@@ -967,7 +977,7 @@ static int sd_set_power_mode(struct realtek_pci_sdmmc *host,
 	if (power_mode == MMC_POWER_OFF)
 		err = sd_power_off(host);
 	else
-		err = sd_power_on(host);
+		err = sd_power_on(host, power_mode);
 
 	return err;
 }
@@ -1404,10 +1414,11 @@ static int rtsx_pci_sdmmc_drv_probe(struct platform_device *pdev)
 
 	host = mmc_priv(mmc);
 	host->pcr = pcr;
+	mmc->ios.power_delay_ms = 5;
 	host->mmc = mmc;
 	host->pdev = pdev;
 	host->cookie = -1;
-	host->power_state = SDMMC_POWER_OFF;
+	host->prev_power_state = MMC_POWER_OFF;
 	INIT_WORK(&host->work, sd_request);
 	platform_set_drvdata(pdev, host);
 	pcr->slots[RTSX_SD_CARD].p_dev = pdev;
-- 
2.34.1
Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Christian Löhle]

>commit 1f311c94aabd ("mmc: rtsx: add 74 Clocks in power on flow") upstream.
>
>SD spec definition:
>"Host provides at least 74 Clocks before issuing first command"
>After 1ms for the voltage stable then start issuing the Clock signals
>
>if POWER STATE is
>MMC_POWER_OFF to MMC_POWER_UP to issue Clock signal to card
>MMC_POWER_UP to MMC_POWER_ON to stop issuing signal to card
>
>Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>Signed-off-by: Christian Loehle <cloehle@hyperstone.com>

Based on 5.10.y, applies to all down to 4.9.y=
Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Greg Kroah-Hartman]

On Tue, May 10, 2022 at 12:09:45PM +0000, Christian Löhle wrote:
> >commit 1f311c94aabd ("mmc: rtsx: add 74 Clocks in power on flow") upstream.
> >
> >SD spec definition:
> >"Host provides at least 74 Clocks before issuing first command"
> >After 1ms for the voltage stable then start issuing the Clock signals
> >
> >if POWER STATE is
> >MMC_POWER_OFF to MMC_POWER_UP to issue Clock signal to card
> >MMC_POWER_UP to MMC_POWER_ON to stop issuing signal to card
> >
> >Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> >Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> 
> Based on 5.10.y, applies to all down to 4.9.y=

It breaks the build on 4.14.y and 4.9.y :(

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Christian Löhle]

Can we get 1f311c94aabdb419c28e3147bcc8ab89269f1a7e merged into the stable tree?
I have some compatibility issues on Realtek chips because of the missing initialization clocks.

Thanks!
Regards,
Christian



From: Ulf Hansson <ulf.hansson@linaro.org>
Sent: Monday, February 28, 2022 5:12 PM
To: Ricky WU
Cc: gregkh@linuxfoundation.org; kai.heng.feng@canonical.com; tommyhebb@gmail.com; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow
    
On Tue, 22 Feb 2022 at 08:28, Ricky WU <ricky_wu@realtek.com> wrote:
>
> After 1ms stabilizing the voltage time
> add "Host provides at least 74 Clocks
> before issuing first command" that is
> spec definition
>
> Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> ---
>  drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 2a3f14afe9f8..e016d720e453 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -940,10 +940,17 @@ static int sd_power_on(struct realtek_pci_sdmmc *host)
>         if (err < 0)
>                 return err;
>
> +       mdelay(1);
> +
>         err = rtsx_pci_write_register(pcr, CARD_OE, SD_OUTPUT_EN, SD_OUTPUT_EN);
>         if (err < 0)
>                 return err;
>
> +       /* send init 74 clocks */
> +       rtsx_pci_write_register(pcr, SD_BUS_STAT, SD_CLK_TOGGLE_EN, SD_CLK_TOGGLE_EN);
> +       mdelay(5);
> +       rtsx_pci_write_register(pcr, SD_BUS_STAT, SD_CLK_TOGGLE_EN, 0);
> +
>         if (PCI_PID(pcr) == PID_5261) {
>                 /*
>                  * If test mode is set switch to SD Express mandatorily,

As you probably are aware of, the mmc core uses three power states
(MMC_POWER_ON, MMC_POWER_UP and MMC_POWER_OFF) to manage the
initialization, while it invokes the ->set_ios() callback for the mmc
host driver. During these steps the core also tries to manage the
different delays that are needed according to the eMMC/SD specs. You
may have a look at mmc_power_up() in drivers/mmc/core/core.c. In the
rtsx case, MMC_POWER_ON and MMC_POWER_UP are treated as one single
step.

Moreover, it has turned out that some mmc HWs are actually controlling
these delays during the initialization themselves, which makes the
delays in the core superfluous. Therefore we have made the delays
configurable for host drivers. For DT based platforms, we have the DT
property "post-power-on-delay-ms" and for others, it's perfectly fine
to update host->power_delay_ms before calling mmc_add_host().

Would it be possible to take advantage of the above "features" from
the core, to avoid hard coded and superfluous delays?

Kind regards
Uffe
    =
Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[gregkh]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Mon, May 09, 2022 at 09:35:25AM +0000, Christian Löhle wrote:
> Can we get 1f311c94aabdb419c28e3147bcc8ab89269f1a7e merged into the stable tree?

Which stable branches do you want this added to?

thanks,

greg k-h

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Christian Löhle]

>> Can we get 1f311c94aabdb419c28e3147bcc8ab89269f1a7e merged into the stable tree?
>
>Which stable branches do you want this added to?
>
>thanks,
>
>greg k-h

From what I can tell the issue is present since the addition of the driver in ff984e57d36e8
So I would suggest adding them to all? Maybe Ricky and Ulf could comment?

Regards,
Christian=
Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[Ulf Hansson]

On Mon, 9 May 2022 at 12:12, Christian Löhle <CLoehle@hyperstone.com> wrote:
>
> >> Can we get 1f311c94aabdb419c28e3147bcc8ab89269f1a7e merged into the stable tree?
> >
> >Which stable branches do you want this added to?
> >
> >thanks,
> >
> >greg k-h
>
> From what I can tell the issue is present since the addition of the driver in ff984e57d36e8
> So I would suggest adding them to all? Maybe Ricky and Ulf could comment?

That seems correct.

Although, it's likely that 1f311c94aabd doesn't apply to older stable
trees, but I guess we can try and see how it goes.

Perhaps an even better option is that you submit a backported patch to
Greg for those stable kernels you want it to be applied to.

Kind regards
Uffe

Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow

[gregkh]

On Tue, May 10, 2022 at 11:56:38AM +0200, Ulf Hansson wrote:
> On Mon, 9 May 2022 at 12:12, Christian Löhle <CLoehle@hyperstone.com> wrote:
> >
> > >> Can we get 1f311c94aabdb419c28e3147bcc8ab89269f1a7e merged into the stable tree?
> > >
> > >Which stable branches do you want this added to?
> > >
> > >thanks,
> > >
> > >greg k-h
> >
> > From what I can tell the issue is present since the addition of the driver in ff984e57d36e8
> > So I would suggest adding them to all? Maybe Ricky and Ulf could comment?
> 
> That seems correct.
> 
> Although, it's likely that 1f311c94aabd doesn't apply to older stable
> trees, but I guess we can try and see how it goes.
> 
> Perhaps an even better option is that you submit a backported patch to
> Greg for those stable kernels you want it to be applied to.

It only applies cleanly to 5.17.y and 5.15.y, so I've added it there.
If anyone wants it to be added to older kernels, they need to provide
working backports.

thanks,

greg k-h