[PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[Franky Lin]

For high clock frequency modes, ie. SDR104 and possibly SDR50, the data
window on the CMD and DAT lines needs to be tuned. Once tuned to a
sample clock rate, a re-tuning might required because of drifted signal
on different lines. This re-tuning is needed and only necessary for
commands using the DAT lines, ie. adtc type commands. This patch is an
optimization by adding the condition of the requested command being adtc
type in order to execute the re-tuning procedure.

Signed-off-by: Franky Lin <frankyl@broadcom.com>
---
 drivers/mmc/host/sdhci.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index cc00bed..f301cd6 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1389,7 +1389,8 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		 * tuning procedure before sending command.
 		 */
 		if ((host->flags & SDHCI_NEEDS_RETUNING) &&
-		    !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) {
+		    !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) &&
+		    mmc_cmd_type(mrq->cmd) == MMC_CMD_ADTC) {
 			if (mmc->card) {
 				/* eMMC uses cmd21 but sd and sdio use cmd19 */
 				tuning_opcode =
-- 
1.7.9.5

Re: [PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[Franky Lin]

Hi Chris,

On 01/07/2014 10:37 AM, Franky Lin wrote:
> For high clock frequency modes, ie. SDR104 and possibly SDR50, the data
> window on the CMD and DAT lines needs to be tuned. Once tuned to a
> sample clock rate, a re-tuning might required because of drifted signal
> on different lines. This re-tuning is needed and only necessary for
> commands using the DAT lines, ie. adtc type commands. This patch is an
> optimization by adding the condition of the requested command being adtc
> type in order to execute the re-tuning procedure.
>
> Signed-off-by: Franky Lin <frankyl@broadcom.com>
> ---

Is this still in your queue? Any remarks on it?

Thanks,
Franky

>   drivers/mmc/host/sdhci.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index cc00bed..f301cd6 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1389,7 +1389,8 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>   		 * tuning procedure before sending command.
>   		 */
>   		if ((host->flags & SDHCI_NEEDS_RETUNING) &&
> -		    !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) {
> +		    !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) &&
> +		    mmc_cmd_type(mrq->cmd) == MMC_CMD_ADTC) {
>   			if (mmc->card) {
>   				/* eMMC uses cmd21 but sd and sdio use cmd19 */
>   				tuning_opcode =
>

Re: [PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[Franky Lin]

On 02/24/2014 11:57 AM, Franky Lin wrote:
> Hi Chris,
>
> On 01/07/2014 10:37 AM, Franky Lin wrote:
>> For high clock frequency modes, ie. SDR104 and possibly SDR50, the data
>> window on the CMD and DAT lines needs to be tuned. Once tuned to a
>> sample clock rate, a re-tuning might required because of drifted signal
>> on different lines. This re-tuning is needed and only necessary for
>> commands using the DAT lines, ie. adtc type commands. This patch is an
>> optimization by adding the condition of the requested command being adtc
>> type in order to execute the re-tuning procedure.
>>
>> Signed-off-by: Franky Lin <frankyl@broadcom.com>
>> ---
>
> Is this still in your queue? Any remarks on it?
>
> Thanks,
> Franky
>

Ping?

>>   drivers/mmc/host/sdhci.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index cc00bed..f301cd6 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1389,7 +1389,8 @@ static void sdhci_request(struct mmc_host *mmc,
>> struct mmc_request *mrq)
>>            * tuning procedure before sending command.
>>            */
>>           if ((host->flags & SDHCI_NEEDS_RETUNING) &&
>> -            !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) {
>> +            !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) &&
>> +            mmc_cmd_type(mrq->cmd) == MMC_CMD_ADTC) {
>>               if (mmc->card) {
>>                   /* eMMC uses cmd21 but sd and sdio use cmd19 */
>>                   tuning_opcode =
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[Franky Lin]

Hi Chris and Ulf,

On 03/07/2014 11:54 AM, Franky Lin wrote:
> On 02/24/2014 11:57 AM, Franky Lin wrote:
>> Hi Chris,
>>
>> On 01/07/2014 10:37 AM, Franky Lin wrote:
>>> For high clock frequency modes, ie. SDR104 and possibly SDR50, the data
>>> window on the CMD and DAT lines needs to be tuned. Once tuned to a
>>> sample clock rate, a re-tuning might required because of drifted signal
>>> on different lines. This re-tuning is needed and only necessary for
>>> commands using the DAT lines, ie. adtc type commands. This patch is an
>>> optimization by adding the condition of the requested command being adtc
>>> type in order to execute the re-tuning procedure.
>>>
>>> Signed-off-by: Franky Lin <frankyl@broadcom.com>
>>> ---
>>
>> Is this still in your queue? Any remarks on it?
>>
>> Thanks,
>> Franky
>>
>
> Ping?

Need to try my luck again. :) Could you please share your thought about 
this patch?

Thanks,
Franky

>
>>>   drivers/mmc/host/sdhci.c |    3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index cc00bed..f301cd6 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1389,7 +1389,8 @@ static void sdhci_request(struct mmc_host *mmc,
>>> struct mmc_request *mrq)
>>>            * tuning procedure before sending command.
>>>            */
>>>           if ((host->flags & SDHCI_NEEDS_RETUNING) &&
>>> -            !(present_state & (SDHCI_DOING_WRITE |
>>> SDHCI_DOING_READ))) {
>>> +            !(present_state & (SDHCI_DOING_WRITE |
>>> SDHCI_DOING_READ)) &&
>>> +            mmc_cmd_type(mrq->cmd) == MMC_CMD_ADTC) {
>>>               if (mmc->card) {
>>>                   /* eMMC uses cmd21 but sd and sdio use cmd19 */
>>>                   tuning_opcode =
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[Ulf Hansson]

On 7 January 2014 19:37, Franky Lin <frankyl@broadcom.com> wrote:
> For high clock frequency modes, ie. SDR104 and possibly SDR50, the data
> window on the CMD and DAT lines needs to be tuned. Once tuned to a
> sample clock rate, a re-tuning might required because of drifted signal
> on different lines. This re-tuning is needed and only necessary for
> commands using the DAT lines, ie. adtc type commands. This patch is an
> optimization by adding the condition of the requested command being adtc
> type in order to execute the re-tuning procedure.
>
> Signed-off-by: Franky Lin <frankyl@broadcom.com>

Hi Franky,

Sorry for the delay.

> ---
>  drivers/mmc/host/sdhci.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index cc00bed..f301cd6 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1389,7 +1389,8 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>                  * tuning procedure before sending command.
>                  */
>                 if ((host->flags & SDHCI_NEEDS_RETUNING) &&
> -                   !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) {
> +                   !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) &&
> +                   mmc_cmd_type(mrq->cmd) == MMC_CMD_ADTC) {

I don't have the in-depth knowledge about the sdhci; Still, to me it
seems like checking for MMC_CMD_ADTC, is that not just the same a
checking if there are a READ/WRITE?

Another note, we really should be working on moving the periodic
"re-tuning trigger" mechanism into the mmc core layer, so all host
driver's could benefit from this. Do you want to help out fixing this?

Kind regards
Ulf Hansson

>                         if (mmc->card) {
>                                 /* eMMC uses cmd21 but sd and sdio use cmd19 */
>                                 tuning_opcode =
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[Franky Lin]

Hi Ulf,

On 03/17/2014 02:12 AM, Ulf Hansson wrote:
> On 7 January 2014 19:37, Franky Lin <frankyl@broadcom.com> wrote:
>> For high clock frequency modes, ie. SDR104 and possibly SDR50, the data
>> window on the CMD and DAT lines needs to be tuned. Once tuned to a
>> sample clock rate, a re-tuning might required because of drifted signal
>> on different lines. This re-tuning is needed and only necessary for
>> commands using the DAT lines, ie. adtc type commands. This patch is an
>> optimization by adding the condition of the requested command being adtc
>> type in order to execute the re-tuning procedure.
>>
>> Signed-off-by: Franky Lin <frankyl@broadcom.com>
>
> Hi Franky,
>
> Sorry for the delay.
>
>> ---
>>   drivers/mmc/host/sdhci.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index cc00bed..f301cd6 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1389,7 +1389,8 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>>                   * tuning procedure before sending command.
>>                   */
>>                  if ((host->flags & SDHCI_NEEDS_RETUNING) &&
>> -                   !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) {
>> +                   !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) &&
>> +                   mmc_cmd_type(mrq->cmd) == MMC_CMD_ADTC) {
>
> I don't have the in-depth knowledge about the sdhci; Still, to me it
> seems like checking for MMC_CMD_ADTC, is that not just the same a
> checking if there are a READ/WRITE?

non-adtc commands like CMD17/CMD24/CMD52 do not use the data lines. 
Tuning is only necessary for adtc commands as CMD18/CMD25/CMD53 which 
involve data lines.

> Another note, we really should be working on moving the periodic
> "re-tuning trigger" mechanism into the mmc core layer, so all host
> driver's could benefit from this. Do you want to help out fixing this?

I thought about this before but hesitated because of non-sdhci drivers. 
I have seen some of them don't have standardized implementation(well if 
they do they would have used sdhci anyway).

Thanks,
Franky

> Kind regards
> Ulf Hansson
>
>>                          if (mmc->card) {
>>                                  /* eMMC uses cmd21 but sd and sdio use cmd19 */
>>                                  tuning_opcode =
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] mmc: sdhci: only invoke clock re-tuning for adtc type commands

[Franky Lin]

On 03/17/2014 09:51 AM, Franky Lin wrote:
> Hi Ulf,
>
> On 03/17/2014 02:12 AM, Ulf Hansson wrote:
>> On 7 January 2014 19:37, Franky Lin <frankyl@broadcom.com> wrote:
>>> For high clock frequency modes, ie. SDR104 and possibly SDR50, the data
>>> window on the CMD and DAT lines needs to be tuned. Once tuned to a
>>> sample clock rate, a re-tuning might required because of drifted signal
>>> on different lines. This re-tuning is needed and only necessary for
>>> commands using the DAT lines, ie. adtc type commands. This patch is an
>>> optimization by adding the condition of the requested command being 
>>> adtc
>>> type in order to execute the re-tuning procedure.
>>>
>>> Signed-off-by: Franky Lin <frankyl@broadcom.com>
>>
>> Hi Franky,
>>
>> Sorry for the delay.
>>
>>> ---
>>>   drivers/mmc/host/sdhci.c |    3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index cc00bed..f301cd6 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1389,7 +1389,8 @@ static void sdhci_request(struct mmc_host 
>>> *mmc, struct mmc_request *mrq)
>>>                   * tuning procedure before sending command.
>>>                   */
>>>                  if ((host->flags & SDHCI_NEEDS_RETUNING) &&
>>> -                   !(present_state & (SDHCI_DOING_WRITE | 
>>> SDHCI_DOING_READ))) {
>>> +                   !(present_state & (SDHCI_DOING_WRITE | 
>>> SDHCI_DOING_READ)) &&
>>> +                   mmc_cmd_type(mrq->cmd) == MMC_CMD_ADTC) {
>>
>> I don't have the in-depth knowledge about the sdhci; Still, to me it
>> seems like checking for MMC_CMD_ADTC, is that not just the same a
>> checking if there are a READ/WRITE?
>
> non-adtc commands like CMD17/CMD24/CMD52 do not use the data lines. 
> Tuning is only necessary for adtc commands as CMD18/CMD25/CMD53 which 
> involve data lines.
>
>> Another note, we really should be working on moving the periodic
>> "re-tuning trigger" mechanism into the mmc core layer, so all host
>> driver's could benefit from this. Do you want to help out fixing this?
>
> I thought about this before but hesitated because of non-sdhci 
> drivers. I have seen some of them don't have standardized 
> implementation(well if they do they would have used sdhci anyway).
>
Ping?