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

[Yangbo Lu]

Hi Jaehoon,

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Sent: Thursday, July 28, 2016 1:01 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
> 
> 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.

[Lu Yangbo-B47093] Ok, this should be no problem :)

> 
> 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. :)
> 

[Lu Yangbo-B47093] I think you're right. Although this would be ok from hardware perspective, there is still potential risk.
But how to remove them, is there any good idea? Is it proper to add timeout checking here ?

Thanks.

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