Re: [PATCH v2 0/6] Fix regulator voting for the Qualcomm SGMII SerDes PHY

From: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>
To: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Vinod Koul <vkoul@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bartosz Golaszewski <brgl@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Wasim Nazir <wasim.nazir@oss.qualcomm.com>,
	Mohd Ayaan Anwar <quic_mohdayaa@quicinc.com>,
	Umang Chheda <umang.chheda@oss.qualcomm.com>,
	Yijie Yang <quic_yijiyang@quicinc.com>
Cc: Loic Poulain <loic.poulain@oss.qualcomm.com>,
	linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/6] Fix regulator voting for the Qualcomm SGMII SerDes PHY
Date: Tue, 4 Aug 2026 14:06:30 +0530	[thread overview]
Message-ID: <anGkjjDJRRMVT7ds@oss.qualcomm.com> (raw)
In-Reply-To: <20260804-b4-sgmiieth_serdes_regulator-v2-0-c4bc688177dd@oss.qualcomm.com>

On Tue, Aug 04, 2026 at 12:57:56AM +0530, Mohd Ayaan Anwar wrote:
> This series corrects this by:
> 
>   1. Fixing the DT binding to describe both named supplies.
>   2. Fixing all affected board DTS to use the correct vdda-0p9-supply
>      and vdda-1p2-supply properties.
>   3. Adding bulk regulator support to the PHY driver so it enables both
>      supplies and sets the proper current loads.
> 
> The patches are sequenced to maintain bisectability. With the driver
> change, the PHY framework will continue to vote for the regulator
> described by 'phy-supply'. The DT changes which follow then remove
> 'phy-supply' in favour of 'vdda-0p9' and 'vdda-1p2'.
> 

Sashiko raises some valid concerns [0]:
--
1. This isn't a bug introduced by this patch, but I noticed that
another file in the tree, monaco-monza-som.dtsi, still has the exact
same configuration you are fixing here. It retains the deprecated
phy-supply property and lacks the required 0.9V rail.

Should monaco-monza-som.dtsi be updated similarly in this patch or as a
follow-up in this series?

2. 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?
--

For 1., I can either post a follow-up patch or add it as part of v3.

For 2., this is a genuine pre-existing bug. I will raise a separate
patch to move phy_set_drvdata() before devm_of_phy_provider_register()
to close the race window.

	Ayaan

[0] https://sashiko.dev/#/patchset/20260804-b4-sgmiieth_serdes_regulator-v2-0-c4bc688177dd@oss.qualcomm.com

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy