Re: [PATCH v6 5/8] mmc: sdhci-cadence: rename SD4HC symbols for SD6HC groundwork

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Mismatch between property counting and parsing logic leads to uninitialized array elements being written to PHY registers if a property is malformed.
--

commit 8f13dab23e01e84497b28e9b7e93c757eb2c1d11
Author: Tanmay Kathpalia [off-list ref]

mmc: sdhci-cadence: rename SD4HC symbols for SD6HC groundwork

This commit renames SD4HC PHY helpers and default operations with a cdns4
prefix to separate them from the shared driver core. This prepares the
driver for future SD6HC support by avoiding symbol clashes.

[ ... ]

-static void sdhci_cdns_phy_param_parse(struct device_node *np,
-				       struct sdhci_cdns_priv *priv)
+static void sdhci_cdns4_phy_param_parse(struct device_node *np, struct sdhci_cdns_priv *priv)
 {
-	struct sdhci_cdns_phy_param *p = priv->phy_params;
+	struct sdhci_cdns4_phy_param *p = priv->phy_params;
 	u32 val;
 	int ret, i;
 
-	for (i = 0; i < ARRAY_SIZE(sdhci_cdns_phy_cfgs); i++) {
-		ret = of_property_read_u32(np, sdhci_cdns_phy_cfgs[i].property,
-					   &val);
+	for (i = 0; i < ARRAY_SIZE(sdhci_cdns4_phy_cfgs); i++) {
+		ret = of_property_read_u32(np, sdhci_cdns4_phy_cfgs[i].property, &val);
 		if (ret)
 			continue;

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
mismatch in sdhci_cdns4_phy_param_parse between how properties are counted
and parsed.

If a device tree property is present but malformed (for example, defined as
an empty property or a boolean instead of a u32), of_property_read_u32
fails and this loop continues without advancing the p pointer.

Because sdhci_cdns4_phy_param_count previously incremented the count
simply because the property was present, priv->nr_phy_params will end up
larger than the number of elements actually parsed. This leaves the trailing
elements in the priv->phy_params array zero-initialized.

-		p->addr = sdhci_cdns_phy_cfgs[i].addr;
+		p->addr = sdhci_cdns4_phy_cfgs[i].addr;
 		p->data = val;
 		p++;
 	}
 }
 
-static int sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv)
+static int sdhci_cdns4_phy_init(struct sdhci_cdns_priv *priv)
 {
 	int ret, i;
 
 	for (i = 0; i < priv->nr_phy_params; i++) {
-		ret = sdhci_cdns_write_phy_reg(priv, priv->phy_params[i].addr,
-					       priv->phy_params[i].data);
+		ret = sdhci_cdns4_write_phy_reg(priv, priv->phy_params[i].addr,
+						priv->phy_params[i].data);

[Severity: High]
This isn't a bug introduced by this patch, but following from the parsing
mismatch above, can this loop write uninitialized array elements to the PHY
registers?

Since priv->nr_phy_params includes the count of malformed properties,
sdhci_cdns4_phy_init will iterate over the trailing zero-initialized elements
in the array.

This would inadvertently write data 0 to address 0, which corresponds to
SDHCI_CDNS_PHY_DLY_SD_HS. Could this unintentionally corrupt unrelated
hardware configurations and potentially break SD/eMMC access?

 		if (ret)
 			return ret;
 	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829184252.27644-1-tanmay.kathpalia@altera.com?part=5