Re: [PATCH 3/4] mmc: sdhci-cadence6: configure PHY control register per speed mode

["NG, TZE YEE" <tze.yee.ng@altera.com>]

On 4/10/2026 2:53 am, Kathpalia, Tanmay wrote:
> 
> On 21-09-2026 09:20, NG, TZE YEE wrote:
>>
>>
>> On 13/9/2026 5:06 am, Kathpalia, Tanmay wrote:
>>> Hi Tze,
>>>
>>> On 26-08-2026 08:23, tze.yee.ng@altera.com wrote:
>>>> From: Tze Yee Ng <tze.yee.ng@altera.com>
>>>>
>>>> The PHY control register (0x2080) holds phony_dqs_timing (bits [9:4]),
>>>> which shifts the read-capture strobe relative to the data slices.
>>>>
>>>> Extend the per-mode PHY configuration arrays with a "cdns,phy-ctrl-*"
>>>> entry (default 0x41F0) and program register 0x2080 during PHY
>>>> adjustment. This lets each speed mode - and each board via device 
>>>> tree -
>>>> select an appropriate value.
>>>>
>>>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>>>> ---
>>>
>>> The PHY_CTRL default needs to be corrected. All six tables also use the
>>> same 0x41F0, so this is not actually per-mode.
>>>
>>> Please fix the defaults.
>>>
>>> Regards,
>>> Tanmay
>>
>> Hi Tanmay,
>>
>> 0x41F0 is the PHY_CTRL (0x2080) reset value from the PHY spec. The 
>> driver never wrote this register before, so the PHY already ran at 
>> 0x41F0 in every mode. Using it as the default in all six tables just 
>> preserves that (the write is a no-op unless overridden), while making 
>> the register tunable per mode/board via cdns,phy-ctrl-<mode> in DT, 
>> e.g. on Agilex5:
>>
>>   013b:    cdns,phy-ctrl-emmc-ddr = <0x4010>; (phony_dqs_timing = 1)
>>   modular: cdns,phy-ctrl-emmc-ddr = <0x4000>; (phony_dqs_timing = 0)
>>
>>
>> So the defaults match only because the reset value is mode-independent;
>> the real tuning lives in DT. You're right that six bare 0x41F0 
>> literals are misleading. For v3 I'll add a named constant 
>> (SDHCI_CDNS6_PHY_CTRL_REG_DEFAULT = 0x000041F0) for the default and 
>> reword the commit to say the register is made programmable with the 
>> reset value as default (no functional change), overridable per mode/ 
>> board via DT.
>>
>> Does that work for you, or did you expect distinct per-mode defaults? I
>> don't have validated per-mode values, that tuning is meant to live in DT.
>>
>>
> 
> I expect mode-specific, validated defaults, consistent with the other
> PHY configuration values in each mode table.
> Please determine the appropriate default for each speed mode rather
> than using 0x41F0 universally. Board-specific DT values may still
> override those defaults where required.

Hi Tanmay,

For v3, I'll drop the universal 0x41F0 and the per-mode cdns,phy-ctrl 
table entirely. The PHY_CTRL reset value already works for the SD/eMMC
SDR and HS modes, so there are no per-mode defaults to validate there,
the driver simply leaves PHY_CTRL untouched in those modes.

Only the DDR read path is sensitive to phony_dqs_timing, so the driver
programs it just for DDR50/DDR52 via a single mode-neutral property,
cdns,ddr-phony-dqs-timing (shared by SD DDR50 and eMMC DDR52). It
defaults to 0 (the neutral assertion timing) and a board whose DDR read
path needs tuning sets it explicitly: SoCDK, eMMC and 013B use 1, while
the modular SoM's longer SD path uses the default 0.

The binding goes through Linux (its canonical home), currently under
review:
https://lore.kernel.org/all/25649477d9967bb04b12c6e60b0ee76443f356af.1790074790.git.tze.yee.ng@altera.com/
I've adopted the cdns,ddr-* naming you recommended there, so the U-Boot
property matches the binding.

Let me know if you see it differently.

Thanks,
Tze Yee