mmc: rtsx: add 74 Clocks in power on flow
Subsystems (2)
PATCH 1/1 mmc: rtsx: add 74 Clocks in power on flow
After 1ms stabilizing the voltage time
add "Host provides at least 74 Clocks
before issuing first command" that is
spec definition
--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/mmc/host/rtsx_pci_sdmmc.c +7 -0
@@ -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,
Signature · 1 line
2.25.1
[email protected]
maintainerQuoted 4 lines · Ricky WU
After 1ms stabilizing the voltage time
add "Host provides at least 74 Clocks
before issuing first command" that is
spec definition
You do have 72 columns to use here, no need to wrap this so tightly.
Quoted 11 lines
Signed-off-by Ricky Wu <[email protected]>--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 943
Quoted 4 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +1
-0@@ -940,10 +940,17 @@ static int sd_power_on(struct realtek_pci_sdmmc *host)if (err <0 )return err; +mdelay (1 );
What is this delay for?
thanks,
greg k-h
Quoted 9 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 3:42 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow
Quoted 6 lines
Quoted 2 lines · Ricky WU
After 1ms stabilizing the voltage time add "Host provides at least 74 Clocks before issuing first command" that is spec definition
You do have 72 columns to use here, no need to wrap this so tightly.
Ok...
so I need to have next patch to fix this format?
Quoted 13 lines
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc
Quoted 12 lines
Signed-off-by Ricky Wu <[email protected]>--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0@@ -940,10 +940,17 @@ static int sd_power_on(struct realtek_pci_sdmmc*host)
Quoted 7 lines
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 943
Quoted 4 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +1
-0if (err <0 )return err; +mdelay (1 );What is this delay for?
Spec definition, need to wait 1ms for voltage stable and below mdelay(5) is our device send 74 Clocks time
Quoted 4 lines
thanks,
greg k-h
------Please consider the environment before printing this e-mail.
[email protected]
maintainerQuoted 10 lines · Ricky WU
Quoted 10 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 3:42 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowOn Tue, Feb 22, 2022 at 07:27:52AM +0000, Ricky WU wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
After 1ms stabilizing the voltage time add "Host provides at least 74 Clocks before issuing first command" that is spec definition
You do have 72 columns to use here, no need to wrap this so tightly.
Ok...
so I need to have next patch to fix this format?
Please do, because:
Quoted 17 lines
Quoted 17 lines
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc
Quoted 13 lines
Signed-off-by Ricky Wu <[email protected]>--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0@@ -940,10 +940,17 @@ static int sd_power_on(struct realtek_pci_sdmmc*host)
Quoted 3 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0if (err <0 )return err;
Quoted 7 lines
Quoted 4 lines
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 943
Quoted 1 line
drivers/mmc/host/rtsx_pci_sdmmc.c +1
-0+mdelay (1 );What is this delay for?
Spec definition, need to wait 1ms for voltage stable and below mdelay(5) is our device send 74 Clocks time
Clock cycles and mdelay() are not going to always stay the same, right?
I really have no idea what "74 clocks time" means, is this some specific timing value for this hardware? What is the units? This needs to be documented better in both the changelog and in the code.
thanks,
greg k-h
Quoted 57 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 4:59 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowQuoted 10 lines · Ricky WU
Quoted 10 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 3:42 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowOn Tue, Feb 22, 2022 at 07:27:52AM +0000, Ricky WU wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
After 1ms stabilizing the voltage time add "Host provides at least 74 Clocks before issuing first command" that is spec definition
You do have 72 columns to use here, no need to wrap this so tightly.
Ok...
so I need to have next patch to fix this format?Please do, because:
Quoted 18 lines
Quoted 18 lines
Quoted 13 lines
Signed-off-by Ricky Wu <[email protected]>--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0@@ -940,10 +940,17 @@ static int sd_power_on(structQuoted 1 line
realtek_pci_sdmmc*host)
Quoted 3 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0if (err <0 )return err;Quoted 7 lines
Quoted 4 lines
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 943
Quoted 1 line
drivers/mmc/host/rtsx_pci_sdmmc.c +1
-0+mdelay (1 );What is this delay for?
Spec definition, need to wait 1ms for voltage stable and below mdelay(5) is our device send 74 Clocks time
Quoted 8 lines
Clock cycles and
mdelay()are not going to always stay the same, right?I really have no idea what "74 clocks time" means, is this some specific timing value for this hardware? What is the units? This needs to be documented better in both the changelog and in the code.
thanks,
Please ref: https://www.sdcard.org/downloads/pls/ Version8
And see the 6.4.1.2 Power Up Time of Host Figure 6-5: Power Up Diagram of Host mdelay(1) corresponds to Stable supply voltage
mdelay(5) corresponds to Host provides at least 74 Clocks before issuing first command our device need 5ms to issue 74 Clocks
Quoted 2 lines
greg k-h
------Please consider the environment before printing this e-mail.
[email protected]
maintainerQuoted 59 lines · Ricky WU
Quoted 59 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 4:59 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowQuoted 10 lines · Ricky WU
Quoted 10 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 3:42 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowOn Tue, Feb 22, 2022 at 07:27:52AM +0000, Ricky WU wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
After 1ms stabilizing the voltage time add "Host provides at least 74 Clocks before issuing first command" that is spec definition
You do have 72 columns to use here, no need to wrap this so tightly.
Ok...
so I need to have next patch to fix this format?Please do, because:
Quoted 18 lines
Quoted 18 lines
Quoted 13 lines
Signed-off-by Ricky Wu <[email protected]>--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0@@ -940,10 +940,17 @@ static int sd_power_on(structQuoted 1 line
realtek_pci_sdmmc*host)
Quoted 3 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0if (err <0 )return err;Quoted 7 lines
Quoted 4 lines
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 943
Quoted 1 line
drivers/mmc/host/rtsx_pci_sdmmc.c +1
-0+mdelay (1 );What is this delay for?
Spec definition, need to wait 1ms for voltage stable and below mdelay(5) is our device send 74 Clocks time
Clock cycles and
mdelay()are not going to always stay the same, right?
Quoted 8 lines
Quoted 6 lines
I really have no idea what "74 clocks time" means, is this some specific timing value for this hardware? What is the units? This needs to be documented better in both the changelog and in the code.
thanks,
Please ref: https://www.sdcard.org/downloads/pls/ Version8
I can not download those specifications according to the license that they ask me to abide by.
Quoted 4 lines
And see the 6.4.1.2 Power Up Time of Host Figure 6-5: Power Up Diagram of Host mdelay(1) corresponds to Stable supply voltage
mdelay(5) corresponds to Host provides at least 74 Clocks before issuing first command our device need 5ms to issue 74 Clocks
What is a "clock"? The kernel works with time units, how does "5" equal to 74?
thanks,
greg k-h
Quoted 83 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 10:18 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowQuoted 61 lines · Ricky WU
Quoted 61 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 4:59 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowQuoted 15 lines · Ricky WU
Quoted 15 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Tuesday, February 22, 2022 3:42 PM
To: Ricky WU <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flowQuoted 4 lines · Ricky WU
After 1ms stabilizing the voltage time add "Host provides at least
74 Clocks before issuing first command" that is spec definitionQuoted 5 lines
Quoted 2 lines
You do have 72 columns to use here, no need to wrap this so tightly.
Ok...
so I need to have next patch to fix this format?Please do, because:
Quoted 18 lines
Quoted 18 lines
Quoted 13 lines
Signed-off-by Ricky Wu <[email protected]>--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0@@ -940,10 +940,17 @@ static int sd_power_on(structQuoted 1 line
realtek_pci_sdmmc*host)
Quoted 3 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +0
-0if (err <0 )return err;Quoted 7 lines
Quoted 4 lines
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 943
Quoted 1 line
drivers/mmc/host/rtsx_pci_sdmmc.c +1
-0+mdelay (1 );What is this delay for?
Spec definition, need to wait 1ms for voltage stable and below mdelay(5) is our device send 74 Clocks time
Clock cycles and
mdelay()are not going to always stay the same, right?Quoted 8 lines
Quoted 6 lines
I really have no idea what "74 clocks time" means, is this some specific timing value for this hardware? What is the units? This needs to be documented better in both the changelog and in the code.
thanks,
Please ref: https://www.sdcard.org/downloads/pls/ Version8
I can not download those specifications according to the license that they ask me to abide by.
Quoted 9 lines
Quoted 5 lines
And see the 6.4.1.2 Power Up Time of Host Figure 6-5: Power Up Diagram of Host
mdelay(1) corresponds to Stable supply voltage
mdelay(5) corresponds to Host provides at least 74 Clocks before issuing first command our device need 5ms to issue 74 ClocksWhat is a "clock"? The kernel works with time units, how does "5" equal to 74?
spec original is
"Host provides at least 74 Clocks before issuing first command"
Let me clarify it,
74 Clocks means 74 clock signals, our Card Reader need to issue more than 74 Clock signals to Card before first command So mdelay(5) , our device will keep issue signal to card in this 5ms make sure over 74
Quoted 4 lines
thanks,
greg k-h
------Please consider the environment before printing this e-mail.
Quoted 23 lines · Ricky WU
After 1ms stabilizing the voltage time
add "Host provides at least 74 Clocks
before issuing first command" that is
spec definitionSigned-off-by Ricky Wu <[email protected]>--- drivers/mmc/host/rtsx_pci_sdmmc.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +2
-0@@ -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 )
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 956
Quoted 10 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +5
-0return 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
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 <[email protected]>
Sent: Monday, February 28, 2022 5:12 PM
To: Ricky WU
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] mmc: rtsx: add 74 Clocks in power on flow
Quoted 23 lines · Ricky WU
After 1ms stabilizing the voltage time
add "Host provides at least 74 Clocks
before issuing first command" that is
spec definitionSigned-off-by Ricky Wu <[email protected]>---
drivers/mmc/host/rtsx_pci_sdmmc.c| 7 +++++++1 file changed, 7 insertions(+)drivers/mmc/host/rtsx_pci_sdmmc.c +2
-0@@ -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)
drivers/mmc/host/rtsx_pci_sdmmc.c static int sd_power_on(struct realtek_pci_sdmmc *host) · line 956
Quoted 10 lines
drivers/mmc/host/rtsx_pci_sdmmc.c +5
-0return 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
[email protected]
maintainerA: 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
Quoted 1 line · Christian Löhle
Can we get
1f311c94aabdb419c28e3147bcc8ab89269f1a7emerged into the stable tree?
Which stable branches do you want this added to?
thanks,
greg k-h
Quoted 7 lines
Quoted 1 line
Can we get
1f311c94aabdb419c28e3147bcc8ab89269f1a7emerged 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
Quoted 11 lines · Christian Löhle
Quoted 8 lines
Quoted 2 lines
Can we get
1f311c94aabdb419c28e3147bcc8ab89269f1a7emerged 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
ff984e57d36e8So 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
[email protected]
maintainerQuoted 13 lines · Ulf Hansson
Quoted 11 lines · Christian Löhle
Quoted 8 lines
Quoted 2 lines
Can we get
1f311c94aabdb419c28e3147bcc8ab89269f1a7emerged 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
ff984e57d36e8So I would suggest adding them to all? Maybe Ricky and Ulf could comment?
Quoted 7 lines
That seems correct.
Although, it's likely that
1f311c94aabddoesn'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