[U-Boot] [RFC] mmc:fix: Increase the timeout value for SDHCI_send_command()

[Jagan Teki]

Hi,

On Fri, Jan 11, 2013 at 8:49 PM, Lukasz Majewski <l.majewski at samsung.com> wrote:
> Hi Wolfgang,
>
>> Dear Lukasz Majewski,
>>
>> In message <1357665792-8141-1-git-send-email-l.majewski at samsung.com>
>> you wrote:
>> > I'd like to ask for your opinion about the following problem:
>>
>> I cannot comment on the problem - only a bit about the proposed patch
>> ;-)
>>
>> > From a brief checking I can say that it happens when we are doing
>> > consecutive MMC operations (i.e. many reads), and the 10ms timeout
>> > might be too short when eMMC firmware is forced to do some internal
>> > time consuming operations (e.g. flash blocks management, wear
>> > leveling).
>> > In this situation, the SDHCI_CMD_INHIBIT bit is set, which means
>> > that SDHCI controller didn't received response from eMMC.
>> >
>> > One proposition would be to define the per device/per memory chip
>> > specific timeouts, to replace those defined at ./drivers/mmc/sdhci.c
>> > file.
>>
>> Is there no way to ask the device and/or controller when it is done,
>> so we can poll for ready state instead of adding delays, which will
>> always have to be tailored for the so far known worst case, i. e. they
>> will be always too long on all almost all systems.
>
> We are doing this already - the SDHCI_PRESENT_STATE register's bit 0
> (SDHCI_CMD_INHIBIT) and bit 1 (DATA_INHIBIT) are for this purpose.
> Those indicate when host controller can send further command/data to
> the card.
>
> Moreover, there are also timeouts in the case when someone pull out SD
> card inserted to the slot (or any other use case which I'm not aware).
>
>
>>
>> > --- a/drivers/mmc/sdhci.c
>> > +++ b/drivers/mmc/sdhci.c
>> > @@ -137,8 +137,8 @@ int sdhci_send_command(struct mmc *mmc, struct
>> > mmc_cmd *cmd, unsigned int timeout, start_addr = 0;
>> >     unsigned int retry = 10000;
>> >
>> > -   /* Wait max 10 ms */
>> > -   timeout = 10;
>> > +   /* Wait max 100 ms */
>> > +   timeout = 100;
>>
>> We have cases where we struggle for sub-second boot times.  Adding
>> 100 ms delay here is clearly prohbitive.  [Even the 10 ms are way too
>> long IMHO.]  There must be a better way to handle this.
>
> That's why I'm asking.
>
> It is strange that, when I'm increasing delay it works.
>
> Maybe we will find some areas of optimization?

BTW: I am also finding the similar issue.

But when I enabled CONFIG_MMC_TRACE for log traces, i never see the
issue..it's pretty much working fine.
As per my latest debug, the issue is fire for CMD6 (SWITCH_FUNC).

May be we need to update the logic on this while loop...

Thanks,
Jagan.

>
>>
>> Best regards,
>>
>> Wolfgang Denk
>>
>
>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Poland (SRPOL) | Linux Platform Group
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot