[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 6:02 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:37, Potthuri, Sai Krishna wrote:
> 
> Hi,
> 
> [...]
> 
> >>>> 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/fe1fa205fc
> > a4d1dd4a1b1755942956dbca65d573 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 not helpful, since I cannot update firmware on this platform.
> A Linux-only fix for this bug is necessary. What are the options here ?
I don’t see any option where we can write directly from the Linux, as tap
related registers are in secure space, and it requires ATF interface.
Since ATF interface is broken in this case, we cannot write 0x0 to ITAPDLYENA
(also Tap delay sequence is also incorrect in this ATF version).
Only option i can see to make eMMC interface work with firmware version
you are using is fallback to High-Speed mode.

> 
> (this here is really a good example of why burying important functionality
> into firmware instead of keeping it in Linux is problematic harmful practice)
> 
> > 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.
> 
> What is the difference in behavior between:
> - SD0_ITAPDLYENA=0 SD0_ITAPDLYSEL=0
> and
> - SD0_ITAPDLYENA=1 SD0_ITAPDLYSEL=0
> ?
> 
> My understanding is that the behavior should be identical, but it is not. Why?
eMMC HS200 requires auto-tuning, as part of auto-tuning, controller calculates
the required ITAP delay and used it for further operations. As per RTL logic, if
ITAPDLYENA bit is 1 then it takes the taps from ITAP register (manual tap
programmed by user) otherwise taps will be considered from auto-tuning logic.

Regards
Sai Krishna