linux-phy.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 4/5] phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot
From: Loic Poulain @ 2026-07-21 13:05 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
  Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa,
	Dmitry Baryshkov, Loic Poulain
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com>

Runtime PM has to be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHY is not yet registered.

Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHY has been created to
prevent the device from being runtime suspended during that window.

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index fc490589c8e4888770807a60170905a1c7610421..eb1826fb26f0868f7758245b0cbe1a0590fea9a3 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -1285,10 +1285,16 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * Enable runtime PM before creating the PHY, phy_create() only enables
+	 * it on the PHY device if already enabled on the parent. Hold a usage
+	 * reference so callbacks cannot run before qmp->phy is assigned.
+	 */
+	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
-		goto err_node_put;
+		goto err_pm_put;
 	/*
 	 * Prevent runtime pm from being ON by default. Users can enable
 	 * it using power/control in sysfs.
@@ -1297,23 +1303,27 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
 
 	ret = phy_pipe_clk_register(qmp, np);
 	if (ret)
-		goto err_node_put;
+		goto err_pm_put;
 
 	qmp->phy = devm_phy_create(dev, np, &qmp_usb_legacy_phy_ops);
 	if (IS_ERR(qmp->phy)) {
 		ret = PTR_ERR(qmp->phy);
 		dev_err(dev, "failed to create PHY: %d\n", ret);
-		goto err_node_put;
+		goto err_pm_put;
 	}
 
 	phy_set_drvdata(qmp->phy, qmp);
 
+	pm_runtime_put(dev);
+
 	of_node_put(np);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 
+err_pm_put:
+	pm_runtime_put_noidle(dev);
 err_node_put:
 	of_node_put(np);
 	return ret;

-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v5 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend
From: Loic Poulain @ 2026-07-21 13:05 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
  Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
	Konrad Dybcio, Abel Vesa, Loic Poulain
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com>

Runtime PM must be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. However, the runtime PM callbacks dereference the
hsphy instance, which is not yet ready, leaving a window where a suspend
callback may trigger a NULL pointer dereference.

Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHY has been created, so that
no runtime suspend can run before the PHY is ready. This also prevents a
short window where an unnecessary runtime suspend can occur.

Use the devres-managed version to ensure PM runtime is symmetrically
disabled during driver removal for proper cleanup.

Fixes: 0d75f508a9d5 ("phy: qcom-snps: Add runtime suspend and resume handlers")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
index eb0b0f61d98e03963bf92e5c822334f52636abe7..b1a85518fed0a7a35516d16da3ebf69afe760e40 100644
--- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
+++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
@@ -599,8 +599,18 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret,
 				     "failed to get regulator supplies\n");
 
+	/*
+	 * Enable runtime PM before creating the PHY, phy_create() only enables
+	 * it on the PHY device if already enabled on the parent. Hold a usage
+	 * reference so callbacks cannot run before the PHY is ready.
+	 */
+	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret) {
+		pm_runtime_put_noidle(dev);
+		return ret;
+	}
 	/*
 	 * Prevent runtime pm from being ON by default. Users can enable
 	 * it using power/control in sysfs.
@@ -611,6 +621,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 	if (IS_ERR(generic_phy)) {
 		ret = PTR_ERR(generic_phy);
 		dev_err(dev, "failed to create phy, %d\n", ret);
+		pm_runtime_put_noidle(dev);
 		return ret;
 	}
 	hsphy->phy = generic_phy;
@@ -622,8 +633,8 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 	if (!IS_ERR(phy_provider))
 		dev_dbg(dev, "Registered Qcom-SNPS HS phy\n");
-	else
-		pm_runtime_disable(dev);
+
+	pm_runtime_put(dev);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 }

-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v5 3/5] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
From: Loic Poulain @ 2026-07-21 13:05 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
  Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa,
	Dmitry Baryshkov, Loic Poulain
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com>

There is a small window where the runtime suspend callback may run
after pm_runtime_enable() and before pm_runtime_forbid(). In this
case, a crash occurs because runtime suspend/resume dereferences
qmp->phy pointer, which is not yet initialized:
        `if (!qmp->phy->init_count) {`

This can also happen if user re-enables runtime-pm via the sysfs
attribute before qmp phy is initialized.

Similarly to other qcom phy drivers, introduce a qmp->phy_initialized
variable that can be used to avoid relying on the possibly uninitialized
phy pointer.

Fixes: e464a3180a43 ("phy: qcom-qmp-usb: split off the legacy USB+dp_com support")
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index 8bf951b0490cfd811635df8940de1b789e21b46c..fc490589c8e4888770807a60170905a1c7610421 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -542,6 +542,8 @@ struct qmp_usb {
 
 	enum phy_mode mode;
 
+	bool phy_initialized;
+
 	struct phy *phy;
 
 	struct clk_fixed_rate pipe_clk_fixed;
@@ -895,6 +897,7 @@ static int qmp_usb_legacy_power_off(struct phy *phy)
 
 static int qmp_usb_legacy_enable(struct phy *phy)
 {
+	struct qmp_usb *qmp = phy_get_drvdata(phy);
 	int ret;
 
 	ret = qmp_usb_legacy_init(phy);
@@ -904,14 +907,19 @@ static int qmp_usb_legacy_enable(struct phy *phy)
 	ret = qmp_usb_legacy_power_on(phy);
 	if (ret)
 		qmp_usb_legacy_exit(phy);
+	else
+		qmp->phy_initialized = true;
 
 	return ret;
 }
 
 static int qmp_usb_legacy_disable(struct phy *phy)
 {
+	struct qmp_usb *qmp = phy_get_drvdata(phy);
 	int ret;
 
+	qmp->phy_initialized = false;
+
 	ret = qmp_usb_legacy_power_off(phy);
 	if (ret)
 		return ret;
@@ -988,7 +996,7 @@ static int __maybe_unused qmp_usb_legacy_runtime_suspend(struct device *dev)
 
 	dev_vdbg(dev, "Suspending QMP phy, mode:%d\n", qmp->mode);
 
-	if (!qmp->phy->init_count) {
+	if (!qmp->phy_initialized) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");
 		return 0;
 	}
@@ -1009,7 +1017,7 @@ static int __maybe_unused qmp_usb_legacy_runtime_resume(struct device *dev)
 
 	dev_vdbg(dev, "Resuming QMP phy, mode:%d\n", qmp->mode);
 
-	if (!qmp->phy->init_count) {
+	if (!qmp->phy_initialized) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");
 		return 0;
 	}

-- 
2.34.1


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

^ permalink raw reply related

* Re: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
From: sashiko-bot @ 2026-07-21 13:13 UTC (permalink / raw)
  To: Loic Poulain; +Cc: vkoul, linux-phy, neil.armstrong, olteanv
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-2-a181e2adbd2d@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Asynchronous pm_runtime_put() before devm_of_phy_provider_register() can cause an async suspend race if registration fails.
--

--- Patch [2]: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot ---
commit 4e67cff2d4e3ab76976662b7c3a8f55faa25f2bd
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

This commit takes a runtime PM usage reference with pm_runtime_get_noresume()
before enabling runtime PM. It then releases the reference once the PHYs
have been created to prevent the device from being runtime suspended during
the creation window.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index ab3055bb5b0c1..4317224070fd8 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c

[ ... ]

> @@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  		if (IS_ERR(qmp->dp_phy)) {
>  			ret = PTR_ERR(qmp->dp_phy);
>  			dev_err(dev, "failed to create PHY: %d\n", ret);
> -			goto err_node_put;
> +			goto err_pm_put;
>  		}
>  		phy_set_drvdata(qmp->dp_phy, qmp);
>  	}
>  
> +	pm_runtime_put(dev);
> +
>  	of_node_put(np);
>  
>  	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>  
>  	return PTR_ERR_OR_ZERO(phy_provider);

[Severity: High]
Since pm_runtime_put() queues an asynchronous idle notification, is there a
race condition here if devm_of_phy_provider_register() fails?

If devm_of_phy_provider_register() returns an error, the function returns
immediately and triggers devm unwinding. This unwinding will call
pm_runtime_disable() due to the earlier devm_pm_runtime_enable() call.

If pm_runtime_disable() cancels the pending asynchronous idle/suspend work,
could the hardware be left in an incorrect active power state?

Would it be safer to move devm_of_phy_provider_register() before
pm_runtime_put(dev), and use the err_pm_put label (which safely uses the
synchronous pm_runtime_put_noidle()) if it fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com?part=2

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

^ permalink raw reply

* Re: [PATCH v5 3/5] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
From: sashiko-bot @ 2026-07-21 13:16 UTC (permalink / raw)
  To: Loic Poulain; +Cc: linux-phy, vkoul, olteanv, neil.armstrong
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-3-a181e2adbd2d@oss.qualcomm.com>

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

Pre-existing issues:
- [High] Incomplete Fix / Unpatched Vulnerability: The identical NULL pointer dereference fixed by this patch in `phy-qcom-qmp-usb-legacy.c` remains unpatched in `phy-qcom-qmp-usb.c`.
--

--- Patch [3]: [PATCH v5 3/5] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend ---
commit 10908699bb99258db58b260c0d16ab93fd29f9d2
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend

This commit introduces a phy_initialized flag to prevent a NULL pointer
dereference in the runtime suspend and resume callbacks. This occurs because
pm_runtime_enable() is called before the phy pointer is fully initialized.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> index 8bf951b0490cf..fc490589c8e48 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
[ ... ]
> @@ -988,7 +996,7 @@ static int __maybe_unused qmp_usb_legacy_runtime_suspend(struct device *dev)
>  
>  	dev_vdbg(dev, "Suspending QMP phy, mode:%d\n", qmp->mode);
>  
> -	if (!qmp->phy->init_count) {
> +	if (!qmp->phy_initialized) {

[Severity: High]
This is a pre-existing issue, but does the same vulnerability still exist
in the sibling driver phy-qcom-qmp-usb.c?

Looking at qmp_usb_probe() in drivers/phy/qualcomm/phy-qcom-qmp-usb.c,
devm_pm_runtime_enable(dev) is called before qmp->phy is allocated via
devm_phy_create(). Similar to the legacy driver, there appears to be a
window where early runtime PM suspend (or unprivileged sysfs access) can
trigger qmp_usb_runtime_suspend() and crash:

drivers/phy/qualcomm/phy-qcom-qmp-usb.c:qmp_usb_runtime_suspend() {
        ...
        if (!qmp->phy->init_count) {
                dev_vdbg(dev, "PHY not initialized, bailing out\n");
                return 0;
        }
        ...
}

Should this fix be extended, or a second patch added, to introduce a similar
phy_initialized flag for qmp_usb_runtime_suspend() and
qmp_usb_runtime_resume() in phy-qcom-qmp-usb.c?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-qcom-usb-phy-fix-null-v5-0-a181e2adbd2d@oss.qualcomm.com?part=3

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

^ permalink raw reply

* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-21 13:19 UTC (permalink / raw)
  To: Bryan O'Donoghue, Vladimir Zapolskiy, Vinod Koul,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong
  Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel,
	Krzysztof Kozlowski
In-Reply-To: <ec3d6035-d4de-4e38-8c72-b29a04b263ae@linaro.org>

On 21/07/2026 12:24, Bryan O'Donoghue wrote:
> On 21/07/2026 12:05, Vladimir Zapolskiy wrote:
>> On 7/21/26 13:47, Bryan O'Donoghue wrote:
>>> On 21/07/2026 10:56, Vladimir Zapolskiy wrote:
>>>> On 7/21/26 12:39, Bryan O'Donoghue wrote:
>>>>> On 21/07/2026 09:17, Vladimir Zapolskiy wrote:
>>>>>>> +  "#phy-cells":
>>>>>>> +    const: 1
>>>>>>> +    description:
>>>>>>> +      The single cell specifies the PHY operating mode.
>>>>>> This has been reported before, #phy-cells shall be 0, the operation
>>>>>> mode is selected by bus-type of endpoints as it is described in
>>>>>> video-interfaces.yaml
>>>>>>
>>>>>> Two competing values are redundant and it opens a door to a wrong
>>>>>> hardware description, the technical discussion has not been started
>>>>>> yet.
>>>>> So as I said before, I'm following Rob Herring's input on this and
>>>>> Krzysztof has given RB for this binding. At this stage, I don't see
>>>>> scope to change it.
>>>>>
>>>>> Adding endpoints was already a compromise to address your feedback,
>>>>> which I hoped would bring you on board with the design.
>>>> That's my comment dated March 27, 2026, and it says moving phy type
>>>> from phy cell to the endpoint property excludes the need to set any
>>>> phy cells:
>>>>
>>>> https://lore.kernel.org/linux-arm-msm/e37ce438-12c7-462d-
>>>> b2bc-1351be62b806@linaro.org/
>>>>
>>>> The technical comment is left unresponded so far since then.
>>>>
>>>> Since there is no expected phy cells to get on consumer's side, and
>>>> since there is another link between CSIPHY and CSID described in
>>>> video-interfaces.yaml, any kind of dependency of this IP on phy can
>>>> be just removed, which serves the purpose of hardware description
>>>> simplification.
>>>
>>> Rob's v1 feedback was consumer-decides-mode via the cell; Krzysztof has
>>> R-b'd the binding with cells=1.
>>
>> Unfortunately this is not a technical discussion to move forward
>> a better hardware description.
>>
>>> I'm happy to lock down this binding as-is on that basis.
>>>
>>> Are you arguing about phy-cells or are you actually arguing the entire
>>> instantiation of Qualcomm MIPI CSI2 as a standalone generic driver in
>>> drivers/phy ? Its not the same debate at all.
>>
>> So far I don't review the driver, only CSIPHY IP hardware description.
>>
>> In the current version of the hardware description one may find that
>> the exploited 'video-intefaces' interface completely covers the 'phys'
>> interface between CSIPHY and CSID, quite probably both are not needed
>> at the same time, and the simpler 'phys' can be removed without any
>> losses. But if it is kept, then its cell value is anyway redundant.
>>
>>> As the CAMSS maintainer, I'm fully against adding more inline monolithic
>>> CSIPHY init sequences. That could be represented as a separate
>>> linux-media driver but, then that implies that _all_ upstream CSIPHY
>>> drivers in drivers/phy are wrong, and I just don't accept that.
>>
>> Other two drivers are different and should be excluded as good
>> references, both of them links exactly one sensor to exactly one
>> CSI-2 decoder, it's not the case here. Also one of these two drivers
>> has no upstream users.
>>
>>> The bindings have DT maintainer RB, and the driver has Qcom engineer
>>> tested-by, review-by.
>>>
>>> Vinod, Kishon - Again, I'd request this merges as-is.
>>>
>>
> 
> The graph and phys are different layers, endpoints describe the
> media topology, phys is how CSID controls the PHY through the PHY
> framework (phy_get/configure/power_on).
> 
> Removing phys doesn't simplify the description, it removes the control
> interface which is the monolith again.
> 
> We have multiple examples of this model upstream, cdns-dphy-rx +
> ti-csi2rx in-tree, with users.
> 
> I'm still kicking this up to Vinod, Kishon, request stands.
> 
> Multiple DT and driver users are RB/Tested-by on the schema and driver,
> lots of other work is gated.
> 
> It is time to merge.
> 
> Please adjudicate.
> 
> ---
> bod

Had a brief discussion with Vlad, Neil and Vinod

Resolution:

- phys = <> retained
- phy-cells = <0>
- bus-type from the media-controller
   Already specified with camss still aligns with Rob's declaration
   "the consumer defines the mode" so I'm comfortable with this
   compromise position.
   This will still facilitate CPHY work/mode-selection via
   phy_ops->configure() with how to define the CPHY part TBD.
- data-lanes = <0 1 2 3> valid
- MAX_LANES 8 instead of MAX_DATA_LANES 7

- v14 with these changes shortly.

---
bod

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

^ permalink raw reply

* Re: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
From: Dmitry Baryshkov @ 2026-07-21 13:32 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng,
	linux-arm-msm, linux-phy, linux-kernel, Konrad Dybcio, Abel Vesa
In-Reply-To: <20260721-qcom-usb-phy-fix-null-v5-2-a181e2adbd2d@oss.qualcomm.com>

On Tue, Jul 21, 2026 at 03:05:46PM +0200, Loic Poulain wrote:
> Runtime PM has to be enabled before creating the PHYs, since phy_create()
> only enables runtime PM on the PHY devices if it is already enabled on
> this parent device. This opens a small window where the device can be
> runtime suspended after pm_runtime_enable() and before the later
> pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
> the PHYs are not yet registered.
> 
> Take a runtime PM usage reference with pm_runtime_get_noresume() before
> enabling runtime PM and release it once the PHYs have been created to
> prevent the device from being runtime suspended during that window.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> @@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_node_put;
>  
> +	/*
> +	 * Enable runtime PM before creating the PHYs, phy_create() only enables
> +	 * it on the PHY devices if already enabled on the parent. Hold a usage
> +	 * reference so callbacks cannot run until the PHY is ready.
> +	 */
> +	pm_runtime_get_noresume(dev);
>  	pm_runtime_set_active(dev);
>  	ret = devm_pm_runtime_enable(dev);
>  	if (ret)
> -		goto err_node_put;
> +		goto err_pm_put;
>  	/*
>  	 * Prevent runtime pm from being ON by default. Users can enable
>  	 * it using power/control in sysfs.
> @@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  
>  	ret = qmp_usbc_register_clocks(qmp, np);
>  	if (ret)
> -		goto err_node_put;
> +		goto err_pm_put;
>  
>  	qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
>  	if (IS_ERR(qmp->usb_phy)) {
>  		ret = PTR_ERR(qmp->usb_phy);
>  		dev_err(dev, "failed to create PHY: %d\n", ret);
> -		goto err_node_put;
> +		goto err_pm_put;
>  	}
>  
>  	phy_set_drvdata(qmp->usb_phy, qmp);
> @@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  		if (IS_ERR(qmp->dp_phy)) {
>  			ret = PTR_ERR(qmp->dp_phy);
>  			dev_err(dev, "failed to create PHY: %d\n", ret);
> -			goto err_node_put;
> +			goto err_pm_put;
>  		}
>  		phy_set_drvdata(qmp->dp_phy, qmp);
>  	}
>  
> +	pm_runtime_put(dev);

Should be _put_sync() ?

> +
>  	of_node_put(np);
>  
>  	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>  
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  
> +err_pm_put:
> +	pm_runtime_put_noidle(dev);

Why is it _noidle()?

>  err_node_put:
>  	of_node_put(np);
>  	return ret;
> 
> -- 
> 2.34.1
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-07-21 13:35 UTC (permalink / raw)
  To: Bryan O'Donoghue, Bryan O'Donoghue, Vinod Koul,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong
  Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel,
	Krzysztof Kozlowski
In-Reply-To: <1f128571-ed46-4a60-9594-74fdd5f440d0@kernel.org>

On 7/21/26 16:19, Bryan O'Donoghue wrote:
> On 21/07/2026 12:24, Bryan O'Donoghue wrote:
>> On 21/07/2026 12:05, Vladimir Zapolskiy wrote:
>>> On 7/21/26 13:47, Bryan O'Donoghue wrote:
>>>> On 21/07/2026 10:56, Vladimir Zapolskiy wrote:
>>>>> On 7/21/26 12:39, Bryan O'Donoghue wrote:
>>>>>> On 21/07/2026 09:17, Vladimir Zapolskiy wrote:
>>>>>>>> +  "#phy-cells":
>>>>>>>> +    const: 1
>>>>>>>> +    description:
>>>>>>>> +      The single cell specifies the PHY operating mode.
>>>>>>> This has been reported before, #phy-cells shall be 0, the operation
>>>>>>> mode is selected by bus-type of endpoints as it is described in
>>>>>>> video-interfaces.yaml
>>>>>>>
>>>>>>> Two competing values are redundant and it opens a door to a wrong
>>>>>>> hardware description, the technical discussion has not been started
>>>>>>> yet.
>>>>>> So as I said before, I'm following Rob Herring's input on this and
>>>>>> Krzysztof has given RB for this binding. At this stage, I don't see
>>>>>> scope to change it.
>>>>>>
>>>>>> Adding endpoints was already a compromise to address your feedback,
>>>>>> which I hoped would bring you on board with the design.
>>>>> That's my comment dated March 27, 2026, and it says moving phy type
>>>>> from phy cell to the endpoint property excludes the need to set any
>>>>> phy cells:
>>>>>
>>>>> https://lore.kernel.org/linux-arm-msm/e37ce438-12c7-462d-
>>>>> b2bc-1351be62b806@linaro.org/
>>>>>
>>>>> The technical comment is left unresponded so far since then.
>>>>>
>>>>> Since there is no expected phy cells to get on consumer's side, and
>>>>> since there is another link between CSIPHY and CSID described in
>>>>> video-interfaces.yaml, any kind of dependency of this IP on phy can
>>>>> be just removed, which serves the purpose of hardware description
>>>>> simplification.
>>>>
>>>> Rob's v1 feedback was consumer-decides-mode via the cell; Krzysztof has
>>>> R-b'd the binding with cells=1.
>>>
>>> Unfortunately this is not a technical discussion to move forward
>>> a better hardware description.
>>>
>>>> I'm happy to lock down this binding as-is on that basis.
>>>>
>>>> Are you arguing about phy-cells or are you actually arguing the entire
>>>> instantiation of Qualcomm MIPI CSI2 as a standalone generic driver in
>>>> drivers/phy ? Its not the same debate at all.
>>>
>>> So far I don't review the driver, only CSIPHY IP hardware description.
>>>
>>> In the current version of the hardware description one may find that
>>> the exploited 'video-intefaces' interface completely covers the 'phys'
>>> interface between CSIPHY and CSID, quite probably both are not needed
>>> at the same time, and the simpler 'phys' can be removed without any
>>> losses. But if it is kept, then its cell value is anyway redundant.
>>>
>>>> As the CAMSS maintainer, I'm fully against adding more inline monolithic
>>>> CSIPHY init sequences. That could be represented as a separate
>>>> linux-media driver but, then that implies that _all_ upstream CSIPHY
>>>> drivers in drivers/phy are wrong, and I just don't accept that.
>>>
>>> Other two drivers are different and should be excluded as good
>>> references, both of them links exactly one sensor to exactly one
>>> CSI-2 decoder, it's not the case here. Also one of these two drivers
>>> has no upstream users.
>>>
>>>> The bindings have DT maintainer RB, and the driver has Qcom engineer
>>>> tested-by, review-by.
>>>>
>>>> Vinod, Kishon - Again, I'd request this merges as-is.
>>>>
>>>
>>
>> The graph and phys are different layers, endpoints describe the
>> media topology, phys is how CSID controls the PHY through the PHY
>> framework (phy_get/configure/power_on).
>>
>> Removing phys doesn't simplify the description, it removes the control
>> interface which is the monolith again.
>>
>> We have multiple examples of this model upstream, cdns-dphy-rx +
>> ti-csi2rx in-tree, with users.
>>
>> I'm still kicking this up to Vinod, Kishon, request stands.
>>
>> Multiple DT and driver users are RB/Tested-by on the schema and driver,
>> lots of other work is gated.
>>
>> It is time to merge.
>>
>> Please adjudicate.
>>
>> ---
>> bod
> 
> Had a brief discussion with Vlad, Neil and Vinod
> 
> Resolution:
> 
> - phys = <> retained

It should suffice as an optional property, moreover any kind of
phy interface usage is not supposed to be done from CSID driver.

> - phy-cells = <0>
> - bus-type from the media-controller
>     Already specified with camss still aligns with Rob's declaration
>     "the consumer defines the mode" so I'm comfortable with this
>     compromise position.
>     This will still facilitate CPHY work/mode-selection via
>     phy_ops->configure() with how to define the CPHY part TBD.

As it was discussed there should be no phy specific interfaces
between this phy driver and CAMSS driver.

> - data-lanes = <0 1 2 3> valid

Would be nice to get an ack from linux-media maintainers on it.

> - MAX_LANES 8 instead of MAX_DATA_LANES 7

Since 'data-lanes' value follows some register bits and not
the actual PCB lanes as in video-intefaces.yaml, this shall be
explicitly described in the CSIPHY dt binding documentation.

> - v14 with these changes shortly.
> 

-- 
Best wishes,
Vladimir

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

^ permalink raw reply

* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-21 14:06 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Bryan O'Donoghue, Vinod Koul,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong
  Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel,
	Krzysztof Kozlowski
In-Reply-To: <864c13a0-a52c-43f4-9d94-646bf66ba31a@linaro.org>

On 21/07/2026 14:35, Vladimir Zapolskiy wrote:
>>
>> Had a brief discussion with Vlad, Neil and Vinod
>>
>> Resolution:
>>
>> - phys = <> retained
> 
> It should suffice as an optional property, moreover any kind of
> phy interface usage is not supposed to be done from CSID driver.
> 
>> - phy-cells = <0>
>> - bus-type from the media-controller
>>     Already specified with camss still aligns with Rob's declaration
>>     "the consumer defines the mode" so I'm comfortable with this
>>     compromise position.
>>     This will still facilitate CPHY work/mode-selection via
>>     phy_ops->configure() with how to define the CPHY part TBD.
> 
> As it was discussed there should be no phy specific interfaces
> between this phy driver and CAMSS driver.

I'm probably not getting this point, sorry about that. For clarity 
though CAMSS will use phys = <> and the PHY API phy_get(), 
phy_power_on(), or phy_configure() as there's no other possible 
interface to this driver.

> 
>> - data-lanes = <0 1 2 3> valid
> 
> Would be nice to get an ack from linux-media maintainers on it.
>> - MAX_LANES 8 instead of MAX_DATA_LANES 7
> 
> Since 'data-lanes' value follows some register bits and not
> the actual PCB lanes as in video-intefaces.yaml, this shall be
> explicitly described in the CSIPHY dt binding documentation.

Yes we said that.
>> - v14 with these changes shortly.
>>

---
bod

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

^ permalink raw reply

* Re: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
From: Loic Poulain @ 2026-07-21 14:22 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng,
	linux-arm-msm, linux-phy, linux-kernel, Konrad Dybcio, Abel Vesa
In-Reply-To: <4nd4f7bfliwumptfr2uykm4x5kyqd2gy7oputk7q57yu4g55df@4i5nzxl44i7p>

Hi Dmitry,


On Tue, Jul 21, 2026 at 3:32 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:05:46PM +0200, Loic Poulain wrote:
> > Runtime PM has to be enabled before creating the PHYs, since phy_create()
> > only enables runtime PM on the PHY devices if it is already enabled on
> > this parent device. This opens a small window where the device can be
> > runtime suspended after pm_runtime_enable() and before the later
> > pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
> > the PHYs are not yet registered.
> >
> > Take a runtime PM usage reference with pm_runtime_get_noresume() before
> > enabling runtime PM and release it once the PHYs have been created to
> > prevent the device from being runtime suspended during that window.
> >
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> > index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> > @@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> >       if (ret)
> >               goto err_node_put;
> >
> > +     /*
> > +      * Enable runtime PM before creating the PHYs, phy_create() only enables
> > +      * it on the PHY devices if already enabled on the parent. Hold a usage
> > +      * reference so callbacks cannot run until the PHY is ready.
> > +      */
> > +     pm_runtime_get_noresume(dev);
> >       pm_runtime_set_active(dev);
> >       ret = devm_pm_runtime_enable(dev);
> >       if (ret)
> > -             goto err_node_put;
> > +             goto err_pm_put;
> >       /*
> >        * Prevent runtime pm from being ON by default. Users can enable
> >        * it using power/control in sysfs.
> > @@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> >
> >       ret = qmp_usbc_register_clocks(qmp, np);
> >       if (ret)
> > -             goto err_node_put;
> > +             goto err_pm_put;
> >
> >       qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
> >       if (IS_ERR(qmp->usb_phy)) {
> >               ret = PTR_ERR(qmp->usb_phy);
> >               dev_err(dev, "failed to create PHY: %d\n", ret);
> > -             goto err_node_put;
> > +             goto err_pm_put;
> >       }
> >
> >       phy_set_drvdata(qmp->usb_phy, qmp);
> > @@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> >               if (IS_ERR(qmp->dp_phy)) {
> >                       ret = PTR_ERR(qmp->dp_phy);
> >                       dev_err(dev, "failed to create PHY: %d\n", ret);
> > -                     goto err_node_put;
> > +                     goto err_pm_put;
> >               }
> >               phy_set_drvdata(qmp->dp_phy, qmp);
> >       }
> >
> > +     pm_runtime_put(dev);
>
> Should be _put_sync() ?

Well, yes for avoiding inconsistent state on error path, but it might
be cleaner to move the async put after
devm_of_phy_provider_register(), once we know the probe has succeeded,
and then rely on put_noidle() for all error paths.

>
> > +
> >       of_node_put(np);
> >
> >       phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
> >
> >       return PTR_ERR_OR_ZERO(phy_provider);
> >
> > +err_pm_put:
> > +     pm_runtime_put_noidle(dev);
>
> Why is it _noidle()?

The device starts in the active state (pm_runtime_set_active()). Here,
I use put_noidle() only to balance the PM usage counter while keeping
the device active. This restores the runtime PM state to what it was
before the probe (pm_runtime_get_noresume), allowing the device to be
reprobed from the same state.


>
> >  err_node_put:
> >       of_node_put(np);
> >       return ret;
> >
> > --
> > 2.34.1
> >
> >
> > --
> > linux-phy mailing list
> > linux-phy@lists.infradead.org
> > https://lists.infradead.org/mailman/listinfo/linux-phy
>
> --
> With best wishes
> Dmitry

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

^ permalink raw reply

* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Frank Li @ 2026-07-21 18:57 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
	linux-phy, linux-media, devicetree, linux-kernel,
	Krzysztof Kozlowski
In-Reply-To: <20260720-x1e-csi2-phy-v13-1-160c31958863@linaro.org>

On Mon, Jul 20, 2026 at 02:11:34AM +0100, Bryan O'Donoghue wrote:
> Add a base schema for the MIPI CSI2 PHYs on Qualcomm SoCs. This PHY
> supports both DPHY and CPHY operation. A special mode of DPHY operation -
> called variously split-mode or combo-mode also allows for two sensors to be
> connected to one PHY.
>
> The submitted binding here describes the DPHY modes of operation only. CPHY
> is left to future work.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 195 +++++++++++++++++++++
>  1 file changed, 195 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> new file mode 100644
> index 0000000000000..880fe602945cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> @@ -0,0 +1,195 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm X1E80100 SoC CSI2 PHY
> +
> +maintainers:
> +  - Bryan O'Donoghue <bod@kernel.org>
> +
> +description:
> +  Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
> +  to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
> +  modes.
> +
> +properties:
> +  compatible:
> +    const: qcom,x1e80100-csi2-phy
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#phy-cells":
> +    const: 1
> +    description:
> +      The single cell specifies the PHY operating mode.
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: core
> +      - const: timer
> +      - const: ahb
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  operating-points-v2: true
> +
> +  opp-table:
> +    type: object
> +
> +  power-domains:
> +    items:
> +      - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
> +      - description: MMCX voltage rail
> +      - description: MXC or MXA voltage rail
> +
> +  power-domain-names:
> +    items:
> +      - const: top
> +      - const: mmcx
> +      - const: mx
> +
> +  vdda-0p9-supply:
> +    description: Phandle to a 0.9V regulator supply to a PHY.
> +
> +  vdda-1p2-supply:
> +    description: Phandle to 1.2V regulator supply to a PHY.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description:
> +          Sensor input. Always present. A single sensor is described by a
> +          single endpoint with one to four data lanes. DPHY split mode,
> +          where two independent sensors share the same PHY, is described
> +          by two endpoints; endpoint@0 with exactly two data-lanes and
> +          endpoint@1 with exactly one data-lane.

Sorry, jump in so late. I have questions about this design

why not model each lane as sperated phys.

csi1 {
	...
	phys = <&phy 0>, <&phy 1>; // use lane 0 and 1, connect sensor 1
};

csi2 {
	...
	phys = <&phy 2>; // use lan 2, connect sensor 2.
};

Frank

> +        unevaluatedProperties: false
> +
> +        patternProperties:
> +          "^endpoint(@[0-9a-f]+)?$":
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +            required:
> +              - data-lanes
> +              - remote-endpoint
> +
> +        allOf:
> +          - if:
> +              required:
> +                - endpoint@1
> +            then:
> +              properties:
> +                endpoint@0:
> +                  properties:
> +                    data-lanes:
> +                      minItems: 2
> +                      maxItems: 2
> +                endpoint@1:
> +                  properties:
> +                    data-lanes:
> +                      maxItems: 1
> +              required:
> +                - endpoint@0
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Output to the CAMSS CSID controller.
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - operating-points-v2
> +  - power-domains
> +  - power-domain-names
> +  - vdda-0p9-supply
> +  - vdda-1p2-supply
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
> +    #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
> +    #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> +    phy@ace4000 {
> +        compatible = "qcom,x1e80100-csi2-phy";
> +        reg = <0x0ace4000 0x2000>;
> +        #phy-cells = <1>;
> +
> +        clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> +                 <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> +                 <&camcc CAM_CC_CORE_AHB_CLK>;
> +        clock-names = "core",
> +                      "timer",
> +                      "ahb";
> +
> +        interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> +        operating-points-v2 = <&csiphy_opp_table>;
> +
> +        power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>,
> +                        <&rpmhpd RPMHPD_MMCX>,
> +                        <&rpmhpd RPMHPD_MX>;
> +        power-domain-names = "top",
> +                             "mmcx",
> +                             "mx";
> +
> +        vdda-0p9-supply = <&vreg_l2c_0p9>;
> +        vdda-1p2-supply = <&vreg_l1c_1p2>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                csiphy0_in: endpoint {
> +                    data-lanes = <0 1 2 3>;
> +                    remote-endpoint = <&sensor_out>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                csiphy0_out: endpoint {
> +                    remote-endpoint = <&csid_in>;
> +                };
> +            };
> +        };
> +
> +        csiphy_opp_table: opp-table {
> +            compatible = "operating-points-v2";
> +
> +            opp-300000000 {
> +                opp-hz = /bits/ 64 <300000000>;
> +                required-opps = <&rpmhpd_opp_low_svs_d1>,
> +                                <&rpmhpd_opp_low_svs_d1>;
> +            };
> +        };
> +    };
>
> --
> 2.54.0
>

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

^ permalink raw reply

* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-21 19:24 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Bryan O'Donoghue, Vinod Koul,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong
  Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel,
	Krzysztof Kozlowski
In-Reply-To: <864c13a0-a52c-43f4-9d94-646bf66ba31a@linaro.org>

On 21/07/2026 14:35, Vladimir Zapolskiy wrote:
>> - bus-type from the media-controller
>>     Already specified with camss still aligns with Rob's declaration
>>     "the consumer defines the mode" so I'm comfortable with this
>>     compromise position.
>>     This will still facilitate CPHY work/mode-selection via
>>     phy_ops->configure() with how to define the CPHY part TBD.
> 
> As it was discussed there should be no phy specific interfaces
> between this phy driver and CAMSS driver.
> 
>> - data-lanes = <0 1 2 3> valid
> 
> Would be nice to get an ack from linux-media maintainers on it.

OK so clearly the consensus I thought we had reached doesn't exist.

Vinod I would ask to merge this series as-is.

---
bod

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

^ permalink raw reply

* Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-07-21 21:49 UTC (permalink / raw)
  To: Frank Li, Bryan O'Donoghue
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
	devicetree, linux-kernel, Krzysztof Kozlowski
In-Reply-To: <al_BEUNJOrqVPQWw@lizhi-Precision-Tower-5810>

Hi Frank.

On 7/21/26 21:57, Frank Li wrote:
> On Mon, Jul 20, 2026 at 02:11:34AM +0100, Bryan O'Donoghue wrote:
>> Add a base schema for the MIPI CSI2 PHYs on Qualcomm SoCs. This PHY
>> supports both DPHY and CPHY operation. A special mode of DPHY operation -
>> called variously split-mode or combo-mode also allows for two sensors to be
>> connected to one PHY.
>>
>> The submitted binding here describes the DPHY modes of operation only. CPHY
>> is left to future work.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   .../bindings/phy/qcom,x1e80100-csi2-phy.yaml       | 195 +++++++++++++++++++++
>>   1 file changed, 195 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>> new file mode 100644
>> index 0000000000000..880fe602945cb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
>> @@ -0,0 +1,195 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm X1E80100 SoC CSI2 PHY
>> +
>> +maintainers:
>> +  - Bryan O'Donoghue <bod@kernel.org>
>> +
>> +description:
>> +  Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
>> +  to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
>> +  modes.
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,x1e80100-csi2-phy
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  "#phy-cells":
>> +    const: 1
>> +    description:
>> +      The single cell specifies the PHY operating mode.
>> +
>> +  clocks:
>> +    maxItems: 3
>> +
>> +  clock-names:
>> +    items:
>> +      - const: core
>> +      - const: timer
>> +      - const: ahb
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  operating-points-v2: true
>> +
>> +  opp-table:
>> +    type: object
>> +
>> +  power-domains:
>> +    items:
>> +      - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller.
>> +      - description: MMCX voltage rail
>> +      - description: MXC or MXA voltage rail
>> +
>> +  power-domain-names:
>> +    items:
>> +      - const: top
>> +      - const: mmcx
>> +      - const: mx
>> +
>> +  vdda-0p9-supply:
>> +    description: Phandle to a 0.9V regulator supply to a PHY.
>> +
>> +  vdda-1p2-supply:
>> +    description: Phandle to 1.2V regulator supply to a PHY.
>> +
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port@0:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        description:
>> +          Sensor input. Always present. A single sensor is described by a
>> +          single endpoint with one to four data lanes. DPHY split mode,
>> +          where two independent sensors share the same PHY, is described
>> +          by two endpoints; endpoint@0 with exactly two data-lanes and
>> +          endpoint@1 with exactly one data-lane.
> 
> Sorry, jump in so late. I have questions about this design
> 
> why not model each lane as sperated phys.
> 
> csi1 {
> 	...
> 	phys = <&phy 0>, <&phy 1>; // use lane 0 and 1, connect sensor 1
> };
> 
> csi2 {
> 	...
> 	phys = <&phy 2>; // use lan 2, connect sensor 2.
> };
> 

Such design has a number of flaws and in general it looks overly
complicated. From the beginning it's unclear how to distinguish
a clock lane from a data lane, how to specify lane frequencies etc.

-- 
Best wishes,
Vladimir

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

^ permalink raw reply

* Re: [PATCH v2 2/3] phy: qcom: qmp-combo: Specify PLL version in qmp_v6_dp_serdes_tbl structs
From: Dmitry Baryshkov @ 2026-07-21 22:46 UTC (permalink / raw)
  To: esteuwu
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, linux-phy, devicetree, linux-kernel,
	phone-devel
In-Reply-To: <20260715-sm8475-bup-usbss-v2-2-2d8def39b190@proton.me>

On Wed, Jul 15, 2026 at 03:06:55AM -0400, Esteban Urrutia via B4 Relay wrote:
> From: Esteban Urrutia <esteuwu@proton.me>
> 
> Two versions of PLLs for v6 PHYs exist: v1 and v1.1.
> These tables only cover v1.1 PLLs.
> For serdes, the difference between v1 and v1.1 PLLs is the value written to
> the BG_TIMER register.
> This register is set to 0x0e for v1 PLLs and 0x0a for v1.1 PLLs.
> As reference, SM8550 and SM8650 SoCs use v1.1 PLLs.
> For RBR/HBR/HBR2/HBR3, most values change.
> Make this distinction clear to avoid confusion when adding support for SoCs
> making use of v1 PLLs.

Checking the hardware docs I see no mention of v1 or v1.1 for these
PHYs.

> 
> Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 90 +++++++++++++++----------------
>  1 file changed, 45 insertions(+), 45 deletions(-)

-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v2 3/3] phy: qcom: qmp-combo: Add SM8475 support
From: Dmitry Baryshkov @ 2026-07-21 22:47 UTC (permalink / raw)
  To: esteuwu
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, linux-phy, devicetree, linux-kernel,
	phone-devel
In-Reply-To: <20260715-sm8475-bup-usbss-v2-3-2d8def39b190@proton.me>

On Wed, Jul 15, 2026 at 03:06:56AM -0400, Esteban Urrutia via B4 Relay wrote:
> From: Esteban Urrutia <esteuwu@proton.me>
> 
> Has been tested with the following capabilities:
> - USB Type-C at 10Gb/s
> - DP Alt Mode, using HBR2
> - USB Type-C at 480Mb/s + DP Alt Mode, using HBR2
> 
> RX and PCS USB tables had to be added, while serdes, TX and PCS tables were
> reused from other SoCs.
> Since SM8475 uses a v1 PLL, add and use v1 PLL tables as well.
> 
> Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 169 ++++++++++++++++++++++++++++++
>  1 file changed, 169 insertions(+)

With the v1 mentions being removed, LGTM.

-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v2 2/3] phy: qcom: qmp-pcie: Add pcs_lane1 offset to V5 offsets
From: Dmitry Baryshkov @ 2026-07-21 22:51 UTC (permalink / raw)
  To: Esteban Urrutia
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, linux-phy, devicetree, linux-kernel,
	phone-devel
In-Reply-To: <20260715-sm8475-bup-pcie-v2-2-48bd91a19abf@proton.me>

On Wed, Jul 15, 2026 at 02:37:47AM -0400, Esteban Urrutia wrote:
> Some SoCs such as SM8475 write data to registers using this offset,
> specifically SW_CTRL2 and MX_CTRL2.
> Add pcs_lane1 offset to V5 offsets to support this.
> 
> Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Fixes: 0fd0b31965b0 ("phy: qualcomm: qmp-pcie: add support for SAR2130P")


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

^ permalink raw reply

* Re: [PATCH v2 3/3] phy: qcom: qmp-pcie: Add support for SM8475 Gen3x1 PCIe0 port
From: Dmitry Baryshkov @ 2026-07-21 22:53 UTC (permalink / raw)
  To: Esteban Urrutia
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, linux-phy, devicetree, linux-kernel,
	phone-devel
In-Reply-To: <20260715-sm8475-bup-pcie-v2-3-48bd91a19abf@proton.me>

On Wed, Jul 15, 2026 at 02:37:48AM -0400, Esteban Urrutia wrote:
> This gets the port working. Tested with WCN6856 WLAN capabilities.
> Serdes, RX and PCS Misc tables had to be added, while TX and PCS tables
> were reused from SM8550, and PCS Lane1 was reused from SAR2130P.
> 
> Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 108 +++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

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

^ permalink raw reply

* [PATCH v4 phy-next 0/9] RCW override for 10G Lynx dynamic protocol reconfiguration
From: Vladimir Oltean @ 2026-07-21 23:15 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel, Conor Dooley, Krzysztof Kozlowski, Rob Herring

Previous set "New Generic PHY driver for Lynx 10G SerDes":
https://lore.kernel.org/linux-phy/20260610151952.2141019-1-vladimir.oltean@nxp.com/
introduced the 10G Lynx SerDes driver with a reduced functionality set.
Namely, only minor protocol changes are supported (1GbE <-> 2.5GbE).
The major protocol changes need a procedure named RCW override,
explained in more detail in commits 6/8 and 7/8.

This series adds kernel and device tree binding support for RCW
override, completing the SerDes PHY driver functionality.

Two components are involved:
- drivers/soc/fsl/guts.c (binding is fsl,layerscape-dcfg.yaml) - Device
  Configuration Unit, this is API provider for the SerDes driver to
  request RCW override depending on SoC
- drivers/phy/freescale/phy-fsl-lynx-10g.c - SerDes PHY driver, this is
  API consumer

The guts driver probes on DCFG blocks from multiple Freescale SoC
generations:
- MPC85xx, BSC and QorIQ (PowerPC) are all covered by the
  Documentation/devicetree/bindings/soc/fsl/guts.txt schema
- Layerscape (Arm) is covered by
  Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml

It is ultimately the same hardware block, just that (from what I can
tell) the Layerscape nodes are also compatible with syscon, and PowerPC
aren't.

RCW override has only been validated on select Layerscape SoCs, so
converting guts.txt to a PowerPC schema is out of scope for this
series - we don't even touch that (just in case it gets asked).

Using syscon to map the DCFG_DCSR register block in the Lynx SerDes
driver instead of creating this guts <-> lynx API was considered, but
because the RCW procedure is SoC-specific, it was ruled out for
polluting the SerDes driver. The guts driver is all about SoC awareness
anyway, and it offers some abstraction of all the gory details.

Changes since v3:
- new patch (1/9) to improve the fsl_guts_init() error path. The entire
  patch set is adapted to use the new convention.
- patch 8/9: avoid accessing an invalid soc.rcwcr_lock if
  fsl_guts_lane_set_mode() would be called too early
- patch 8/9: fix serdes block and lane count bounds
- patch 8/9 and 9/9: reimplement LS2088A RCW override in a way that is
  self-contained within the guts driver (based on reading and decoding
  RCWSR29[SRDS_PRTCL_S1]). Only a subset of SRDS_PRTCL_S1 values is
  supported for RCW override, due to availabilty of hardware to test on
  my side. Notably, protocols with XAUI and QSGMII are deliberately
  omitted from the SerDes table because only testing can show whether
  these protocols require RCWSR30_SRDS_CLK_SEL set to GMII or XGMII.
  This new method completely replaces fsl_guts_lane_init(), and has the
  advantage of also figuring out the initial protocol of unmanaged lanes,
  thereby not breaking them on RCW overrides.

v3 at:
https://lore.kernel.org/linux-phy/20260720133642.136324-1-vladimir.oltean@nxp.com/

Changes since v2:
- fix error handling in fsl_guts_init() and make sure that all newly
  introduced RCW override API functions fail if fsl_guts_init() failed.
- replace __bf_shf() use with __ffs()
- use read_poll_timeout_atomic() in fsl_guts_rcw_rmw() to clarify that
  DCFG_DCSR writes are supposed to reflect back in DCFG_CCSR
- only operate on lanes on which fsl_guts_lane_init() was called in
  ls2088a_serdes_init_rcwcr()
- put parentheses around (lane) in LS1088A_RCWSR29_SRDS_PRTCL_S1_LNn()
  and LS1088A_RCWSR30_SRDS_PRTCL_S2_LNn() macro expressions

v2 at:
https://lore.kernel.org/linux-phy/20260612210859.266759-8-vladimir.oltean@nxp.com/

Changes since v1:
- add Conor's review tag on 6/8
- update email addresses of DT maintainers
- drop DT maintainers from explicit CC on patch 7/8
- keep devicetree@vger.kernel.org CCed on entire series
- include missing <linux/bitfield.h> in patch 7/8
- namespace SRDS_PRTCL values for LS1046A and LS1088A, even if they are
  the same. For LS1028A (not covered here) they are not.
- prefix SRDS_CLK_SEL_{GMII,XGMII} with LS2088A_
- reorder alphanumerically (LS1046A should come before LS1088A)

Change logs also in individual patches.

v1 at:
https://lore.kernel.org/linux-phy/20260611193940.44416-1-vladimir.oltean@nxp.com/

Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>

Ioana Ciornei (4):
  soc: fsl: guts: use a macro to encode the DCFG CCSR space
  soc: fsl: guts: add a global structure to hold state
  soc: fsl: guts: add a central fsl_guts_read() function
  soc: fsl: guts: make it easier to determine on which SoC we are
    running

Vladimir Oltean (5):
  soc: fsl: guts: perform fsl_guts_init() error teardown in reverse
    order of setup
  soc: fsl: guts: make fsl_soc_data available after fsl_guts_init()
  dt-bindings: fsl: layerscape-dcfg: define DCFG_DCSR region
  soc: fsl: guts: implement the RCW override procedure
  phy: lynx-10g: use RCW override procedure for dynamic protocol change

 .../bindings/soc/fsl/fsl,layerscape-dcfg.yaml |  15 +-
 drivers/phy/freescale/Kconfig                 |   1 +
 drivers/phy/freescale/phy-fsl-lynx-10g.c      |  23 +-
 drivers/soc/fsl/guts.c                        | 528 ++++++++++++++++--
 include/linux/fsl/guts.h                      |  18 +-
 5 files changed, 532 insertions(+), 53 deletions(-)

-- 
2.34.1


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

^ permalink raw reply

* [PATCH v4 phy-next 1/9] soc: fsl: guts: perform fsl_guts_init() error teardown in reverse order of setup
From: Vladimir Oltean @ 2026-07-21 23:15 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260721231603.67865-1-vladimir.oltean@nxp.com>

fsl_guts_init() is about to get much more complicated and the central
error handling procedure cannot scale in its current design, unless we
add a lot of "if" conditions to detect what has been allocated and what
hasn't.

Currently the code relies on the fact that kfree(NULL) is safe, but this
doesn't scale to the case where "soc_dev_attr" itself is NULL, because
this would dereference "soc_dev_attr->family" and friends of a NULL
pointer.

Convert to the more typical error handling pattern where the teardown is
in the strict reverse order of setup, and a teardown step is only called
if its corresponding setup step was executed.

At the same time, maintain the optionality of soc_dev_attr->serial_number
by not checking whether that kasprintf() has returned NULL. In the error
path, kfree(NULL) is safe, so we don't need to add an "if" condition for
it. Michael Walle has confirmed that ignoring the error was intentional,
and we preserve that:
https://lore.kernel.org/linux-phy/DK44809N7Y8I.J2Z3U4N32H0Q@kernel.org/

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v3->v4: patch is new
---
 drivers/soc/fsl/guts.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 9bee7baec2b9..453456f31800 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -227,17 +227,23 @@ static int __init fsl_guts_init(void)
 	} else {
 		soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ");
 	}
-	if (!soc_dev_attr->family)
-		goto err_nomem;
+	if (!soc_dev_attr->family) {
+		ret = -ENOMEM;
+		goto err_free_soc_dev_attr;
+	}
 
 	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", svr);
-	if (!soc_dev_attr->soc_id)
-		goto err_nomem;
+	if (!soc_dev_attr->soc_id) {
+		ret = -ENOMEM;
+		goto err_free_family;
+	}
 
 	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d",
 					   (svr >>  4) & 0xf, svr & 0xf);
-	if (!soc_dev_attr->revision)
-		goto err_nomem;
+	if (!soc_dev_attr->revision) {
+		ret = -ENOMEM;
+		goto err_free_soc_id;
+	}
 
 	if (soc_data)
 		soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
@@ -249,7 +255,7 @@ static int __init fsl_guts_init(void)
 	soc_dev = soc_device_register(soc_dev_attr);
 	if (IS_ERR(soc_dev)) {
 		ret = PTR_ERR(soc_dev);
-		goto err;
+		goto err_free_serial_number;
 	}
 
 	pr_info("Machine: %s\n", soc_dev_attr->machine);
@@ -259,13 +265,14 @@ static int __init fsl_guts_init(void)
 
 	return 0;
 
-err_nomem:
-	ret = -ENOMEM;
-err:
-	kfree(soc_dev_attr->family);
-	kfree(soc_dev_attr->soc_id);
-	kfree(soc_dev_attr->revision);
+err_free_serial_number:
 	kfree(soc_dev_attr->serial_number);
+	kfree(soc_dev_attr->revision);
+err_free_soc_id:
+	kfree(soc_dev_attr->soc_id);
+err_free_family:
+	kfree(soc_dev_attr->family);
+err_free_soc_dev_attr:
 	kfree(soc_dev_attr);
 
 	return ret;
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v4 phy-next 2/9] soc: fsl: guts: use a macro to encode the DCFG CCSR space
From: Vladimir Oltean @ 2026-07-21 23:15 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260721231603.67865-1-vladimir.oltean@nxp.com>

From: Ioana Ciornei <ioana.ciornei@nxp.com>

Instead of using a hardcoded value when iomapping the DCFG CCSR space,
add a new macro for it. The code will be easier to follow this way,
especially when we add support for the DCFG DCSR space as well.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v4: none
---
 drivers/soc/fsl/guts.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 453456f31800..b97eb80cea95 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -14,6 +14,8 @@
 #include <linux/platform_device.h>
 #include <linux/fsl/guts.h>
 
+#define DCFG_CCSR	0
+
 struct fsl_soc_die_attr {
 	char	*die;
 	u32	svr;
@@ -197,7 +199,7 @@ static int __init fsl_guts_init(void)
 		return 0;
 	soc_data = match->data;
 
-	regs = of_iomap(np, 0);
+	regs = of_iomap(np, DCFG_CCSR);
 	if (!regs) {
 		of_node_put(np);
 		return -ENOMEM;
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v4 phy-next 3/9] soc: fsl: guts: add a global structure to hold state
From: Vladimir Oltean @ 2026-07-21 23:15 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260721231603.67865-1-vladimir.oltean@nxp.com>

From: Ioana Ciornei <ioana.ciornei@nxp.com>

Add the fsl_soc_guts structure in order to pass information like base
addresses, endianness etc between the init time and the runtime
operations (RCW override) which will get added in future patches.
There is no point in mapping and unmapping the DCFG CCSR space every
time we need to make a read, just map it once and keep its reference in
this new global struture.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v3->v4:
- change error handling to a dedicated 'err_unmap_dcfg_ccsr' label
v2->v3:
- fix error handling in fsl_guts_init() - iounmap() the CCSR range and
  set it to NULL on the "err" label rather than "err_nomem"
v1->v2: none
---
 drivers/soc/fsl/guts.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index b97eb80cea95..bb65b62ed805 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -106,6 +106,11 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
 	{ },
 };
 
+static struct fsl_soc_guts {
+	struct ccsr_guts __iomem *dcfg_ccsr;
+	bool little_endian;
+} soc;
+
 static const struct fsl_soc_die_attr *fsl_soc_die_match(
 	u32 svr, const struct fsl_soc_die_attr *matches)
 {
@@ -187,9 +192,7 @@ static int __init fsl_guts_init(void)
 	const struct fsl_soc_die_attr *soc_die;
 	const struct fsl_soc_data *soc_data;
 	const struct of_device_id *match;
-	struct ccsr_guts __iomem *regs;
 	struct device_node *np;
-	bool little_endian;
 	u64 soc_uid = 0;
 	u32 svr;
 	int ret;
@@ -199,24 +202,25 @@ static int __init fsl_guts_init(void)
 		return 0;
 	soc_data = match->data;
 
-	regs = of_iomap(np, DCFG_CCSR);
-	if (!regs) {
+	soc.dcfg_ccsr = of_iomap(np, DCFG_CCSR);
+	if (!soc.dcfg_ccsr) {
 		of_node_put(np);
 		return -ENOMEM;
 	}
 
-	little_endian = of_property_read_bool(np, "little-endian");
-	if (little_endian)
-		svr = ioread32(&regs->svr);
+	soc.little_endian = of_property_read_bool(np, "little-endian");
+	if (soc.little_endian)
+		svr = ioread32(&soc.dcfg_ccsr->svr);
 	else
-		svr = ioread32be(&regs->svr);
-	iounmap(regs);
+		svr = ioread32be(&soc.dcfg_ccsr->svr);
 	of_node_put(np);
 
 	/* Register soc device */
 	soc_dev_attr = kzalloc_obj(*soc_dev_attr);
-	if (!soc_dev_attr)
-		return -ENOMEM;
+	if (!soc_dev_attr) {
+		ret = -ENOMEM;
+		goto err_unmap_dcfg_ccsr;
+	}
 
 	ret = soc_attr_read_machine(soc_dev_attr);
 	if (ret)
@@ -276,6 +280,9 @@ static int __init fsl_guts_init(void)
 	kfree(soc_dev_attr->family);
 err_free_soc_dev_attr:
 	kfree(soc_dev_attr);
+err_unmap_dcfg_ccsr:
+	iounmap(soc.dcfg_ccsr);
+	soc.dcfg_ccsr = NULL;
 
 	return ret;
 }
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v4 phy-next 5/9] soc: fsl: guts: make it easier to determine on which SoC we are running
From: Vladimir Oltean @ 2026-07-21 23:15 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260721231603.67865-1-vladimir.oltean@nxp.com>

From: Ioana Ciornei <ioana.ciornei@nxp.com>

The guts driver will need to easily determine on which SoC it's running
when it will need to perform RCW override at runtime. The guts driver
knows this already because fsl_guts_init() reads the QorIQ/Layerscape
architectural System Version Register (SVR), but it doesn't save this
for later lookups.

Add a new qoriq_die enum to be used as an index in the fsl_soc_die
array. A new fsl_soc_die_match_one() function is also added so that we
can directly determine if the SVR is a match with a specific die.
The SVR value read from the DCFG CCSR is also kept in the global soc
structure so that it can be accessed when needed.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v3->v4: adapt to different error handling scheme
v1->v3: none
---
 drivers/soc/fsl/guts.c | 47 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 8e5c3cae811e..15674c6734c6 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -27,6 +27,23 @@ struct fsl_soc_data {
 	u32 uid_offset;
 };
 
+enum qoriq_die {
+	DIE_T4240,
+	DIE_T1040,
+	DIE_T2080,
+	DIE_T1024,
+	DIE_LS1043A,
+	DIE_LS2080A,
+	DIE_LS1088A,
+	DIE_LS1012A,
+	DIE_LS1046A,
+	DIE_LS2088A,
+	DIE_LS1021A,
+	DIE_LX2160A,
+	DIE_LS1028A,
+	DIE_MAX,
+};
+
 /* SoC die attribute definition for QorIQ platform */
 static const struct fsl_soc_die_attr fsl_soc_die[] = {
 	/*
@@ -34,21 +51,25 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
 	 */
 
 	/* Die: T4240, SoC: T4240/T4160/T4080 */
+	[DIE_T4240] =
 	{ .die		= "T4240",
 	  .svr		= 0x82400000,
 	  .mask		= 0xfff00000,
 	},
 	/* Die: T1040, SoC: T1040/T1020/T1042/T1022 */
+	[DIE_T1040] =
 	{ .die		= "T1040",
 	  .svr		= 0x85200000,
 	  .mask		= 0xfff00000,
 	},
 	/* Die: T2080, SoC: T2080/T2081 */
+	[DIE_T2080] =
 	{ .die		= "T2080",
 	  .svr		= 0x85300000,
 	  .mask		= 0xfff00000,
 	},
 	/* Die: T1024, SoC: T1024/T1014/T1023/T1013 */
+	[DIE_T1024] =
 	{ .die		= "T1024",
 	  .svr		= 0x85400000,
 	  .mask		= 0xfff00000,
@@ -59,46 +80,55 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
 	 */
 
 	/* Die: LS1043A, SoC: LS1043A/LS1023A */
+	[DIE_LS1043A] =
 	{ .die		= "LS1043A",
 	  .svr		= 0x87920000,
 	  .mask		= 0xffff0000,
 	},
 	/* Die: LS2080A, SoC: LS2080A/LS2040A/LS2085A */
+	[DIE_LS2080A] =
 	{ .die		= "LS2080A",
 	  .svr		= 0x87010000,
 	  .mask		= 0xff3f0000,
 	},
 	/* Die: LS1088A, SoC: LS1088A/LS1048A/LS1084A/LS1044A */
+	[DIE_LS1088A] =
 	{ .die		= "LS1088A",
 	  .svr		= 0x87030000,
 	  .mask		= 0xff3f0000,
 	},
 	/* Die: LS1012A, SoC: LS1012A */
+	[DIE_LS1012A] =
 	{ .die		= "LS1012A",
 	  .svr		= 0x87040000,
 	  .mask		= 0xffff0000,
 	},
 	/* Die: LS1046A, SoC: LS1046A/LS1026A */
+	[DIE_LS1046A] =
 	{ .die		= "LS1046A",
 	  .svr		= 0x87070000,
 	  .mask		= 0xffff0000,
 	},
 	/* Die: LS2088A, SoC: LS2088A/LS2048A/LS2084A/LS2044A */
+	[DIE_LS2088A] =
 	{ .die		= "LS2088A",
 	  .svr		= 0x87090000,
 	  .mask		= 0xff3f0000,
 	},
 	/* Die: LS1021A, SoC: LS1021A/LS1020A/LS1022A */
+	[DIE_LS1021A] =
 	{ .die		= "LS1021A",
 	  .svr		= 0x87000000,
 	  .mask		= 0xfff70000,
 	},
 	/* Die: LX2160A, SoC: LX2160A/LX2120A/LX2080A */
+	[DIE_LX2160A] =
 	{ .die          = "LX2160A",
 	  .svr          = 0x87360000,
 	  .mask         = 0xff3f0000,
 	},
 	/* Die: LS1028A, SoC: LS1028A */
+	[DIE_LS1028A] =
 	{ .die          = "LS1028A",
 	  .svr          = 0x870b0000,
 	  .mask         = 0xff3f0000,
@@ -109,6 +139,7 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
 static struct fsl_soc_guts {
 	struct ccsr_guts __iomem *dcfg_ccsr;
 	bool little_endian;
+	u32 svr;
 } soc;
 
 static unsigned int fsl_guts_read(const void __iomem *reg)
@@ -119,11 +150,16 @@ static unsigned int fsl_guts_read(const void __iomem *reg)
 	return ioread32be(reg);
 }
 
+static bool fsl_soc_die_match_one(u32 svr, const struct fsl_soc_die_attr *match)
+{
+	return match->svr == (svr & match->mask);
+}
+
 static const struct fsl_soc_die_attr *fsl_soc_die_match(
 	u32 svr, const struct fsl_soc_die_attr *matches)
 {
 	while (matches->svr) {
-		if (matches->svr == (svr & matches->mask))
+		if (fsl_soc_die_match_one(svr, matches))
 			return matches;
 		matches++;
 	}
@@ -202,7 +238,6 @@ static int __init fsl_guts_init(void)
 	const struct of_device_id *match;
 	struct device_node *np;
 	u64 soc_uid = 0;
-	u32 svr;
 	int ret;
 
 	np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, &match);
@@ -217,7 +252,7 @@ static int __init fsl_guts_init(void)
 	}
 
 	soc.little_endian = of_property_read_bool(np, "little-endian");
-	svr = fsl_guts_read(&soc.dcfg_ccsr->svr);
+	soc.svr = fsl_guts_read(&soc.dcfg_ccsr->svr);
 	of_node_put(np);
 
 	/* Register soc device */
@@ -231,7 +266,7 @@ static int __init fsl_guts_init(void)
 	if (ret)
 		of_machine_read_compatible(&soc_dev_attr->machine, 0);
 
-	soc_die = fsl_soc_die_match(svr, fsl_soc_die);
+	soc_die = fsl_soc_die_match(soc.svr, fsl_soc_die);
 	if (soc_die) {
 		soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ %s",
 						 soc_die->die);
@@ -243,14 +278,14 @@ static int __init fsl_guts_init(void)
 		goto err_free_soc_dev_attr;
 	}
 
