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

[sashiko-bot@kernel.org]

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 <tanmay.kathpalia@altera.com>

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.

> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 435603c8c00b2..8a6ef6d9d1dd8 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c

[ ... ]

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