Re: [PATCH 2/4] mmc: sdhci-cadence6: name PHY and controller config table slots

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

On 13/9/2026 4:36 am, Kathpalia, Tanmay wrote:
> 
> On 26-08-2026 08:23, tze.yee.ng@altera.com wrote:
>> From: Tze Yee Ng <tze.yee.ng@altera.com>
>>
>> sdhci_cdns6_phy_adj() programs the PHY and HRS controller registers by
>> indexing the *_phy_cfgs[] and *_ctrl_cfgs[] tables with bare positional
>> constants. This is fragile: reordering or inserting a table entry
>> silently changes which register each slot programs.
>>
>> Introduce enums that name every PHY and controller configuration slot,
>> whose terminators double as the table sizes (SDHCI_CDNS6_PHY_CFG_NUM and
>> SDHCI_CDNS6_CTRL_CFG_NUM), and use those names when programming the
>> registers. No functional change.
>>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>> ---
>>   drivers/mmc/sdhci-cadence6.c | 54 ++++++++++++++++++++++++++++--------
>>   1 file changed, 43 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci-cadence6.c b/drivers/mmc/sdhci-cadence6.c
>> index 1ff974ecd44..7bd47b462a4 100644
>> --- a/drivers/mmc/sdhci-cadence6.c
>> +++ b/drivers/mmc/sdhci-cadence6.c
>> @@ -59,8 +59,6 @@
>>   #define PHY_DLL_SLAVE_CTRL_REG_READ_DQS_CMD_DELAY    GENMASK(31, 24)
>>   #define PHY_DLL_SLAVE_CTRL_REG_READ_DQS_DELAY        GENMASK(7, 0)
>> -#define SDHCI_CDNS6_PHY_CFG_NUM        5
>> -#define SDHCI_CDNS6_CTRL_CFG_NUM    4
> 
> Dropping the two size macros leaves a double blank line.

Hi Tanmay,

I will drop the blank line in v2.

Thanks,
Tze Yee