-	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", svr);
+	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", soc.svr);
 	if (!soc_dev_attr->soc_id) {
 		ret = -ENOMEM;
 		goto err_free_family;
 	}
 
 	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d",
-					   (svr >>  4) & 0xf, svr & 0xf);
+					   (soc.svr >>  4) & 0xf, soc.svr & 0xf);
 	if (!soc_dev_attr->revision) {
 		ret = -ENOMEM;
 		goto err_free_soc_id;
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v4 phy-next 4/9] soc: fsl: guts: add a central fsl_guts_read() function
From: Vladimir Oltean @ 2026-07-21 23:15 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260721231603.67865-1-vladimir.oltean@nxp.com>

From: Ioana Ciornei <ioana.ciornei@nxp.com>

Add a central fsl_guts_read() function which will take into account the
endianness that was already determined. No point is duplicating the
if-else statement each time we need to read a DCFG register.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v4: none
---
 drivers/soc/fsl/guts.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index bb65b62ed805..8e5c3cae811e 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -111,6 +111,14 @@ static struct fsl_soc_guts {
 	bool little_endian;
 } soc;
 
+static unsigned int fsl_guts_read(const void __iomem *reg)
+{
+	if (soc.little_endian)
+		return ioread32(reg);
+
+	return ioread32be(reg);
+}
+
 static const struct fsl_soc_die_attr *fsl_soc_die_match(
 	u32 svr, const struct fsl_soc_die_attr *matches)
 {
@@ -209,10 +217,7 @@ static int __init fsl_guts_init(void)
 	}
 
 	soc.little_endian = of_property_read_bool(np, "little-endian");
