mmc: Provide enhanced strobe support for sdhci-msm
Subsystems (3)
cover mmc: Provide enhanced strobe support for sdhci-msm
Hi,
Patch series is to enable enhanced strobe feature for sdhci-msm driver. But there are also some changes done in core layer for this.
This was tested on msm8996 based interal target with emmc-5.1 card.
Ritesh Harjani (4):
mmc: core: Return from mmc_set_clock if hz is already set to ios.clock
mmc: mmc: Add change to set controller to HS400 frequency in enhanced
strobe
mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing
mmc: sdhci-msm: provide enhanced_strobe mode feature support
drivers/mmc/core/core.c | 3 +++ drivers/mmc/core/mmc.c | 9 ++++++++- drivers/mmc/host/sdhci-msm.c | 32 +++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 10 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/4 mmc: core: Return from mmc_set_clock if hz is already set to ios.clock
--- drivers/mmc/core/core.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/mmc/core/core.c +3 -0
@@ -1139,6 +1139,9 @@ void mmc_set_clock(struct mmc_host *host, unsigned int hz)if (hz > host->f_max) hz = host->f_max; +if (hz == host->ios.clock) +return ; + host->ios.clock = hz;mmc_set_ios (host); }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 2/4 mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
In reply to Ritesh HarjaniCurrently mmc_select_hs400es is not setting the desired frequency before sending switch command. This makes CMD13 to timeout on some controllers.
Thus add a change to set the desired HS400 frequency
in mmc_select_hs400es when the timing is switched to HS400.
--- drivers/mmc/core/mmc.c | 1 + 1 file changed, 1 insertion(+)
drivers/mmc/core/mmc.c +1 -0
@@ -1329,6 +1329,7 @@ static int mmc_select_hs400es(struct mmc_card *card)/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 ); +mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ host->ios.enhanced_strobe = true;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
Quoted 5 lines · Ritesh Harjani
Currently
mmc_select_hs400esis not setting the desired frequency before sending switch command. This makes CMD13 to timeout on some controllers.
Thus add a change to set the desired HS400 frequency
inmmc_select_hs400eswhen the timing is switched to HS400.
What is the desired HS400 freq? I guess it is 200MHz, right?
Well, per the spec, it just say "host *may* changes frequency to <= 200Mhz". So before setting timing to MMC_TIMING_MMC_HS400, we already reach ext_csd.hs_max_dtr. So it should meet the spec. If the fact that some controllers would see CMD13 timeout here, I guess you wouldn't let folks add max-frequency to the DT as if the max freq is set to just lower than your *desired HS400 freq* , it the same failure result even you add this patch, right?
the root cause the controllers see failure for CMD13 is it didn't configure the right delay phase or something similar to fit the timing under the freq of hs_max_dtr, I guess.
Quoted 19 lines
Signed-off-by Ritesh Harjani <[email protected]>--- drivers/mmc/core/mmc.c | 1 + 1 file changed, 1 insertion(+)drivers/mmc/core/mmc.c +1
-0@@ -1329,6 +1329,7 @@ static int mmc_select_hs400es(struct mmc_card *card)/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 ); +mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ host->ios.enhanced_strobe = true;
Signature · 2 lines
Best Regards Shawn Lin
Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
Hi Shawn,
Quoted 17 lines · Shawn Lin
Quoted 5 lines · Ritesh Harjani
Currently
mmc_select_hs400esis not setting the desired frequency before sending switch command. This makes CMD13 to timeout on some controllers.
Thus add a change to set the desired HS400 frequency
inmmc_select_hs400eswhen the timing is switched to HS400.What is the desired HS400 freq? I guess it is 200MHz, right?
Well, per the spec, it just say "host *may* changes frequency to <= 200Mhz". So before setting timing to
MMC_TIMING_MMC_HS400, we already reach ext_csd.hs_max_dtr. So it should meet the spec. If the fact that some controllers would see CMD13 timeout here, I guess you wouldn't let folks add max-frequency to the DT as if the max freq is set to just lower than your *desired HS400 freq* , it the same failure result even you add this patch, right?
Quoted 3 lines
the root cause the controllers see failure for CMD13 is it didn't configure the right delay phase or something similar to fit the timing under the freq of
hs_max_dtr, I guess.
Actually this may be limitation of sdhc msm controller that it cannot work at lower frequency while in HS400 mode. I would have to see more on this to confirm on why this limitation is there (mostly it was with some DLL limitation). In that case maybe I should not generalize this for other controllers.
But still the below code change to set the bus_speed after setting the timing should be fine right ? Since host controller may as well change the frequency to 200MHz.
Do you think the below change is fine in this sense? I can change the commit message to just - "set both timing mode and frequency before sending CMD13 in mmc_select_hs400es."
Or do you see any other approach to this ?
Regards
Ritesh
Quoted 23 lines
Quoted 21 lines
Signed-off-by Ritesh Harjani <[email protected]>--- drivers/mmc/core/mmc.c | 1 + 1 file changed, 1 insertion(+)drivers/mmc/core/mmc.c +1
-0@@ -1329,6 +1329,7 @@ static int mmc_select_hs400es(struct mmc_card *card)/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 ); +mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ host->ios.enhanced_strobe = true;
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
Quoted 30 lines · Ritesh Harjani
Hi Shawn,
Quoted 17 lines · Shawn Lin
Quoted 5 lines · Ritesh Harjani
Currently
mmc_select_hs400esis not setting the desired frequency before sending switch command. This makes CMD13 to timeout on some controllers.
Thus add a change to set the desired HS400 frequency
inmmc_select_hs400eswhen the timing is switched to HS400.What is the desired HS400 freq? I guess it is 200MHz, right?
Well, per the spec, it just say "host *may* changes frequency to <= 200Mhz". So before setting timing to
MMC_TIMING_MMC_HS400, we already reach ext_csd.hs_max_dtr. So it should meet the spec. If the fact that some controllers would see CMD13 timeout here, I guess you wouldn't let folks add max-frequency to the DT as if the max freq is set to just lower than your *desired HS400 freq* , it the same failure result even you add this patch, right?Quoted 3 lines
the root cause the controllers see failure for CMD13 is it didn't configure the right delay phase or something similar to fit the timing under the freq of
hs_max_dtr, I guess.Actually this may be limitation of sdhc msm controller that it cannot work at lower frequency while in HS400 mode. I would have to see more on this to confirm on why this limitation is there (mostly it was with some DLL limitation).
In that case maybe I should not generalize this for other controllers.
Quoted 3 lines
But still the below code change to set the
bus_speedafter setting the timing should be fine right ? Since host controller may as well change the frequency to 200MHz.
maybe not, I was guessing what would happend when folks add max-frequency = <100000000> as well as mmc-hs400-enhanced-strobe in the DT? It won't work for your case. So the question is how you would make that case work?
Quoted 8 lines
Do you think the below change is fine in this sense? I can change the commit message to just -
"set both timing mode and frequency before sending CMD13 inmmc_select_hs400es."Or do you see any other approach to this ?
I haven't had clear thought about how to deal with it but I guess you are not alone for this case maybe. Now there will be more and more mmc controllers using PHY or DLL and some more special limitions is ineluctable, so we need to discuss a more general/legit way to make it work.
Your patch is a good begin to open this topic. :) I would like to hear some input from Ulf and Adian.
Quoted 28 lines
Regards
RiteshQuoted 23 lines
Quoted 21 lines
Signed-off-by Ritesh Harjani <[email protected]>--- drivers/mmc/core/mmc.c | 1 + 1 file changed, 1 insertion(+)drivers/mmc/core/mmc.c +1
-0@@ -1329,6 +1329,7 @@ static int mmc_select_hs400es(struct mmc_card *card)/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 ); +mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ host->ios.enhanced_strobe = true;
Signature · 2 lines
Best Regards Shawn Lin
Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
Hi Shawn,
Quoted 49 lines · Shawn Lin
Quoted 30 lines · Ritesh Harjani
Hi Shawn,
Quoted 17 lines · Shawn Lin
Quoted 5 lines · Ritesh Harjani
Currently
mmc_select_hs400esis not setting the desired frequency before sending switch command. This makes CMD13 to timeout on some controllers.
Thus add a change to set the desired HS400 frequency
inmmc_select_hs400eswhen the timing is switched to HS400.What is the desired HS400 freq? I guess it is 200MHz, right?
Well, per the spec, it just say "host *may* changes frequency to <= 200Mhz". So before setting timing to
MMC_TIMING_MMC_HS400, we already reach ext_csd.hs_max_dtr. So it should meet the spec. If the fact that some controllers would see CMD13 timeout here, I guess you wouldn't let folks add max-frequency to the DT as if the max freq is set to just lower than your *desired HS400 freq* , it the same failure result even you add this patch, right?Quoted 3 lines
the root cause the controllers see failure for CMD13 is it didn't configure the right delay phase or something similar to fit the timing under the freq of
hs_max_dtr, I guess.Actually this may be limitation of sdhc msm controller that it cannot work at lower frequency while in HS400 mode. I would have to see more on this to confirm on why this limitation is there (mostly it was with some DLL limitation).
In that case maybe I should not generalize this for other controllers.Quoted 3 lines
But still the below code change to set the
bus_speedafter setting the timing should be fine right ? Since host controller may as well change the frequency to 200MHz.maybe not, I was guessing what would happend when folks add max-frequency = <100000000> as well as mmc-hs400-enhanced-strobe in the DT? It won't work for your case. So the question is how you would make that case work?
Quoted 8 lines
Do you think the below change is fine in this sense? I can change the commit message to just -
"set both timing mode and frequency before sending CMD13 inmmc_select_hs400es."Or do you see any other approach to this ?
Quoted 8 lines
I haven't had clear thought about how to deal with it but I guess you are not alone for this case maybe. Now there will be more and more mmc controllers using PHY or DLL and some more special limitions is ineluctable, so we need to discuss a more general/legit way to make it work.
Your patch is a good begin to open this topic. :) I would like to hear some input from Ulf and Adian.
I see there is a miss from my side here. Although there is still some limitation on msm host controller which I cannot recollect. But for this case we can get it done by configuring the DLL properly.
I missed that particular check of clock >= 100MHz in sdhci_msm_set_uhs_signaling which configures the DLL.
I guess if I could use the host->mmc_host_ops.hs400_enhanced_strobe ops and provide a callback to configure the DLL without any restriction on clock frequency, then this can be worked out.
I will check more on this and get back. Thanks for your inputs.
Regards
Ritesh
Quoted 31 lines
Quoted 29 lines
Regards
RiteshQuoted 23 lines
Quoted 21 lines
Signed-off-by Ritesh Harjani <[email protected]>--- drivers/mmc/core/mmc.c | 1 + 1 file changed, 1 insertion(+)drivers/mmc/core/mmc.c +1
-0@@ -1329,6 +1329,7 @@ static int mmc_select_hs400es(struct mmc_card *card)/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 ); +mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ host->ios.enhanced_strobe = true;
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 3/4 mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing
In reply to Ritesh HarjaniSome controllers may need to configure few registers based on enhanced strobe mode while configuring to HS400 timing, thus make ios.enhanced_strobe to true before mmc_set_timing in mmc_select_hs400es.
--- drivers/mmc/core/mmc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/mmc/core/mmc.c +7 -1
@@ -1327,12 +1327,18 @@ static int mmc_select_hs400es(struct mmc_card *card) goto out_err; } +/* +* Enable enhanced_strobe in ios, as some controllers +* may need to configure few registers based on enhanced +* strobe while changing HS400 timing. +*/ + host->ios.enhanced_strobe = true; +/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 );mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ - host->ios.enhanced_strobe = true;if (host->ops->hs400_enhanced_strobe) host->ops->hs400_enhanced_strobe (host, &host->ios);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing
Hi Shawn,
Do you think, below change should be fine? I am still checking on what we discussed on Patch2.
why I am asking is because - for HS400 in SDHC-MSM, we do the DLL re-calibration as per the HW sequence. So it is done in both the cases, for HS400 mode without and with enhanced strobe mode.
This can be done as part of set_ios when mmc_set_timing is called in sdhci-msm driver.
I am still trying to check more on what would be more generic and appropriate way inside sdhci-msm, for that I would like to know if ios.enhanced_strobe = true before calling mmc_set_timing should be acceptable or not ?
Regards
Ritesh
Quoted 34 lines · Ritesh Harjani
Some controllers may need to configure few registers based on enhanced strobe mode while configuring to HS400 timing, thus make ios.enhanced_strobe to true before
mmc_set_timinginmmc_select_hs400es.Signed-off-by Ritesh Harjani <[email protected]>--- drivers/mmc/core/mmc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)drivers/mmc/core/mmc.c +7
-1@@ -1327,12 +1327,18 @@ static int mmc_select_hs400es(struct mmc_card *card) goto out_err; } +/* +* Enable enhanced_strobe in ios, as some controllers +* may need to configure few registers based on enhanced +* strobe while changing HS400 timing. +*/ + host->ios.enhanced_strobe = true; +/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 );mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ - host->ios.enhanced_strobe = true;if (host->ops->hs400_enhanced_strobe) host->ops->hs400_enhanced_strobe (host, &host->ios);
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing
Quoted 3 lines · Ritesh Harjani
Hi Shawn,
Do you think, below change should be fine?
yes.
Quoted 6 lines
I am still checking on what we discussed on Patch2.
why I am asking is because - for HS400 in SDHC-MSM, we do the DLL re-calibration as per the HW sequence. So it is done in both the cases, for HS400 mode without and with enhanced strobe mode.
Quoted 8 lines
This can be done as part of
set_ioswhenmmc_set_timingis called in sdhci-msm driver.I am still trying to check more on what would be more generic and appropriate way inside sdhci-msm, for that I would like to know if ios.enhanced_strobe = true before calling
mmc_set_timingshould be acceptable or not ?
Sure.
Quoted 41 lines
Regards
RiteshQuoted 35 lines · Ritesh Harjani
Some controllers may need to configure few registers based on enhanced strobe mode while configuring to HS400 timing, thus make ios.enhanced_strobe to true before
mmc_set_timinginmmc_select_hs400es.Signed-off-by Ritesh Harjani <[email protected]>--- drivers/mmc/core/mmc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)drivers/mmc/core/mmc.c +7
-1@@ -1327,12 +1327,18 @@ static int mmc_select_hs400es(struct mmc_card *card) goto out_err; } +/* +* Enable enhanced_strobe in ios, as some controllers +* may need to configure few registers based on enhanced +* strobe while changing HS400 timing. +*/ + host->ios.enhanced_strobe = true; +/* Set host controller to HS400 timing and frequency */ mmc_set_timing (host,MMC_TIMING_MMC_HS400 );mmc_set_bus_speed (card);/* Controller enable enhanced strobe function */ - host->ios.enhanced_strobe = true;if (host->ops->hs400_enhanced_strobe) host->ops->hs400_enhanced_strobe (host, &host->ios);
Signature · 2 lines
Best Regards Shawn Lin
PATCH 4/4 mmc: sdhci-msm: provide enhanced_strobe mode feature support
In reply to Ritesh HarjaniThis provide support for enhanced_strobe feature to sdhci-msm.
--- drivers/mmc/host/sdhci-msm.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
drivers/mmc/host/sdhci-msm.c +23 -9
@@ -102,6 +102,7 @@#define CORE_DDR_200_CFG 0x184#define CORE_CDC_T4_DLY_SEL BIT(0) +#define CORE_CMDIN_RCLK_EN BIT(1)#define CORE_START_CDC_TRAFFIC BIT(6)#define CORE_VENDOR_SPEC3 0x1b0#define CORE_PWRSAVE_DLL BIT(3)
@@ -579,6 +580,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)static int sdhci_msm_cm_dll_sdc4_calibration (struct sdhci_host *host) { +struct mmc_host *mmc = host->mmc;u32 dll_status, config;int ret;
@@ -593,6 +595,12 @@ static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)*/ writel_relaxed (DDR_CONFIG_POR_VAL , host->ioaddr +CORE_DDR_CONFIG ); +if (mmc->ios.enhanced_strobe) { + config =readl_relaxed (host->ioaddr +CORE_DDR_200_CFG ); + config |=CORE_CMDIN_RCLK_EN ; +writel_relaxed (config, host->ioaddr +CORE_DDR_200_CFG ); + } + config =readl_relaxed (host->ioaddr +CORE_DLL_CONFIG_2 ); config |=CORE_DDR_CAL_EN ;writel_relaxed (config, host->ioaddr +CORE_DLL_CONFIG_2 );
@@ -627,6 +635,7 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host) {struct sdhci_pltfm_host *pltfm_host =sdhci_priv (host);struct sdhci_msm_host *msm_host =sdhci_pltfm_priv (pltfm_host); +struct mmc_host *mmc = host->mmc;int ret;u32 config;
@@ -640,14 +649,17 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)if (ret) goto out; -/* Set the selected phase in delay line hw block */ - ret =msm_config_cm_dll_phase (host, msm_host->saved_tuning_phase); -if (ret) - goto out; +if (!mmc->ios.enhanced_strobe) { +/* Set the selected phase in delay line hw block */ + ret =msm_config_cm_dll_phase (host, + msm_host->saved_tuning_phase); +if (ret) + goto out; + config =readl_relaxed (host->ioaddr +CORE_DLL_CONFIG ); + config |=CORE_CMD_DAT_TRACK_SEL ; +writel_relaxed (config, host->ioaddr +CORE_DLL_CONFIG ); + } - config =readl_relaxed (host->ioaddr +CORE_DLL_CONFIG ); - config |=CORE_CMD_DAT_TRACK_SEL ; -writel_relaxed (config, host->ioaddr +CORE_DLL_CONFIG );if (msm_host->use_cdclp533) ret =sdhci_msm_cdclp533_calibration (host);else
@@ -802,7 +814,8 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,spin_unlock_irq (&host->lock);/* CDCLP533 HW calibration is only required for HS400 mode*/ if (host->clock >CORE_FREQ_100MHZ && - msm_host->tuning_done && !msm_host->calibration_done && + (msm_host->tuning_done || mmc->ios.enhanced_strobe) && + !msm_host->calibration_done && mmc->ios.timing ==MMC_TIMING_MMC_HS400 )if (!sdhci_msm_hs400_dll_calibration (host)) msm_host->calibration_done = true;
@@ -941,7 +954,8 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)* Select HS400 mode using the HC_SELECT_IN from VENDOR SPEC * register */ -if (msm_host->tuning_done && !msm_host->calibration_done) { +if ((msm_host->tuning_done || curr_ios.enhanced_strobe) && + !msm_host->calibration_done) {/* * Write 0x6 to HC_SELECT_IN and 1 to HC_SELECT_IN_EN * field in VENDOR_SPEC_FUNC
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.