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

[Marek Vasut]

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/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 not helpful, since I cannot update firmware on this platform.
A Linux-only fix for this bug is necessary. What are the options here ?

(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?

[...]