linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] mmc: Provide enhanced strobe support for sdhci-msm
@ 2016-12-20  5:53 Ritesh Harjani
  2016-12-20  5:53 ` [PATCH 1/4] mmc: core: Return from mmc_set_clock if hz is already set to ios.clock Ritesh Harjani
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-20  5:53 UTC (permalink / raw)
  To: ulf.hansson, adrian.hunter
  Cc: linux-mmc, shawn.lin, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd,
	Ritesh Harjani

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(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] mmc: core: Return from mmc_set_clock if hz is already set to ios.clock
  2016-12-20  5:53 [PATCH 0/4] mmc: Provide enhanced strobe support for sdhci-msm Ritesh Harjani
@ 2016-12-20  5:53 ` Ritesh Harjani
  2016-12-20  5:53 ` [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe Ritesh Harjani
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-20  5:53 UTC (permalink / raw)
  To: ulf.hansson, adrian.hunter
  Cc: linux-mmc, shawn.lin, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd,
	Ritesh Harjani

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/core/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 543eadd..125f8a9 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -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);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
  2016-12-20  5:53 [PATCH 0/4] mmc: Provide enhanced strobe support for sdhci-msm Ritesh Harjani
  2016-12-20  5:53 ` [PATCH 1/4] mmc: core: Return from mmc_set_clock if hz is already set to ios.clock Ritesh Harjani
@ 2016-12-20  5:53 ` Ritesh Harjani
  2016-12-20  7:29   ` Shawn Lin
  2016-12-20  5:53 ` [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing Ritesh Harjani
  2016-12-20  5:53 ` [PATCH 4/4] mmc: sdhci-msm: provide enhanced_strobe mode feature support Ritesh Harjani
  3 siblings, 1 reply; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-20  5:53 UTC (permalink / raw)
  To: ulf.hansson, adrian.hunter
  Cc: linux-mmc, shawn.lin, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd,
	Ritesh Harjani

