[U-Boot] [PATCH v1] mmc: dwmmc: Enable small delay before returning error

[Marek Vasut]

On 2/20/19 2:57 PM, Ang, Chee Hong wrote:
> On Mon, 2019-02-18 at 21:38 +0100, Marek Vasut wrote:
>> On 2/18/19 3:51 PM, Ang, Chee Hong wrote:
>>>
>>> On Mon, 2019-02-18 at 12:57 +0100, Marek Vasut wrote:
>>>>
>>>> On 2/18/19 5:16 AM, chee.hong.ang at intel.com wrote:
>>>>>
>>>>>
>>>>> From: "Ang, Chee Hong" <chee.hong.ang at intel.com>
>>>>>
>>>>> 'SET_BLOCKLEN' may occasionally fail on first attempt.
>>>> Why ?
>>> This is part of the workaround of mmc driver which is enabled with
>>> 'CONFIG_MMC_QUIRKS' config.
>>> https://github.com/u-boot/u-boot/blob/dbe70c7d4e3d5c705a98d82952e05
>>> a591
>>> efd0683/drivers/mmc/mmc.c#L272
>> OK, let's take a step back. What problem do you observe, that you're
>> trying to fix ?
> See my detail explanation below.
>>
>>>
>>>>
>>>>>
>>>>> This patch enable a small delay in dwmci_send_cmd() on
>>>>> busy, I/O or CRC error to allow the MMC controller recovers
>>>>> from the failure/error on subsequent retries.
>>>> Why 100 uS ?
>>> This is a good question. Perhaps the driver's authors can explain
>>> this.
>>> The driver delay 100us after dwmci_send_cmd() success with the
>>> command
>>> sent. But it never delay 100us whenever mmc controller response to
>>> the
>>> command sent with I/O or CRC errors. MMC drivers expect the first
>>> 'SET_BLOCKEN' command issued by dwmci_send_cmd() may fail
>>> intermittently so it will retry up to 4 times before it gave up and
>>> return error. Without this 100us delay after error response,
>>> 'SET_BLOCKEN' may sometime fail in 4 attempts in a row. Therefore,
>>> we
>>> encountered intermittent failure in loading u-boot (SSBL) from MMC.
>> Can you be more specific about the failure you saw ?
> 
> Here are the steps for booting u-boot (SSBL) from MMC:
> 
> 1) SPL (FSBL) get loaded to OCRAM
> 2) SPL try to load uboot as SSBL from MMC
> 3) SPL issue 'SET_BLOCKEN' to MMC controller

s/SPL/MMC framework via DWMMC driver/

> 4) If MMC controller response with error continue to step 5 else go to
> step 7
> 5) If is less than 4 attempts go to step 3 else continue to step 6
> 6) Indicate to user there was an error loading uboot as SSBL from MMC
> and stop here
> 7) Send command to read blocks of data from MMC
> 8) uboot successfully loaded to DDR memory
> 9) SPL jumps to uboot and continue the boot flow.
> 
> So this patch actually introduce a small delay at step 4. Without this
> small delay, step 4 might sometime fail in all 4 attempts to issue the
> 'SET_BLOCKEN' command to MMC controller.

Isn't what you observe here some sort of timeout which you need to
respect when the card rejects a command ? If so, such timeout should be
described in either of the SD or MMC specification and it should be in
the MMC framework core instead of a driver.

> Note that this failure is
> intermittent. If it fails at step 3 and there is no small delay in step
> 4, it will most likely fail in subsequent attempts.
>>
>>>
>>>>
>>>> Can we rather detect whether the controller recovered by polling
>>>> some
>>>> bit?
>>> Hmmm...I can take a look at the databook of this controller and dig
>>> further. Probably this is the limitation of the controller itself.
>>> The
>>> mmc driver code which introduce 100us delay after every command
>>> sent in
>>> dwmci_send_cmd() looks iffy.
>> If you have access to it, please do.
> Unfortunately, I can't find any registers I can poll for the status of
> the readiness of the controller from DesignWare storage controller
> databook.
>>
>> btw do you have the same problem if you disable caches ?
> The error happen while SPL trying to load uboot as SSBL from MMC into
> DDR memory.At this point, the caches are disabled.

I-Cache is enabled in SPL, D-cache _might_ be too.

Do you get this error after cold boot too ?

-- 
Best regards,
Marek Vasut