> 
>>   struct sdhci_cdns6_phy_cfg {
>>       const char *property;
>> @@ -72,6 +70,33 @@ struct sdhci_cdns6_ctrl_cfg {
>>       u32 val;
>>   };
>> +/*
>> + * PHY configuration slots. The enumeration order must match the entry
>> + * order in every *_phy_cfgs[] table; SDHCI_CDNS6_PHY_CFG_NUM tracks the
>> + * table size so the two never drift out of sync.
>> + */
>> +enum {
>> +    SDHCI_CDNS6_PHY_CFG_DQS_TIMING,
>> +    SDHCI_CDNS6_PHY_CFG_GATE_LPBK_CTRL,
>> +    SDHCI_CDNS6_PHY_CFG_DLL_SLAVE_CTRL,
>> +    SDHCI_CDNS6_PHY_CFG_DQ_TIMING,
>> +    SDHCI_CDNS6_PHY_CFG_DLL_MASTER_CTRL,
>> +    SDHCI_CDNS6_PHY_CFG_NUM,
>> +};
>> +
>> +/*
>> + * Controller (HRS) configuration slots. The enumeration order must 
>> match
>> + * the entry order in every *_ctrl_cfgs[] table; 
>> SDHCI_CDNS6_CTRL_CFG_NUM
>> + * tracks the table size so the two never drift out of sync.
>> + */
>> +enum {
>> +    SDHCI_CDNS6_CTRL_CFG_HRS09,
>> +    SDHCI_CDNS6_CTRL_CFG_HRS10,
>> +    SDHCI_CDNS6_CTRL_CFG_HRS16,
>> +    SDHCI_CDNS6_CTRL_CFG_HRS07,
>> +    SDHCI_CDNS6_CTRL_CFG_NUM,
>> +};
>> +
>>   static const struct sdhci_cdns6_phy_cfg sd_ds_phy_cfgs[] = {
>>       { "cdns,phy-dqs-timing-delay-sd-ds", 0x00380004, },
>>       { "cdns,phy-gate-lpbk-ctrl-delay-sd-ds", 0x01A00040, },
>> @@ -277,10 +302,14 @@ int sdhci_cdns6_phy_adj(struct udevice *dev, 
>> struct sdhci_cdns_plat *plat, u32 m
>>       /* Switch On the DLL Reset */
>>       sdhci_cdns6_reset_phy_dll(plat, true);
>> -    sdhci_cdns6_write_phy_reg(plat, PHY_DQS_TIMING_REG_ADDR, 
>> phy_cfgs[0].val);
>> -    sdhci_cdns6_write_phy_reg(plat, PHY_GATE_LPBK_CTRL_REG_ADDR, 
>> phy_cfgs[1].val);
>> -    sdhci_cdns6_write_phy_reg(plat, PHY_DLL_MASTER_CTRL_REG_ADDR, 
>> phy_cfgs[4].val);
>> -    sdhci_cdns6_write_phy_reg(plat, PHY_DLL_SLAVE_CTRL_REG_ADDR, 
>> phy_cfgs[2].val);
>> +    sdhci_cdns6_write_phy_reg(plat, PHY_DQS_TIMING_REG_ADDR,
>> +                  phy_cfgs[SDHCI_CDNS6_PHY_CFG_DQS_TIMING].val);
>> +    sdhci_cdns6_write_phy_reg(plat, PHY_GATE_LPBK_CTRL_REG_ADDR,
>> +                  phy_cfgs[SDHCI_CDNS6_PHY_CFG_GATE_LPBK_CTRL].val);
>> +    sdhci_cdns6_write_phy_reg(plat, PHY_DLL_MASTER_CTRL_REG_ADDR,
>> +                  phy_cfgs[SDHCI_CDNS6_PHY_CFG_DLL_MASTER_CTRL].val);
>> +    sdhci_cdns6_write_phy_reg(plat, PHY_DLL_SLAVE_CTRL_REG_ADDR,
>> +                  phy_cfgs[SDHCI_CDNS6_PHY_CFG_DLL_SLAVE_CTRL].val);
>>       /* Switch Off the DLL Reset */
>>       ret = sdhci_cdns6_reset_phy_dll(plat, false);
>> @@ -290,7 +319,8 @@ int sdhci_cdns6_phy_adj(struct udevice *dev, 
>> struct sdhci_cdns_plat *plat, u32 m
>>       }
>>       /* Set PHY DQ TIMING control register */
>> -    sdhci_cdns6_write_phy_reg(plat, PHY_DQ_TIMING_REG_ADDR, 
>> phy_cfgs[3].val);
>> +    sdhci_cdns6_write_phy_reg(plat, PHY_DQ_TIMING_REG_ADDR,
>> +                  phy_cfgs[SDHCI_CDNS6_PHY_CFG_DQ_TIMING].val);
>>       /* Set HRS09 register */
>>       tmp = readl(plat->hrs_addr + SDHCI_CDNS_HRS09);
>> @@ -298,20 +328,22 @@ int sdhci_cdns6_phy_adj(struct udevice *dev, 
>> struct sdhci_cdns_plat *plat, u32 m
>>            SDHCI_CDNS_HRS09_EXTENDED_RD_MODE |
>>            SDHCI_CDNS_HRS09_RDDATA_EN |
>>            SDHCI_CDNS_HRS09_RDCMD_EN);
>> -    tmp |= ctrl_cfgs[0].val;
>> +    tmp |= ctrl_cfgs[SDHCI_CDNS6_CTRL_CFG_HRS09].val;
>>       writel(tmp, plat->hrs_addr + SDHCI_CDNS_HRS09);
>>       /* Set HRS10 register */
>>       tmp = readl(plat->hrs_addr + SDHCI_CDNS_HRS10);
>>       tmp &= ~SDHCI_CDNS_HRS10_HCSDCLKADJ;
>> -    tmp |= ctrl_cfgs[1].val;
>> +    tmp |= ctrl_cfgs[SDHCI_CDNS6_CTRL_CFG_HRS10].val;
>>       writel(tmp, plat->hrs_addr + SDHCI_CDNS_HRS10);
>>       /* Set HRS16 register */
>> -    writel(ctrl_cfgs[2].val, plat->hrs_addr + SDHCI_CDNS_HRS16);
>> +    writel(ctrl_cfgs[SDHCI_CDNS6_CTRL_CFG_HRS16].val,
>> +           plat->hrs_addr + SDHCI_CDNS_HRS16);
>>       /* Set HRS07 register */
>> -    writel(ctrl_cfgs[3].val, plat->hrs_addr + SDHCI_CDNS_HRS07);
>> +    writel(ctrl_cfgs[SDHCI_CDNS6_CTRL_CFG_HRS07].val,
>> +           plat->hrs_addr + SDHCI_CDNS_HRS07);
>>       /*
>>        * Wait for the PHY/DLL to settle before the first data transfer.