[PATCH] firmware: xilinx: Do not call IOCTL_SET_SD_TAPDELAY for value 0

[Potthuri, Sai Krishna]

Hi,

> -----Original Message-----
> From: Marek Vasut <marex at denx.de>
> Sent: Tuesday, October 18, 2022 3:35 PM
> To: Potthuri, Sai Krishna <sai.krishna.potthuri at amd.com>; Simek, Michal
> <michal.simek at amd.com>; linux-arm-kernel at lists.infradead.org
> Cc: Abhyuday Godhasara <abhyuday.godhasara at xilinx.com>; Harsha
> <harsha.harsha at xilinx.com>; Rajan Vaja <rajan.vaja at xilinx.com>; Ronak
> Jain <ronak.jain at xilinx.com>; Tanmay Shah <tanmay.shah at xilinx.com>
> Subject: Re: [PATCH] firmware: xilinx: Do not call IOCTL_SET_SD_TAPDELAY
> for value 0
> 
> On 10/18/22 12:01, Potthuri, Sai Krishna wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Marek Vasut <marex at denx.de>
> >> Sent: Tuesday, October 18, 2022 2:37 PM
> >> To: Potthuri, Sai Krishna <sai.krishna.potthuri at amd.com>; Simek,
> >> Michal <michal.simek at amd.com>; linux-arm-kernel at lists.infradead.org
> >> Cc: Abhyuday Godhasara <abhyuday.godhasara at xilinx.com>; Harsha
> >> <harsha.harsha at xilinx.com>; Rajan Vaja <rajan.vaja at xilinx.com>; Ronak
> >> Jain <ronak.jain at xilinx.com>; Tanmay Shah <tanmay.shah at xilinx.com>
> >> Subject: Re: [PATCH] firmware: xilinx: Do not call
> >> IOCTL_SET_SD_TAPDELAY for value 0
> >>
> >> On 10/18/22 09:07, Potthuri, Sai Krishna wrote:
> >>> Hi,
> >>
> >> Hi,
> >>
> >>>>>> On 10/17/22 14:35, Marek Vasut wrote:
> >>>>>>> In case the tap delay required by Arasan SDHCI is set to 0, the
> >>>>>>> current embeddedsw firmware unconditionally writes IOU_SLCR
> >>>>>> SD_ITAPDLY
> >>>>>>> to 0x100 (SD0_ITAPDLYENA=1, SD0_ITAPDLYSEL=0). Previous
> behavior
> >>>> was
> >>>>>>> to keep the IOU_SLCR SD_ITAPDLY set to 0x0. There is some sort
> >>>>>>> of difference in the behavior between SD0_ITAPDLYENA=1/0 with
> >>>>>>> the same SD0_ITAPDLYSEL=0, even though the behavior should be
> >>>>>>> identical -- zero delay added to rxclk_in line. The former
> >>>>>>> breaks
> >>>>>>> HS200 training in low
> >>>>>> temperature conditions.
> >>>>> We got a similar issue from one of the customers saying tuning was
> >>>>> failing at lower temperatures with 4.19 kernel.
> >>>>> Root cause of this issue is incorrect tap delay sequence in 4.19
> >>>>> kernel which got fixed in 5.4 Xilinx Linux tree (2022.2 release).
> >>>>
> >>>> I am not using the Xilinx downstream stuff, I am using linux-next
> >>>> and Linux 5.10.y from linux-stable for these tests.
> >>>>
> >>>>> 4.19 sequence:
> >>>>> DLL assert->ITAP->DLL de-assert->DLL assert->OTAP->DLL de-assert
> >>>>> 5.4 sequence:
> >>>>> DLL assert->ITAP->OTAP->DLL de-assert
> >>>>>
> >>>>> Please give a try with the updated sequence.
> >>>>
> >>>> Whatever fixes are in Linux 5.4 should already be present, and no,
> >>>> that does NOT work.
> >>> This fix has dependency on ATF version, are you testing with >=2.5
> version?
> >>> https://github.com/ARM-software/arm-trusted-firmware/tree/v2.5
> >>
> >> No, I am still running whatever downstream fork of ATF came with the
> >> hardware and this cannot be updated.
> >>
> >> Can you point me to specific commit(s) in the aforementioned
> >> repository which are related to this topic ?
> > ATF change I am talking about is
> > https://github.com/ARM-software/arm-trusted-
> firmware/commit/2ab0ef8db9
> > 561699fef0f77f5a1735e4903f6b3e
> >
> > Looks like we are already taking care of disabling the ITAP in
> > ATF(below commit) if we get ZERO tap.
> > https://github.com/ARM-software/arm-trusted-
> firmware/commit/fe1fa205fc
> > a4d1dd4a1b1755942956dbca65d573
> >
> > Above changes are part of ATF 2.5 version.
> 
> So what's the fix for systems which run older version of the firmware and
> which also need to be fixed ?
> 
> The ATF change seems unrelated to SD0_ITAPDLYENA=1/0 toggling, right ?
> So how can that fix the problem ? Why does the system fail calibration when
> SD0_ITAPDLYENA=1 and pass with SD0_ITAPDLYENA=0 ?
https://github.com/ARM-software/arm-trusted-firmware/commit/fe1fa205fca4d1dd4a1b1755942956dbca65d573
This commit does what ever this patch is trying to achieve. If my understanding is correct, you want SD0_ITAPDLYENA to be 0 if user tries to write 0 to SD0_ITAPDLYSEL. This commit does the same by writing 0 to ITAP register if ITAPSEL is 0.

This is the recommendation from IP designers to clear the external controls (SDx_ITAPDLYENA should be reset) for auto-tuning. Same mentioned in the ZynqMP TRM under "Receive Clock Tap Delay" section.

Regards
Sai Krishna