On Fri, Jan 13, 2017 at 3:04 PM, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
> Hi Aisheng,
>
> On 2017/1/13 13:03, Dong Aisheng wrote:
>>
>> Hi Shawn,
>>
>> On Fri, Jan 13, 2017 at 11:24 AM, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
>> wrote:
>>>
>>> It always assumed that if the card was not ready after finishing
>>> switching the mode, we should got a CRC, namely -EILSEQ, from the
>>> hosts. But the fact is if the host is in higher speed mode but the
>>> eMMC havn't finished the switch, so the host could fail to sample
>>> the resp of CMD13 due to the mismatch timing in between. Could it is
>>> possible that response timeout was generated instaed of -EILSEQ? It's
>>> quite IP specificed.
>>
>>
>> I wonder this may not work for IMX since changing timing while card busy
>> may cause host unable to send the next CMD anymore.
>> Then SW timeout (5s or 10s) implemented in common SDHCI driver happens.
>>
>
> (1) If the controller is able to send the next CMD, but generate
> response timeout, we should take this into account as the card is
> still in busy state, so let's retry it.
>
> (2) However, for your cases, the next CMD couldn't be sent out and SW
> timeout rountine break out the loop as the SW timeout should take much
> longer than generic_cmd6_time, which means the timeout value is expired
> immediately after getting err from host, namely SW timeout rountine
> throw out a -ETIMEDOUT, right? So it just behave the same and didn't
> improve anything.
>
Yes, it does not help for IMX.
> So I was just wanna cover the case (1). Thought?
>
Yes, it's for case (1).
A bit more thinking that as the physical spec defined in section 4.3.10.5,
The host is advised not to Issue any command during a CMD6 transaction.
It's risky operation.
Probably we could better not allow CMD13 polling during CMD6 swithc and force
controller drivers provide ops->card_busy() function to check CMD6 complete
if they not support MMC_CAP_WAIT_WHILE_BUSY.
Most controller should have such capbility.
No sure if any exception!
Regards
Dong Aisheng
>
>
>
>
>> Regards
>> Dong Aisheng
>>
>>> So I don't think we should take the risk of
>>> relying on that. In another word, we don't expect to bail out early
>>> for timeout errors bounced from hosts when polling the status, no
>>> just for explicit CRC.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
>>>
>>> ---
>>>
>>> drivers/mmc/core/mmc_ops.c | 14 +++++++++-----
>>> drivers/mmc/core/mmc_ops.h | 3 ++-
>>> 2 files changed, 11 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>> index db2969f..a352dea 100644
>>> --- a/drivers/mmc/core/mmc_ops.c
>>> +++ b/drivers/mmc/core/mmc_ops.c
>>> @@ -451,7 +451,7 @@ int mmc_switch_status(struct mmc_card *card)
>>> }
>>>
>>> static int mmc_poll_for_busy(struct mmc_card *card, unsigned int
>>> timeout_ms,
>>> - bool send_status, bool retry_crc_err)
>>> + bool send_status, bool retry_crc_and_tmo_err)
>>> {
>>> struct mmc_host *host = card->host;
>>> int err;
>>> @@ -486,7 +486,8 @@ static int mmc_poll_for_busy(struct mmc_card *card,
>>> unsigned int timeout_ms,
>>> busy = host->ops->card_busy(host);
>>> } else {
>>> err = mmc_send_status(card, &status);
>>> - if (retry_crc_err && err == -EILSEQ) {
>>> + if (retry_crc_and_tmo_err &&
>>> + (err == -EILSEQ || err == -ETIMEDOUT)) {
>>> busy = true;
>>> } else if (err) {
>>> return err;
>>> @@ -523,13 +524,15 @@ static int mmc_poll_for_busy(struct mmc_card *card,
>>> unsigned int timeout_ms,
>>> * @timing: new timing to change to
>>> * @use_busy_signal: use the busy signal as response type
>>> * @send_status: send status cmd to poll for busy
>>> - * @retry_crc_err: retry when CRC errors when polling with CMD13 for
>>> busy
>>> + * @retry_crc_and_tmo_err: retry when CRC errors or response timeout
>>> errors
>>> + * when polling with CMD13 for busy
>>> *
>>> * Modifies the EXT_CSD register for selected card.
>>> */
>>> int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>>> unsigned int timeout_ms, unsigned char timing,
>>> - bool use_busy_signal, bool send_status, bool
>>> retry_crc_err)
>>> + bool use_busy_signal, bool send_status,
>>> + bool retry_crc_and_tmo_err)
>>> {
>>> struct mmc_host *host = card->host;
>>> int err;
>>> @@ -590,7 +593,8 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8
>>> index, u8 value,
>>> }
>>>
>>> /* Let's try to poll to find out when the command is completed.
>>> */
>>> - err = mmc_poll_for_busy(card, timeout_ms, send_status,
>>> retry_crc_err);
>>> + err = mmc_poll_for_busy(card, timeout_ms, send_status,
>>> + retry_crc_and_tmo_err);
>>>
>>> out_tim:
>>> if (err && timing)
>>> diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
>>> index abd525e..fad9963 100644
>>> --- a/drivers/mmc/core/mmc_ops.h
>>> +++ b/drivers/mmc/core/mmc_ops.h
>>> @@ -31,7 +31,8 @@
>>> int __mmc_switch_status(struct mmc_card *card, bool crc_err_fatal);
>>> int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
>>> unsigned int timeout_ms, unsigned char timing,
>>> - bool use_busy_signal, bool send_status, bool
>>> retry_crc_err);
>>> + bool use_busy_signal, bool send_status,
>>> + bool retry_crc_and_tmo_err);
>>>
>>> #endif
>>>
>>> --
>>> 1.9.1
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>>
>
>
> --
> Best Regards
> Shawn Lin
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