-	if (soc.little_endian)
-		svr = ioread32(&soc.dcfg_ccsr->svr);
-	else
-		svr = ioread32be(&soc.dcfg_ccsr->svr);
+	svr = fsl_guts_read(&soc.dcfg_ccsr->svr);
 	of_node_put(np);
 
 	/* Register soc device */
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v4 phy-next 6/9] soc: fsl: guts: make fsl_soc_data available after fsl_guts_init()
From: Vladimir Oltean @ 2026-07-21 23:16 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel
In-Reply-To: <20260721231603.67865-1-vladimir.oltean@nxp.com>

In a future change, struct fsl_soc_data will be extended with methods
for performing RCW override.

Since this will be performed from a calling context outside
fsl_guts_init(), we need to keep track of the soc_data that we determine
at fsl_guts_init() time, so we can reference it later.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v3->v4: adapt to new error handling scheme
v2->v3: don't leave soc.data a valid pointer if fsl_guts_init() fails
v1->v2: none
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/soc/fsl/guts.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 15674c6734c6..c283d44b68a3 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -138,6 +138,7 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
 
 static struct fsl_soc_guts {
 	struct ccsr_guts __iomem *dcfg_ccsr;
+	const struct fsl_soc_data *data;
 	bool little_endian;
 	u32 svr;
 } soc;
