[U-Boot] [PATCH 4/4] mmc: add workaround for eSDHC erratum A009620

[Jaehoon Chung]

Hi Yangbo,

On 07/28/2016 12:20 PM, Yangbo Lu wrote:
> Hi Jaehoon,
> 
> 
>> -----Original Message-----
>> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
>> Sent: Wednesday, July 27, 2016 7:21 PM
>> To: Yangbo Lu; york sun; u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH 4/4] mmc: add workaround for eSDHC erratum
>> A009620
>>
>> On 07/27/2016 04:10 PM, Yangbo Lu wrote:
>>>> -----Original Message-----
>>>> From: york sun
>>>> Sent: Tuesday, July 26, 2016 1:38 AM
>>>> To: Yangbo Lu; u-boot at lists.denx.de
>>>> Subject: Re: [PATCH 4/4] mmc: add workaround for eSDHC erratum
>>>> A009620
>>>>
>>>> On 05/20/2016 03:20 AM, Yangbo Lu wrote:
>>>>> Erratum Title:
>>>>> Data timeout error not getting set in case of command with busy
>>>>> response (R1b) as well as for busy period after last write block
>>>>> transfer.
>>>>>
>>>>> Description:
>>>>> In the event that a busy timeout occurs for a command with a busy
>>>>> response (e.g. R1b response) as well as busy period after the last
>>>>> write block, the eSDHC does not set the IRQSTAT[DTOE] bit or the
>>>>> IRQSTAT[TC]. Therefore, the current command transfer is never
>> completed.
>>>>>
>>>>> Workaround:
>>>>> Workaround for CMD with busy:
>>>>> Don't set the XFRTYP[RSP]=2'b11 for commands with busy response and
>>>>> poll the busy status of the card from the PRSSTAT[DLSL]
>>>>>
>>>>> Workaround for busy period after last write block:
>>>>> 1. After the command completion interrupt (IRQSTAT[CC]), wait for
>>>>>    de-assertion of PRSTAT[WTA].
>>>>> 2. Once PRSTAT[WTA] is de-asserted, start the software timer and poll
>>>>>    the busy signal (DAT0) using PRSTAT[DLSL[0]].
>>>>> 3. Wait for DAT0 signal to go high (which indicate transfer complete)
>>>>>    or software timer expiry (which indicate data timeout error).
>>>>> 4. Issue soft reset for data (SYSCTL[RSTD]).
>>>>> 5. In case of data timeout error (detected in step 3) perform the
>>>>>    error recovery.
>>>>>
>>>>> The workaround for CMD with busy has already been applied in eSDHC
>>>>> driver. This patch is to add workaround for the 2nd issue, and the
>>>>> fixup platforms include PowerPC(P1010/P2020/P5020/P5040/T1024/T1040/
>>>>> T2080/T4240) and ARM(LS1021A/LS1043A/LS2080A).
>>>>>
>>>>> Signed-off-by: Yangbo Lu <yangbo.lu at nxp.com>
>>>>> ---
>>>>>  arch/powerpc/include/asm/config_mpc85xx.h |  8 ++++++++
>>>>>  drivers/mmc/fsl_esdhc.c                   | 26
>>>> ++++++++++++++++++++++++++
>>>>>  include/configs/ls1021aqds.h              |  1 +
>>>>>  include/configs/ls1021atwr.h              |  1 +
>>>>>  include/configs/ls1043a_common.h          |  1 +
>>>>>  include/configs/ls2080a_simu.h            |  1 +
>>>>>  include/configs/ls2080aqds.h              |  1 +
>>>>>  include/configs/ls2080ardb.h              |  1 +
>>>>>  include/fsl_esdhc.h                       |  1 +
>>>>>  9 files changed, 41 insertions(+)
>>>>
>>>> Yangbo,
>>>>
>>>> Why do you put the macro CONFIG_SYS_FSL_ERRATUM_ESDHC_A009620 in
>>>> board header file if the erratum applies to the SoC?
>>>>
>>>> York
>>>>
>>> [Lu Yangbo-B47093] Thanks a lot, York.
>>> Could you suggest any better place where I can put this macro?
>>> I didn't find such a good place :(
>>
>> If use your patch by default, is there performance degression?
>> I don't know fsl_esdhc controller..so i just ask this. :)
>>
> 
> [Lu Yangbo-B47093] The original code polls 'Transfer Complete' interrupt or 'DMA' interrupt for transfer completion.
> This patch will change to poll DAT0/BUSY line. 
> There should be no difference, but I think the udelay(100) in polling DAT0/BUSY may have a little affect on performance.
> Anyway the patch will only apply to the silicon which has this erratum.

Thanks for explanation! :)
I think you can split to config files and mmc side. 

And 

while (esdhc_read32(&regs->prsstat) & PRSSTAT_WTA);

This code has the potential infinite loop..

while ((esdhc_read32(&regs->sysctl) & SYSCTL_RSTD));

Ditto. In my experience, it's good that potential infinite loop case is removed.
Because we can't sure that this condition has to hit. :)

Best Regards,
Jaehoon Chung

> 
> Thanks.
> 
>>>
>>> Thanks.
>>>
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>>>
>>>
> 
> 
> 
>