Currently 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.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/core/mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index b61b52f9..eb69497 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -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;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing
  2016-12-20  5:53 [PATCH 0/4] mmc: Provide enhanced strobe support for sdhci-msm Ritesh Harjani
  2016-12-20  5:53 ` [PATCH 1/4] mmc: core: Return from mmc_set_clock if hz is already set to ios.clock Ritesh Harjani
  2016-12-20  5:53 ` [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe Ritesh Harjani
@ 2016-12-20  5:53 ` Ritesh Harjani
  2016-12-23  9:18   ` Ritesh Harjani
  2016-12-20  5:53 ` [PATCH 4/4] mmc: sdhci-msm: provide enhanced_strobe mode feature support Ritesh Harjani
  3 siblings, 1 reply; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-20  5:53 UTC (permalink / raw)
  To: ulf.hansson, adrian.hunter
  Cc: linux-mmc, shawn.lin, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd,
	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_timing in mmc_select_hs400es.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/core/mmc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index eb69497..052368e 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -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);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] mmc: sdhci-msm: provide enhanced_strobe mode feature support
  2016-12-20  5:53 [PATCH 0/4] mmc: Provide enhanced strobe support for sdhci-msm Ritesh Harjani
                   ` (2 preceding siblings ...)
  2016-12-20  5:53 ` [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing Ritesh Harjani
@ 2016-12-20  5:53 ` Ritesh Harjani
  3 siblings, 0 replies; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-20  5:53 UTC (permalink / raw)
  To: ulf.hansson, adrian.hunter
  Cc: linux-mmc, shawn.lin, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd,
	Ritesh Harjani

This provide support for enhanced_strobe feature to sdhci-msm.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 32879b8..d092a16 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -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
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
  2016-12-20  5:53 ` [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe Ritesh Harjani
@ 2016-12-20  7:29   ` Shawn Lin
  2016-12-20 10:33     ` Ritesh Harjani
  0 siblings, 1 reply; 11+ messages in thread
From: Shawn Lin @ 2016-12-20  7:29 UTC (permalink / raw)
  To: Ritesh Harjani, ulf.hansson, adrian.hunter
  Cc: shawn.lin, linux-mmc, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd

On 2016/12/20 13:53, Ritesh Harjani wrote:
> Currently 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.

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.

>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/core/mmc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index b61b52f9..eb69497 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -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;
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
  2016-12-20  7:29   ` Shawn Lin
@ 2016-12-20 10:33     ` Ritesh Harjani
  2016-12-20 11:03       ` Shawn Lin
  0 siblings, 1 reply; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-20 10:33 UTC (permalink / raw)
  To: Shawn Lin, ulf.hansson, adrian.hunter
  Cc: linux-mmc, linux-kernel, stummala, georgi.djakov, linux-arm-msm,
	pramod.gurav, jeremymc, venkatg, asutoshd

Hi Shawn,

On 12/20/2016 12:59 PM, Shawn Lin wrote:
> On 2016/12/20 13:53, Ritesh Harjani wrote:
>> Currently 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.
>
> 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.
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

>
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/core/mmc.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index b61b52f9..eb69497 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -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;
>>
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
  2016-12-20 10:33     ` Ritesh Harjani
@ 2016-12-20 11:03       ` Shawn Lin
  2016-12-20 12:08         ` Ritesh Harjani
  0 siblings, 1 reply; 11+ messages in thread
From: Shawn Lin @ 2016-12-20 11:03 UTC (permalink / raw)
  To: Ritesh Harjani, ulf.hansson, adrian.hunter
  Cc: shawn.lin, linux-mmc, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd

On 2016/12/20 18:33, Ritesh Harjani wrote:
> Hi Shawn,
>
> On 12/20/2016 12:59 PM, Shawn Lin wrote:
>> On 2016/12/20 13:53, Ritesh Harjani wrote:
>>> Currently 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.
>>
>> 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.
> 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.

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?

>
> 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 ?
>

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.

>
> Regards
> Ritesh
>
>>
>>>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>> ---
>>>  drivers/mmc/core/mmc.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index b61b52f9..eb69497 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -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;
>>>
>>
>>
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe
  2016-12-20 11:03       ` Shawn Lin
@ 2016-12-20 12:08         ` Ritesh Harjani
  0 siblings, 0 replies; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-20 12:08 UTC (permalink / raw)
  To: Shawn Lin, ulf.hansson, adrian.hunter
  Cc: linux-mmc, linux-kernel, stummala, georgi.djakov, linux-arm-msm,
	pramod.gurav, jeremymc, venkatg, asutoshd

Hi Shawn,

On 12/20/2016 4:33 PM, Shawn Lin wrote:
> On 2016/12/20 18:33, Ritesh Harjani wrote:
>> Hi Shawn,
>>
>> On 12/20/2016 12:59 PM, Shawn Lin wrote:
>>> On 2016/12/20 13:53, Ritesh Harjani wrote:
>>>> Currently 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.
>>>
>>> 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.
>> 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.
>
> 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?
>
>>
>> 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 ?
>>
>
> 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




>
>>
>> Regards
>> Ritesh
>>
>>>
>>>>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>> ---
>>>>  drivers/mmc/core/mmc.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index b61b52f9..eb69497 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -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;
>>>>
>>>
>>>
>>
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing
  2016-12-20  5:53 ` [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing Ritesh Harjani
@ 2016-12-23  9:18   ` Ritesh Harjani
  2016-12-26  7:12     ` Shawn Lin
  0 siblings, 1 reply; 11+ messages in thread
From: Ritesh Harjani @ 2016-12-23  9:18 UTC (permalink / raw)
  To: ulf.hansson, adrian.hunter, shawn.lin
  Cc: linux-mmc, linux-kernel, stummala, georgi.djakov, linux-arm-msm,
	pramod.gurav, jeremymc, venkatg, asutoshd

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

On 12/20/2016 11:23 AM, Ritesh Harjani wrote:
> 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_timing in mmc_select_hs400es.
>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/core/mmc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index eb69497..052368e 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -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);
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing
  2016-12-23  9:18   ` Ritesh Harjani
@ 2016-12-26  7:12     ` Shawn Lin
  0 siblings, 0 replies; 11+ messages in thread
From: Shawn Lin @ 2016-12-26  7:12 UTC (permalink / raw)
  To: Ritesh Harjani, ulf.hansson, adrian.hunter
  Cc: shawn.lin, linux-mmc, linux-kernel, stummala, georgi.djakov,
	linux-arm-msm, pramod.gurav, jeremymc, venkatg, asutoshd

On 2016/12/23 17:18, Ritesh Harjani wrote:
> Hi Shawn,
>
> Do you think, below change should be fine?

yes.

> 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 ?
>

Sure.

>
> Regards
> Ritesh
>
> On 12/20/2016 11:23 AM, Ritesh Harjani wrote:
>> 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_timing in mmc_select_hs400es.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/core/mmc.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index eb69497..052368e 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -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);
>>
>>
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-12-26  7:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-20  5:53 [PATCH 0/4] mmc: Provide enhanced strobe support for sdhci-msm Ritesh Harjani
2016-12-20  5:53 ` [PATCH 1/4] mmc: core: Return from mmc_set_clock if hz is already set to ios.clock Ritesh Harjani
2016-12-20  5:53 ` [PATCH 2/4] mmc: mmc: Add change to set controller to HS400 frequency in enhanced strobe Ritesh Harjani
2016-12-20  7:29   ` Shawn Lin
2016-12-20 10:33     ` Ritesh Harjani
2016-12-20 11:03       ` Shawn Lin
2016-12-20 12:08         ` Ritesh Harjani
2016-12-20  5:53 ` [PATCH 3/4] mmc: mmc: enable ios.enhanced_strobe before mmc_set_timing Ritesh Harjani
2016-12-23  9:18   ` Ritesh Harjani
2016-12-26  7:12     ` Shawn Lin
2016-12-20  5:53 ` [PATCH 4/4] mmc: sdhci-msm: provide enhanced_strobe mode feature support Ritesh Harjani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).