@@ -231,10 +232,9 @@ static const struct of_device_id fsl_guts_of_match[] = {
 
 static int __init fsl_guts_init(void)
 {
-	struct soc_device_attribute *soc_dev_attr;
+	struct soc_device_attribute *soc_dev_attr = NULL;
 	static struct soc_device *soc_dev;
 	const struct fsl_soc_die_attr *soc_die;
-	const struct fsl_soc_data *soc_data;
 	const struct of_device_id *match;
 	struct device_node *np;
 	u64 soc_uid = 0;
@@ -243,12 +243,13 @@ static int __init fsl_guts_init(void)
 	np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, &match);
 	if (!np)
 		return 0;
-	soc_data = match->data;
+	soc.data = match->data;
 
 	soc.dcfg_ccsr = of_iomap(np, DCFG_CCSR);
 	if (!soc.dcfg_ccsr) {
 		of_node_put(np);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_clear_soc_data;
 	}
 
 	soc.little_endian = of_property_read_bool(np, "little-endian");
@@ -291,9 +292,9 @@ static int __init fsl_guts_init(void)
 		goto err_free_soc_id;
 	}
 
-	if (soc_data)
-		soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
-					       soc_data->uid_offset);
+	if (soc.data)
+		soc_uid = fsl_guts_get_soc_uid(soc.data->sfp_compat,
+					       soc.data->uid_offset);
 	if (soc_uid)
 		soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX",
 							soc_uid);
