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

[NG, TZE YEE]

On 13/9/2026 4:36 am, Kathpalia, Tanmay wrote:

On 26-08-2026 08:23, tze.yee...@altera.com wrote:

From: Tze Yee Ng <tze.yee...@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...@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.