Re: [PATCH v2 2/6] phy: qcom: sgmii-eth: vote for both voltage rails with correct current loads

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The PHY provider is registered before its driver data is set, creating a race condition where a consumer could trigger a NULL pointer dereference.
--

--- Patch [2]: [PATCH v2 2/6] phy: qcom: sgmii-eth: vote for both voltage rails with correct current loads ---
commit 1203fb5a9ba082470a0383befe64f62c484ac7d5
Author: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

phy: qcom: sgmii-eth: vote for both voltage rails with correct current loads

This commit updates the driver to explicitly vote for both voltage supply
rails with their correct peak current loads using
devm_regulator_bulk_get_const(), rather than relying on the PHY framework's
implicit single-rail voting. It also adds the necessary regulator
enable/disable calls in the power path.

> diff --git a/drivers/phy/qualcomm/phy-qcom-sgmii-eth.c b/drivers/phy/qualcomm/phy-qcom-sgmii-eth.c
> index f48faa2929a68..7137200c3f310 100644
> --- a/drivers/phy/qualcomm/phy-qcom-sgmii-eth.c
> +++ b/drivers/phy/qualcomm/phy-qcom-sgmii-eth.c

[ ... ]

> @@ -381,6 +404,12 @@ static int qcom_dwmac_sgmii_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(data->refclk))
>  		return PTR_ERR(data->refclk);
>  
> +	ret = devm_regulator_bulk_get_const(dev, QCOM_SGMII_NUM_SUPPLIES,
> +					    qcom_dwmac_sgmii_phy_vregs,
> +					    &data->vregs);
> +	if (ret)
> +		return ret;
> +
>  	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>  	if (IS_ERR(provider))
>  		return PTR_ERR(provider);

[Severity: High]
This is a pre-existing issue, but does registering the PHY provider before
setting the driver data create a race condition here?

Looking at the end of qcom_dwmac_sgmii_phy_probe():

	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
	if (IS_ERR(provider))
		return PTR_ERR(provider);

	phy_set_drvdata(phy, data);

By calling devm_of_phy_provider_register() first, the PHY is made available
to consumers before phy_set_drvdata() initializes the private data.

If a consumer (such as a deferred probe workqueue) concurrently looks up the
PHY and immediately invokes an operation like phy_power_on(), could
qcom_dwmac_sgmii_phy_power_on() read a NULL driver data pointer and cause
a NULL pointer dereference when attempting to access data->vregs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804-b4-sgmiieth_serdes_regulator-v2-0-c4bc688177dd@oss.qualcomm.com?part=2