@@ -323,6 +324,8 @@ static int __init fsl_guts_init(void)
 err_unmap_dcfg_ccsr:
 	iounmap(soc.dcfg_ccsr);
 	soc.dcfg_ccsr = NULL;
+err_clear_soc_data:
+	soc.data = NULL;
 
 	return ret;
 }
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v4 phy-next 7/9] dt-bindings: fsl: layerscape-dcfg: define DCFG_DCSR region
From: Vladimir Oltean @ 2026-07-21 23:16 UTC (permalink / raw)
  To: linux-phy
  Cc: devicetree, linuxppc-dev, linux-arm-kernel, Ioana Ciornei,
	Vinod Koul, Neil Armstrong, Tanjeff Moos,
	Christophe Leroy (CS GROUP), Michael Walle, Shawn Guo, Frank Li,
	linux-kernel, Conor Dooley, Conor Dooley, Krzysztof Kozlowski,
	Rob Herring
In-Reply-To: <20260721231603.67865-1-vladimir.oltean@nxp.com>

In Layerscape (Arm) and QorIQ (PowerPC) devices, hardware peripherals
are accessed by the CPU through a portion of the SoC address space
called CCSR ("Configuration, Control, and Status Registers"). All
hardware IP blocks have their registers mapped here, and the Device
Configuration block makes no exception.

However, there exists a secondary range of the address space named DCSR
("Debug Control and Status Registers") which, like CCSR, also holds
registers of hardware IP blocks, except the DCSR contents is hidden in
all public reference manuals.

The intention of the CCSR/DCSR split, to the best of my knowledge, was
to place the functionality that is too low level for normal use, and
which is necessary only for debug, in a completely separate address
space which can be hidden.

A use case has appeared where networking SerDes lanes need to be
reconfigured at runtime for a different protocol (example: 10GBase-R to
SGMII), and the architecture of the SoCs does not normally permit that.
The Reset Configuration Word (RCW) is a data structure read by the SoC
preboot loader (PBL) which contains stuff like pinmuxing and SerDes
protocol mapping for each lane.

The RCW that the PBL has loaded is visible in the DCFG block's normal
status registers (from CCSR), as read only. Turns out, the RCW is also
mapped in the DCFG's shadow register map (in DCSR), in a write-only
form. Writing to the RCW registers from the DCFG's DCSR space to change
what the PBL has loaded is called "RCW override".

It has been validated that the RCW override procedure is necessary to
reconfigure the networking data path when a SerDes lane performs a major
protocol change. It changes some internal muxes which connect the PCS to
either the 10G MAC or to the 1G MAC.

Defining the DCSR area of the DCFG as a secondary 'reg' array element
allows operating systems to perform RCW overrides. Since it is
introduced late in the binding's lifetime, it is optional. It can be
identified by name, but also by index (first 'reg' is CCSR).

Note that while all SoCs should have a DCFG register block in DCSR, we
only need to expose it for the SoCs where the RCW override procedure is
known to be needed and has been validated.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>

v2->v4: none
v1->v2:
- add Conor's review tag
- update email addresses of DT maintainers
---
 .../bindings/soc/fsl/fsl,layerscape-dcfg.yaml     | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml b/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
index 3fb0534ea597..fc14fd0bf84b 100644
--- a/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
+++ b/Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
@@ -36,7 +36,20 @@ properties:
           - const: simple-mfd
 
   reg:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description:
+          Customer-visible DCFG register map from CCSR address space
+          (Configuration, Control and Status Registers)
+      - description:
+          Customer-hidden DCFG register map from DCSR address space
+          (Debug Control and Status Registers)
+
+  reg-names:
+    minItems: 1
+    items:
+      - const: dcfg_ccsr
+      - const: dcfg_dcsr
 
   little-endian: true
   big-endian: true
-- 
2.34.1


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

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).