Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v5 3/4] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock  resources
From: Hongling Zeng @ 2026-05-18  6:29 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
In-Reply-To: <20260518062938.48114-1-zenghongling@kylinos.cn>

ti_pipe3_get_clk() has two issues with -EPROBE_DEFER error handling:

1. When devm_clk_get() for sysclk fails, the function returns -EINVAL
   instead of propagating the actual error code. This masks -EPROBE_DEFER
   to -EINVAL, breaking the probe deferral mechanism and causing permanent
   driver initialization failure on systems with non-deterministic probe
   ordering.

2. For SATA PHY refclk, the function ignores all errors to support older
   DTBs missing the refclk property. However, this incorrectly ignores
   -EPROBE_DEFER as well, causing the driver to proceed without waiting
   for the clock provider to become available.

Fix both issues:
- Return PTR_ERR(phy->sys_clk) instead of -EINVAL to propagate all
  error codes including -EPROBE_DEFER
- Use devm_clk_get_optional() for SATA refclk to handle optional
  clocks while propagating -EPROBE_DEFER and other errors

Fixes: a70143bbef6b ("drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework")
Fixes: 7f33912d2978 ("phy: ti-pipe3: Fix SATA across suspend/resume")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
  Change in v4:
   - Merge refclk leak fix and EPROBE_DEFER fix into a single patch
   - Use devm_clk_get_optional() for SATA refclk
   - Drop manual -ENOENT handling
   - Ensure error paths are fully symmetric
---
  Change in v5:
   -Add Fix ignored clock enable return value in init patch
---
 drivers/phy/ti/phy-ti-pipe3.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index 2d36fe4c4218..9ec228c2a940 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -608,14 +608,20 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
 	struct clk *clk;
 	struct device *dev = phy->dev;
 
-	phy->refclk = devm_clk_get(dev, "refclk");
+	/*
+	 * refclk is optional for SATA PHY to support older DTBs, but
+	 * required for other modes. Use devm_clk_get_optional() for SATA
+	 * which returns NULL for -ENOENT, allowing us to propagate all
+	 * other errors including -EPROBE_DEFER.
+	 */
+	if (phy->mode == PIPE3_MODE_SATA)
+		phy->refclk = devm_clk_get_optional(dev, "refclk");
+	else
+		phy->refclk = devm_clk_get(dev, "refclk");
+
 	if (IS_ERR(phy->refclk)) {
 		dev_err(dev, "unable to get refclk\n");
-		/* older DTBs have missing refclk in SATA PHY
-		 * so don't bail out in case of SATA PHY.
-		 */
-		if (phy->mode != PIPE3_MODE_SATA)
-			return PTR_ERR(phy->refclk);
+		return PTR_ERR(phy->refclk);
 	}
 
 	if (phy->mode != PIPE3_MODE_SATA) {
@@ -632,7 +638,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
 		phy->sys_clk = devm_clk_get(dev, "sysclk");
 		if (IS_ERR(phy->sys_clk)) {
 			dev_err(dev, "unable to get sysclk\n");
-			return -EINVAL;
+			return PTR_ERR(phy->sys_clk);
 		}
 	}
 
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v5 1/4] phy: ti-pipe3: Fix ignored clock enable return value in init
From: Hongling Zeng @ 2026-05-18  6:29 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng, Sashiko AI
In-Reply-To: <20260518062938.48114-1-zenghongling@kylinos.cn>

ti_pipe3_init() ignores the return value of ti_pipe3_enable_clocks(),
which can lead to:

1. Unclocked hardware access if clock enable fails
2. Unbalanced clock disables in error paths

ti_pipe3_enable_clocks() returns an error code when clock enable fails
and rolls back any partially enabled clocks. If we ignore this error
and continue, we access hardware without proper clocking, which can
cause bus errors.

Additionally, if we reach error paths later in the function and call
ti_pipe3_disable_clocks(), we'll be disabling already-disabled clocks,
causing unbalanced disable warnings.

Fix this by checking the return value of ti_pipe3_enable_clocks()
and returning early if it fails.

Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Reported-by: Sashiko AI <sashiko@kernel.org>
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
Change in v5:
  -Add Fix ignored clock enable return value in init patch
---
 drivers/phy/ti/phy-ti-pipe3.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b5543b5c674c..b8c055893742 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -500,7 +500,10 @@ static int ti_pipe3_init(struct phy *x)
 	u32 val;
 	int ret = 0;
 
-	ti_pipe3_enable_clocks(phy);
+	ret = ti_pipe3_enable_clocks(phy);
+	if (ret)
+		return ret;
+
 	/*
 	 * Set pcie_pcs register to 0x96 for proper functioning of phy
 	 * as recommended in AM572x TRM SPRUHZ6, section 18.5.2.2, table
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v5 0/4] phy: ti-pipe3: Fix clock resource handling issues
From: Hongling Zeng @ 2026-05-18  6:29 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng

This patch series fixes multiple clock resource handling issues in the
ti-pipe3 PHY driver.

Patch 1 fixes a critical issue where ti_pipe3_init() was ignoring the
return value of ti_pipe3_enable_clocks(), which could lead to unclocked
hardware access and unbalanced clock disables.

Patch 2 fixes a clock resource leak when probe fails after enabling
the SATA refclk. The error path now properly disables the clock and
cleans up runtime PM resources.

Patch 3 fixes EPROBE_DEFER handling to prevent masking probe deferral
errors. It uses devm_clk_get_optional() for SATA refclk to properly
handle optional clocks while still propagating -EPROBE_DEFER and other
error codes.

Patch 4 fixes a clock leak in the init error path when regmap_update_bits()
fails in PCIe mode, adding proper clock cleanup consistent with other
error paths.

These fixes ensure proper resource cleanup on probe and init failures,
and prevent permanent driver initialization failures due to incorrect
error handling.

Changes in v5:
- Add patch to fix ignored clock enable return value in ti_pipe3_init()
- Improve error handling consistency across all paths

Changes in v4:
- Use devm_clk_get_optional() for SATA refclk
- Drop manual -ENOENT handling

Hongling Zeng (4):
  phy: ti-pipe3: Fix ignored clock enable return value in init
  phy: ti: pipe3: Fix clock resource leak on probe errors
  phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
  phy: ti-pipe3: Fix clock leak in init error path

 drivers/phy/ti/phy-ti-pipe3.c | 58 ++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 12 deletions(-)

--
2.25.1

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

^ permalink raw reply

* [PATCH v5 2/4] phy: ti: pipe3: Fix clock resource leak on probe errors
From: Hongling Zeng @ 2026-05-18  6:29 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
In-Reply-To: <20260518062938.48114-1-zenghongling@kylinos.cn>

When devm_phy_create() or devm_of_phy_provider_register() fails,
the refclk that was enabled earlier is not disabled, causing a
resource leak.

Fix this by adding an error handling path to disable the clock
when these functions fail.

Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
 Change in v2:
  -Add pm_runtime_disable() in error path (reported by Sashiko AI).
---
 change in v3:
  -Fix unbalanced clock disable by checking clk_prepare_enable()return value and
setting sata_refclk_enabledonly on success.
  -Fix error path teardown order to match ti_pipe3_remove()(disable clock before
disabling runtime PM).
---
 change in v4:
  -Fix the patch version
---
Change in v5:
  -Add Fix ignored clock enable return value in init patch
---
 drivers/phy/ti/phy-ti-pipe3.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b8c055893742..2d36fe4c4218 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -834,21 +834,39 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	 */
 	if (phy->mode == PIPE3_MODE_SATA) {
 		if (!IS_ERR(phy->refclk)) {
-			clk_prepare_enable(phy->refclk);
+			ret = clk_prepare_enable(phy->refclk);
+			if (ret) {
+				dev_err(dev, "Failed to enable refclk %d\n", ret);
+				goto err_pm_disable;
+			}
 			phy->sata_refclk_enabled = true;
 		}
 	}
 
 	generic_phy = devm_phy_create(dev, NULL, &ops);
-	if (IS_ERR(generic_phy))
-		return PTR_ERR(generic_phy);
+	if (IS_ERR(generic_phy)) {
+		ret = PTR_ERR(generic_phy);
+		goto err_clk_disable;
+	}
 
 	phy_set_drvdata(generic_phy, phy);
 
 	ti_pipe3_power_off(generic_phy);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
-	return PTR_ERR_OR_ZERO(phy_provider);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		goto err_clk_disable;
+	}
+
+	return 0;
+
+err_clk_disable:
+	if (phy->sata_refclk_enabled && !IS_ERR(phy->refclk))
+		clk_disable_unprepare(phy->refclk);
+err_pm_disable:
+	pm_runtime_disable(dev);
+	return ret;
 }
 
 static void ti_pipe3_remove(struct platform_device *pdev)
-- 
2.25.1


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

^ permalink raw reply related

* Re: [PATCH] phy: ti-pipe3: Fix clock leak in init error path
From: sashiko-bot @ 2026-05-18  4:12 UTC (permalink / raw)
  To: Hongling Zeng; +Cc: vkoul, neil.armstrong, linux-phy, olteanv
In-Reply-To: <20260518035105.26607-1-zenghongling@kylinos.cn>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ti_pipe3_init() completely ignores the return value of ti_pipe3_enable_clocks(), leading to unclocked hardware access and unbalanced clock disables.
--

commit 6e3b53b5ea9af504f2a62f9abfc933bbbb671e25
Author: Hongling Zeng <zenghongling@kylinos.cn>

phy: ti-pipe3: Fix clock leak in init error path

This commit adds a clock cleanup in the PCIe error path of ti_pipe3_init()
to prevent clocks from being permanently enabled when regmap_update_bits()
fails.

Closes: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/

> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index b5543b5c674c8..b5ee0af77c9f6 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -515,6 +515,8 @@ static int ti_pipe3_init(struct phy *x)
>  		val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
>  		ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
>  					 PCIE_PCS_MASK, val);
> +		if (ret)
> +			ti_pipe3_disable_clocks(phy);

Could this error path trigger an unbalanced clock disable warning if the clocks
were never successfully enabled?

Earlier in ti_pipe3_init(), the return value of ti_pipe3_enable_clocks() is
ignored:

drivers/phy/ti/phy-ti-pipe3.c:ti_pipe3_init() {
        ...
        int ret = 0;

        ti_pipe3_enable_clocks(phy);
        /*
         * Set pcie_pcs register to 0x96 for proper functioning of phy
        ...
}

If ti_pipe3_enable_clocks() fails, it rolls back and cleanly disables any
partially enabled clocks.

The initialization then continues anyway, interacting with unclocked hardware,
which might cause a bus error.

If it reaches this new error path because regmap_update_bits() fails,
ti_pipe3_disable_clocks() will be called unconditionally on already-disabled
clocks.

>  		return ret;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518035105.26607-1-zenghongling@kylinos.cn?part=1

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

^ permalink raw reply

* [PATCH] phy: ti-pipe3: Fix clock leak in init error path
From: Hongling Zeng @ 2026-05-18  3:51 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng, Sashiko AI,
	stable

When regmap_update_bits() fails in ti_pipe3_init() for PCIe mode,
the function returns the error without calling ti_pipe3_disable_clocks().
This leaves the clocks permanently enabled since the PHY framework won't
invoke the .exit callback on init failure.

Fix this by adding proper clock cleanup in the PCIe error path, consistent
with how the DPLL program error path handles cleanup.

Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Reported-by: Sashiko AI <sashiko@kernel.org>
Closes: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Cc: stable@vger.kernel.org
---
 drivers/phy/ti/phy-ti-pipe3.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index e0ab7d21e99c..ba1c937272b1 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -515,6 +515,8 @@ static int ti_pipe3_init(struct phy *x)
 		val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
 		ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
 					 PCIE_PCS_MASK, val);
+		if (ret)
+			ti_pipe3_disable_clocks(phy);
 		return ret;
 	}
 
-- 
2.25.1


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

^ permalink raw reply related

* Re: [PATCH v4 2/2] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
From: sashiko-bot @ 2026-05-18  2:59 UTC (permalink / raw)
  To: Hongling Zeng; +Cc: vkoul, olteanv, linux-phy, neil.armstrong
In-Reply-To: <20260518021539.14890-3-zenghongling@kylinos.cn>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Log spam during boot due to lack of dev_err_probe() for deferred clocks.
- [Low] Unnecessary braces used for single-statement if/else branches.
--

commit e5fb8e810f7245e72a8e8c129ec34b40381c5a33
Author: Hongling Zeng <zenghongling@kylinos.cn>

phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources

This commit fixes probe deferral handling in the ti-pipe3 driver by returning
the actual error code for sysclk failures instead of masking it. It also
propagates non-ENOENT errors for the SATA refclk.

> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -605,14 +605,21 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>  	struct clk *clk;
>  	struct device *dev = phy->dev;
>  
> -	phy->refclk = devm_clk_get(dev, "refclk");
> +	/*
> +	 * refclk is optional for SATA PHY to support older DTBs, but
> +	 * required for other modes. Use devm_clk_get_optional() for SATA
> +	 * which returns NULL for -ENOENT, allowing us to propagate all
> +	 * other errors including -EPROBE_DEFER.
> +	 */
> +	if (phy->mode == PIPE3_MODE_SATA) {
> +		phy->refclk = devm_clk_get_optional(dev, "refclk");
> +	} else {
> +		phy->refclk = devm_clk_get(dev, "refclk");
> +	}

This isn't a bug, but is it necessary to use braces here? The Linux kernel
coding style typically recommends omitting braces for single-statement
if/else branches.

> +
>  	if (IS_ERR(phy->refclk)) {
>  		dev_err(dev, "unable to get refclk\n");
> -		/* older DTBs have missing refclk in SATA PHY
> -		 * so don't bail out in case of SATA PHY.
> -		 */
> -		if (phy->mode != PIPE3_MODE_SATA)
> -			return PTR_ERR(phy->refclk);
> +		return PTR_ERR(phy->refclk);
>  	}

Since PTR_ERR() now correctly propagates -EPROBE_DEFER when the clock is not
yet available, will this dev_err() call cause repetitive log spam regressions
during normal boot sequence deferrals?

Could this be replaced with dev_err_probe() to properly suppress the error
message when the clock provider is simply deferred?

[ ... ]

> @@ -629,7 +636,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>  		phy->sys_clk = devm_clk_get(dev, "sysclk");
>  		if (IS_ERR(phy->sys_clk)) {
>  			dev_err(dev, "unable to get sysclk\n");
> -			return -EINVAL;
> +			return PTR_ERR(phy->sys_clk);
>  		}
>  	}

Does the same log spam regression apply here now that sysclk errors,
including -EPROBE_DEFER, are correctly propagated?

Using dev_err_probe() here might also help keep the kernel log clean during
probe deferral.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518021539.14890-1-zenghongling@kylinos.cn?part=2

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

^ permalink raw reply

* Re: [PATCH v4 1/2] phy: ti: pipe3: Fix clock resource leak on probe errors
From: sashiko-bot @ 2026-05-18  2:36 UTC (permalink / raw)
  To: Hongling Zeng; +Cc: vkoul, olteanv, neil.armstrong, linux-phy
In-Reply-To: <20260518021539.14890-2-zenghongling@kylinos.cn>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clock reference leak in ti_pipe3_init() on PCIe error path.
--

commit 26f79663d08d9c0cec90795a1b32d3e5e37b7841
Author: Hongling Zeng <zenghongling@kylinos.cn>

phy: ti: pipe3: Fix clock resource leak on probe errors

When devm_phy_create() or devm_of_phy_provider_register() fails,
the refclk that was enabled earlier is not disabled, causing a
resource leak.

Fix this by adding an error handling path to disable the clock
when these functions fail.

Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

> Fix this by adding an error handling path to disable the clock
> when these functions fail.

This isn't a bug in this patch, but while fixing clock leaks in this driver,
does ti_pipe3_init() also leak the clock references on the PCIe error path?

If regmap_update_bits() fails in ti_pipe3_init(), the function returns the
error without calling ti_pipe3_disable_clocks():

drivers/phy/ti/phy-ti-pipe3.c:ti_pipe3_init() {
	ti_pipe3_enable_clocks(phy);
	...
	if (phy->mode == PIPE3_MODE_PCIE) {
		if (!phy->pcs_syscon) {
			omap_control_pcie_pcs(phy->control_dev, 0x96);
			return 0;
		}

		val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
		ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
					 PCIE_PCS_MASK, val);
		return ret;
	}
	...
}

Because the initialization fails, the core PHY framework will not invoke the
.exit callback, potentially leaving the clocks permanently enabled.

Unlike the ti_pipe3_dpll_program() error path later in the same function
which correctly calls ti_pipe3_disable_clocks(phy), this PCIe error path
appears to miss the cleanup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518021539.14890-1-zenghongling@kylinos.cn?part=1

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

^ permalink raw reply

* Re: [PATCH 2/2] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
From: Hongling Zeng @ 2026-05-18  2:22 UTC (permalink / raw)
  To: Vladimir Oltean, Hongling Zeng
  Cc: vkoul, neil.armstrong, johan, kishon, rogerq, linux-phy,
	linux-kernel
In-Reply-To: <20260515155834.zgdp3xq7cbhq66jm@skbuf>


在 2026年05月15日 23:58, Vladimir Oltean 写道:
> On Fri, May 15, 2026 at 12:07:37PM +0800, Hongling Zeng wrote:
>> ti_pipe3_get_clk() has two issues with -EPROBE_DEFER error handling:
>>
>> 1. When devm_clk_get() for sysclk fails, the function returns -EINVAL
>>     instead of propagating the actual error code. This masks -EPROBE_DEFER
>>     to -EINVAL, breaking the probe deferral mechanism and causing permanent
>>     driver initialization failure on systems with non-deterministic probe
>>     ordering.
>>
>> 2. For SATA PHY refclk, the function ignores all errors to support older
>>     DTBs missing the refclk property. However, this incorrectly ignores
>>     -EPROBE_DEFER as well, causing the driver to proceed without waiting
>>     for the clock provider to become available.
>>
>> Fix both issues:
>> - Return PTR_ERR(phy->sys_clk) instead of -EINVAL to propagate all
>>    error codes including -EPROBE_DEFER
>> - For SATA refclk, only ignore -ENOENT (clock not found in old DTBs)
>>    but propagate other errors like -EPROBE_DEFER
>>
>> Fixes: a70143bbef6b ("drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework")
>> Fixes: 7f33912d2978 ("phy: ti-pipe3: Fix SATA across suspend/resume")
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   drivers/phy/ti/phy-ti-pipe3.c | 17 ++++++++++++-----
>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
>> index 58fbc3b27813..2bfcd0c70abd 100644
>> --- a/drivers/phy/ti/phy-ti-pipe3.c
>> +++ b/drivers/phy/ti/phy-ti-pipe3.c
>> @@ -604,15 +604,22 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>>   {
>>   	struct clk *clk;
>>   	struct device *dev = phy->dev;
>> +	int ret;
>>   
>>   	phy->refclk = devm_clk_get(dev, "refclk");
> When phy->mode == PIPE3_MODE_SATA, I think it would be a good idea to
> call devm_clk_get_optional() which converts ENOENT to 0 for you.
> Otherwise call devm_clk_get(). In both cases, you can propagate the
> returned error code without special-casing anything.
   Thank you for the review and suggestions.

   I've updated the patch to use devm_clk_get_optional() for SATA PHY
   refclk as suggested. This simplifies the error handling by automatically
   converting -ENOENT to NULL, allowing clean propagation of all other
   errors including -EPROBE_DEFER.

   I've also fixed the patch version numbering to v4 and ensured the
   patches are in the same series to avoid breaking Patchwork.

   Please see the attached v4 patch series.

>
>>   	if (IS_ERR(phy->refclk)) {
>> -		dev_err(dev, "unable to get refclk\n");
>> +		ret = PTR_ERR(phy->refclk);
>>   		/* older DTBs have missing refclk in SATA PHY
>> -		 * so don't bail out in case of SATA PHY.
>> +		 * so don't bail out for -ENOENT, but still defer
>> +		 * probe for other errors like -EPROBE_DEFER.
>>   		 */
>> -		if (phy->mode != PIPE3_MODE_SATA)
>> -			return PTR_ERR(phy->refclk);
>> +		if (ret == -ENOENT) {
>> +			if (phy->mode != PIPE3_MODE_SATA)
>> +				return ret;
>> +		} else {
>> +			dev_err(dev, "unable to get refclk\n");
>> +			return ret;
>> +		}
>>   	}
>>   
>>   	if (phy->mode != PIPE3_MODE_SATA) {
>> @@ -629,7 +636,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>>   		phy->sys_clk = devm_clk_get(dev, "sysclk");
>>   		if (IS_ERR(phy->sys_clk)) {
>>   			dev_err(dev, "unable to get sysclk\n");
>> -			return -EINVAL;
>> +			return PTR_ERR(phy->sys_clk);
>>   		}
>>   	}
>>   
>> -- 
>> 2.25.1
>>
>>
> Because of the broken threading this patch can't be applied anyway, so
>
> pw-bot: cr


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

^ permalink raw reply

* [PATCH v4 2/2] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
From: Hongling Zeng @ 2026-05-18  2:15 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
In-Reply-To: <20260518021539.14890-1-zenghongling@kylinos.cn>

ti_pipe3_get_clk() has two issues with -EPROBE_DEFER error handling:

1. When devm_clk_get() for sysclk fails, the function returns -EINVAL
   instead of propagating the actual error code. This masks -EPROBE_DEFER
   to -EINVAL, breaking the probe deferral mechanism and causing permanent
   driver initialization failure on systems with non-deterministic probe
   ordering.

2. For SATA PHY refclk, the function ignores all errors to support older
   DTBs missing the refclk property. However, this incorrectly ignores
   -EPROBE_DEFER as well, causing the driver to proceed without waiting
   for the clock provider to become available.

Fix both issues:
- Return PTR_ERR(phy->sys_clk) instead of -EINVAL to propagate all
  error codes including -EPROBE_DEFER
- For SATA refclk, only ignore -ENOENT (clock not found in old DTBs)
  but propagate other errors like -EPROBE_DEFER

Fixes: a70143bbef6b ("drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework")
Fixes: 7f33912d2978 ("phy: ti-pipe3: Fix SATA across suspend/resume")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
  Change in v4:
   - Merge refclk leak fix and EPROBE_DEFER fix into a single patch
   - Use devm_clk_get_optional() for SATA refclk
   - Drop manual -ENOENT handling
   - Ensure error paths are fully symmetric
---
 drivers/phy/ti/phy-ti-pipe3.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index 58fbc3b27813..e0ab7d21e99c 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -605,14 +605,21 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
 	struct clk *clk;
 	struct device *dev = phy->dev;
 
-	phy->refclk = devm_clk_get(dev, "refclk");
+	/*
+	 * refclk is optional for SATA PHY to support older DTBs, but
+	 * required for other modes. Use devm_clk_get_optional() for SATA
+	 * which returns NULL for -ENOENT, allowing us to propagate all
+	 * other errors including -EPROBE_DEFER.
+	 */
+	if (phy->mode == PIPE3_MODE_SATA) {
+		phy->refclk = devm_clk_get_optional(dev, "refclk");
+	} else {
+		phy->refclk = devm_clk_get(dev, "refclk");
+	}
+
 	if (IS_ERR(phy->refclk)) {
 		dev_err(dev, "unable to get refclk\n");
-		/* older DTBs have missing refclk in SATA PHY
-		 * so don't bail out in case of SATA PHY.
-		 */
-		if (phy->mode != PIPE3_MODE_SATA)
-			return PTR_ERR(phy->refclk);
+		return PTR_ERR(phy->refclk);
 	}
 
 	if (phy->mode != PIPE3_MODE_SATA) {
@@ -629,7 +636,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
 		phy->sys_clk = devm_clk_get(dev, "sysclk");
 		if (IS_ERR(phy->sys_clk)) {
 			dev_err(dev, "unable to get sysclk\n");
-			return -EINVAL;
+			return PTR_ERR(phy->sys_clk);
 		}
 	}
 
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v4 1/2] phy: ti: pipe3: Fix clock resource leak on probe errors
From: Hongling Zeng @ 2026-05-18  2:15 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
In-Reply-To: <20260518021539.14890-1-zenghongling@kylinos.cn>

When devm_phy_create() or devm_of_phy_provider_register() fails,
the refclk that was enabled earlier is not disabled, causing a
resource leak.

Fix this by adding an error handling path to disable the clock
when these functions fail.

Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
 Change in v2:
  -Add pm_runtime_disable() in error path (reported by Sashiko AI).
---
 change in v3:
  -Fix unbalanced clock disable by checking clk_prepare_enable()return value and
setting sata_refclk_enabledonly on success.
  -Fix error path teardown order to match ti_pipe3_remove()(disable clock before
disabling runtime PM).
---
 change in v4:
  -Fix the patch version
---
 drivers/phy/ti/phy-ti-pipe3.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b5543b5c674c..58fbc3b27813 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -831,21 +831,39 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	 */
 	if (phy->mode == PIPE3_MODE_SATA) {
 		if (!IS_ERR(phy->refclk)) {
-			clk_prepare_enable(phy->refclk);
+			ret = clk_prepare_enable(phy->refclk);
+			if (ret) {
+				dev_err(dev, "Failed to enable refclk %d\n", ret);
+				goto err_pm_disable;
+			}
 			phy->sata_refclk_enabled = true;
 		}
 	}
 
 	generic_phy = devm_phy_create(dev, NULL, &ops);
-	if (IS_ERR(generic_phy))
-		return PTR_ERR(generic_phy);
+	if (IS_ERR(generic_phy)) {
+		ret = PTR_ERR(generic_phy);
+		goto err_clk_disable;
+	}
 
 	phy_set_drvdata(generic_phy, phy);
 
 	ti_pipe3_power_off(generic_phy);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
-	return PTR_ERR_OR_ZERO(phy_provider);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		goto err_clk_disable;
+	}
+
+	return 0;
+
+err_clk_disable:
+	if (phy->sata_refclk_enabled && !IS_ERR(phy->refclk))
+		clk_disable_unprepare(phy->refclk);
+err_pm_disable:
+	pm_runtime_disable(dev);
+	return ret;
 }
 
 static void ti_pipe3_remove(struct platform_device *pdev)
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v4 0/2] phy: ti-pipe3: Fix clock resource handling issues
From: Hongling Zeng @ 2026-05-18  2:15 UTC (permalink / raw)
  To: vkoul, neil.armstrong, johan, kishon, rogerq
  Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng

This patch series fixes two clock resource handling issues in the
ti-pipe3 PHY driver:

Patch 1 fixes a clock resource leak when probe fails after enabling
the SATA refclk. The error path now properly disables the clock and
cleans up runtime PM resources.

Patch 2 fixes EPROBE_DEFER handling to prevent masking probe deferral
errors. It uses devm_clk_get_optional() for SATA refclk to properly
handle optional clocks while still propagating -EPROBE_DEFER and other
error codes.

These fixes ensure proper resource cleanup on probe failures and prevent
permanent driver initialization failures due to incorrect error handling.

Changes in v4:
- Fix patch version numbering (was incorrectly labeled as v2/v3)
- Ensure patches are in the same series to avoid breaking Patchwork

Hongling Zeng (2):
  phy: ti: pipe3: Fix clock resource leak on probe errors
  phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources

 drivers/phy/ti/phy-ti-pipe3.c | 47 +++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 11 deletions(-)

--
2.25.1

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

^ permalink raw reply

* Re: [PATCH 1/5] dt-bindings: phy: qcom,qusb2: Document ipq5210 compatible
From: Dmitry Baryshkov @ 2026-05-17 20:09 UTC (permalink / raw)
  To: Varadarajan Narayanan
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Greg Kroah-Hartman, linux-arm-msm,
	linux-phy, devicetree, linux-kernel, linux-usb
In-Reply-To: <20260515-usb2phy-v1-1-5f8338d466bf@oss.qualcomm.com>

On Fri, May 15, 2026 at 04:17:42PM +0530, Varadarajan Narayanan wrote:
> Document the compatible string used for the qusb2 phy in ipq5210.

Tell us something that isn't a part of the patch. Why do we need a
separate compatible for IPQ5210? Is it compatible with any other
platform?

> 
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> index 39851ba9de43..509cadd31adf 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> @@ -18,6 +18,7 @@ properties:
>      oneOf:
>        - items:
>            - enum:
> +              - qcom,ipq5210-qusb2-phy
>                - qcom,ipq5424-qusb2-phy
>                - qcom,ipq6018-qusb2-phy
>                - qcom,ipq8074-qusb2-phy
> 
> -- 
> 2.34.1
> 

-- 
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/4] dt-bindings: phy: qcom,msm8998-qmp-usb3-phy: Add support for Shikra
From: Dmitry Baryshkov @ 2026-05-17 19:16 UTC (permalink / raw)
  To: Krishna Kurapati
  Cc: Krzysztof Kozlowski, Neil Armstrong, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Xiangxu Yin, Johan Hovold, Loic Poulain, Kathiravan Thirumoorthy,
	linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <34586ed6-4f78-490f-a916-baf7657cca7a@oss.qualcomm.com>

On Fri, May 15, 2026 at 09:06:21PM +0530, Krishna Kurapati wrote:
> 
> 
> On 5/14/2026 8:07 PM, Krzysztof Kozlowski wrote:
> > On 14/05/2026 08:22, Krishna Kurapati wrote:
> > > 
> > > 
> > > On 5/14/2026 12:26 AM, Krzysztof Kozlowski wrote:
> > > > On 07/05/2026 13:37, Krishna Kurapati wrote:
> > > > > 
> > > > > 
> > > > > On 5/5/2026 7:30 PM, Krzysztof Kozlowski wrote:
> > > > > > On 05/05/2026 15:57, Krishna Kurapati wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On 5/5/2026 6:59 PM, Krzysztof Kozlowski wrote:
> > > > > > > > On 05/05/2026 15:27, Krishna Kurapati wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 5/5/2026 4:22 PM, Krzysztof Kozlowski wrote:
> > > > > > > > > > On 05/05/2026 12:49, Krzysztof Kozlowski wrote:
> > > > > > > > > > > On Mon, May 04, 2026 at 10:36:57PM +0530, Krishna Kurapati wrote:
> > > > > > > > > > > > Declare the USB-C QMP PHY present on the Qualcomm Shikra platform.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >       .../devicetree/bindings/phy/qcom,msm8998-qmp-usb3-phy.yaml      | 2 ++
> > > > > > > > > > > >       1 file changed, 2 insertions(+)
> > > > > > > > > > > 
> > > > > > > > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> > > > > > > > > > 
> > > > > > > > > > ... and then I looked at the driver. So un-reviewed. Devices are clearly
> > > > > > > > > > compatible. If not, explain what is not compatible.
> > > > > > > > > > 
> > > > > > > > > Talos uses GCC_USB3_PRIM_PHY_AUX_CLK.
> > > > > > > > > 
> > > > > > > > > In Shikra, we are using GCC_USB3_PRIM_PHY_COM_AUX_CLK. We don't have
> > > > > > > > > GCC_USB3_PRIM_PHY_AUX_CLK.
> > > > > > > > > 
> > > > > > > > > Hence, I didn't use a fallback compatible.
> > > > > > > > 
> > > > > > > > This still explains nothing. How different clock makes interface for SW
> > > > > > > > incompatible exactly?
> > > > > > > > 
> > > > > > > So I went by the naming. AUX vs COM_AUX.
> > > > > > 
> > > > > > The naming does not matter. If the clock is called
> > > > > > "no_one_expects_spanish_inquisition", does that make software
> > > > > > incompatible? Why would the name itself matter?
> > > > > > 
> > > > > > > 
> > > > > > > Can I use a fallback compatible and in DT vote for "COM_AUX" clock with
> > > > > > > clock-names mentioning "aux" ?
> > > > > > 
> > > > > > I don't know, I asked what is different in software interface.
> > > > > > 
> > > > > 
> > > > > Hi Krzysztof,
> > > > > 
> > > > >     I checked with the hw team here and found out two things.
> > > > > 
> > > > >     1. Shikra is a spinoff of Agatti and its sw interface (clocks used and
> > > > > regulators used) is the same as agatti.
> > > > > 
> > > > >     2. I thought we could use qcm2290 as a fallback since the phy register
> > > > > init sequence is the same for Talos/Shikra/Agatti. The difference
> > > > > between Talos and agatti when checked in the driver was the init load
> > > > > settings. I checked with the hw team and they suggested using the init
> > > > > load settings which talos was using.
> > > > > 
> > > > >     Hence both these compatibles (qcm2290 and qcs615) cannot be used as
> > > > > fallback for Shikra.
> > > > 
> > > > Then I do not understand why you are using qcs615_usb3phy_cfg for
> > > > Shikra. You say that the initialization is different, but you use
> > > > exactly the same initialization. So in a meaning of compatibility
> > > > between hardware for Devicetree they are compatible.
> > > > 
> > > Hi Krzysztof,
> > > 
> > >    There are 3 things:
> > > 
> > > 1. Clocks used:
> > > -> Talos supports AUX Clock since it supports DP over USB.
> > > -> Agatti and Shikra use COM_AUX clock since they dont support DP over USB.
> > > 
> > > 2. Phy register Init sequence - same for all 3 targets
> > > 
> > > 3. Regulator init load:
> > > -> Different for both Talos and Agatti
> > > -> Recommendation is to use Talos regulator load values.
> > > 
> > > SW interface wise, shikra is comaptible with agatti. If we use agatti as
> > > fallback, we would end up using the platform data of Agatti where the
> > > regulator init load is not suitable for Shikra. Hence not using Agatti
> > > as fallback.
> > > 
> > > Coming to driver changes, I used qcs615_cfg because it has required phy
> > > register sequence and regulator init load as needed by shikra.
> > 
> > So is it compatible with QCS615? If not, then something is incomplete or
> > confusing. The driver uses the same software interface.
> > 
> Sorry for the confusion. The Talos compatible represents the USB/DP PHY with
> aux clock input, while Shikra is a USB-only PHY with com_aux input clock, so
> the two PHYs are not compatible with each other.

According to the memory map, there is an (unused) DP registers part
right after the QMP USB3 PHY. So, sofware-wise it is compatible to
Talos. Having the different clock input means different integration of
the block rather than the differences in the hardware block.

So, the block should be compatible to qcom,qcs615-qmp-usb3-dp-phy

> 
> In the Linux driver implementation the match data is currently used only to
> affect the init sequence and regulator init load and here Shikra can reuse
> the Talos match data structure.
> 
> Regards,
> Krishna,
> 
> -- 
> 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 4/4] arm64: dts: qcom: Add Shikra CQM and CQS EVK boards
From: Komal Bajaj @ 2026-05-17 19:06 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
	Ulf Hansson, linux-arm-msm, devicetree, linux-kernel, linux-phy,
	linux-mmc, Imran Shaik, Krishna Kurapati, Monish Chunara,
	Rakesh Kota, Raviteja Laggyshetty, Sneh Mankad, Vishnu Santhosh,
	Xueyao An
In-Reply-To: <mw2ipu5z26ubmrg6qe2v36cp6ktkqtbpfe3hmr434qdgm66nz4@pu6yy7hw4n3t>

On 5/13/2026 5:34 PM, Dmitry Baryshkov wrote:
> On Tue, May 12, 2026 at 09:38:07AM +0530, Komal Bajaj wrote:
>> Add device trees for the Shikra EVK platform, which combines the CQM
>> SoM variant with a common carrier board.
>>
>> Two EVK boards are introduced:
>>    - shikra-cqm-evk.dts: pairs with CQM SoM (retail, with modem)
>>    - shikra-cqs-evk.dts: pairs with CQM SoM (retail, board has no modem
> CQS

ACK

>
>>      support)
>>
>> Also add shikra-evk.dtsi common across both EVK boards.
> What is the split between shikra-cq[ms]-evk.dts and shikra-evk.dtsi?

shikra-evk.dtsi represents the common carrier‑board and daughter‑card 
configuration shared across all Shikra EVK variants, including CQM, CQS, 
and the upcoming IQS board. It currently enables the debug UART, eMMC, 
and USB along with the required PMIC regulator supplies.
In follow‑up series, this file will be extended to cover the remaining 
common daughter‑card peripherals, such as MIC, display, camera, sensor, 
and NFC daughter cards.

Thanks
Komal

>
>> Each board DTS enables USB (peripheral mode) with the appropriate PMIC
>> regulator supplies for the QUSB2 and QMP PHYs, and eMMC with the
>> correct vmmc/vqmmc supplies for the CQM SoM's PMIC.
>>
>> Co-developed-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> Co-developed-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> Co-developed-by: Monish Chunara <quic_mchunara@quicinc.com>
>> Signed-off-by: Monish Chunara <quic_mchunara@quicinc.com>
>> Co-developed-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
>> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
>> Co-developed-by: Raviteja Laggyshetty <raviteja.laggyshetty@oss.qualcomm.com>
>> Signed-off-by: Raviteja Laggyshetty <raviteja.laggyshetty@oss.qualcomm.com>
>> Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
>> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
>> Co-developed-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
>> Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
>> Co-developed-by: Xueyao An <xueyao.an@oss.qualcomm.com>
>> Signed-off-by: Xueyao An <xueyao.an@oss.qualcomm.com>
>> Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/Makefile           |  2 +
>>   arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts | 61 +++++++++++++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 61 +++++++++++++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/shikra-evk.dtsi    | 13 ++++++
>>   4 files changed, 137 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index cc42829f92eb..6de783bcd133 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -328,6 +328,8 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sdm850-huawei-matebook-e-2019.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm850-lenovo-yoga-c630.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm850-samsung-w737.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdx75-idp.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= shikra-cqm-evk.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= shikra-cqs-evk.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sm4250-oneplus-billie2.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sm4450-qrd.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sm6115-fxtec-pro1x.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
>> new file mode 100644
>> index 000000000000..12eeca84832c
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
>> @@ -0,0 +1,61 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "shikra-cqm-som.dtsi"
>> +#include "shikra-evk.dtsi"
>> +
>> +/ {
>> +	model = "Qualcomm Technologies, Inc. Shikra CQM EVK";
>> +	compatible = "qcom,shikra-cqm-evk", "qcom,shikra-cqm-som", "qcom,shikra";
>> +	chassis-type = "embedded";
>> +
>> +	aliases {
>> +		mmc0 = &sdhc_1;
>> +		serial0 = &uart0;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +};
>> +
>> +&sdhc_1 {
>> +	vmmc-supply = <&pm4125_l20>;
>> +	vqmmc-supply = <&pm4125_l14>;
>> +
>> +	pinctrl-0 = <&sdc1_state_on>;
>> +	pinctrl-1 = <&sdc1_state_off>;
>> +	pinctrl-names = "default", "sleep";
>> +
>> +	non-removable;
>> +	supports-cqe;
>> +	no-sdio;
>> +	no-sd;
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_1 {
>> +	dr_mode = "peripheral";
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_1_hsphy {
>> +	vdd-supply = <&pm4125_l12>;
>> +	vdda-pll-supply = <&pm4125_l13>;
>> +	vdda-phy-dpdm-supply = <&pm4125_l21>;
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_qmpphy {
>> +	vdda-phy-supply = <&pm4125_l8>;
>> +	vdda-pll-supply = <&pm4125_l13>;
>> +
>> +	status = "okay";
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
>> new file mode 100644
>> index 000000000000..bc93282f64cf
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
>> @@ -0,0 +1,61 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "shikra-cqm-som.dtsi"
>> +#include "shikra-evk.dtsi"
>> +
>> +/ {
>> +	model = "Qualcomm Technologies, Inc. Shikra CQS EVK";
>> +	compatible = "qcom,shikra-cqs-evk", "qcom,shikra-cqm-som", "qcom,shikra";
>> +	chassis-type = "embedded";
>> +
>> +	aliases {
>> +		mmc0 = &sdhc_1;
>> +		serial0 = &uart0;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +};
>> +
>> +&sdhc_1 {
>> +	vmmc-supply = <&pm4125_l20>;
>> +	vqmmc-supply = <&pm4125_l14>;
>> +
>> +	pinctrl-0 = <&sdc1_state_on>;
>> +	pinctrl-1 = <&sdc1_state_off>;
>> +	pinctrl-names = "default", "sleep";
>> +
>> +	non-removable;
>> +	supports-cqe;
>> +	no-sdio;
>> +	no-sd;
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_1 {
>> +	dr_mode = "peripheral";
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_1_hsphy {
>> +	vdd-supply = <&pm4125_l12>;
>> +	vdda-pll-supply = <&pm4125_l13>;
>> +	vdda-phy-dpdm-supply = <&pm4125_l21>;
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_qmpphy {
>> +	vdda-phy-supply = <&pm4125_l8>;
>> +	vdda-pll-supply = <&pm4125_l13>;
>> +
>> +	status = "okay";
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/shikra-evk.dtsi b/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
>> new file mode 100644
>> index 000000000000..fae8c75b68b3
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
>> @@ -0,0 +1,13 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +&qupv3_0 {
>> +	firmware-name = "qcom/shikra/qupv3fw.elf";
>> +	status = "okay";
>> +};
>> +
>> +&uart0 {
>> +	status = "okay";
>> +};
>>
>> -- 
>> 2.34.1
>>


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

^ permalink raw reply

* Re: [PATCH 2/4] arm64: dts: qcom: Introduce Shikra SoC base dtsi
From: Komal Bajaj @ 2026-05-17 18:54 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: linux-phy, krzk+dt, devicetree, neil.armstrong, olteanv, conor+dt,
	robh, vkoul
In-Reply-To: <20260513040623.03F36C2BCC7@smtp.kernel.org>

On 5/13/2026 9:36 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Invalid PPI interrupt numbers (1, 2, 3, 0) are specified for the ARMv8 generic timer instead of the architecturally required numbers (13, 14, 11, 10).
> - [Medium] The `sdhc_1` node incorrectly uses RPMh interconnect tags (`QCOM_ICC_TAG_ALWAYS`, `QCOM_ICC_TAG_ACTIVE_ONLY`) on an RPM-based interconnect topology.
> - [Medium] The USB SuperSpeed wakeup interrupt (`ss_phy_irq`) directly references the GIC instead of routing through the MPM, breaking system wake-up from deep sleep.
> - [Low] The `sdhc_1` node uses the undeclared macro `RPMHPD_CX` for its `power-domains` property, which will cause a Device Tree compiler syntax error.
> --
>
> commit e2ee94ef3a5741f2cb9aae14dbef52c5f4a760f1
> Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> Subject: arm64: dts: qcom: Introduce Shikra SoC base dtsi
>
> This commit adds initial device tree support for the Qualcomm Shikra SoC, an
> IoT-focused platform featuring a Cortex-A55 and Cortex-A78C cluster. It
> configures RPM-based power and clock management alongside various peripherals
> like USB, eMMC, and the generic timer.
>
>> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
>> new file mode 100644
>> index 0000000000000..262c488add1ec
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
> [ ... ]
>
>> +		sdhc_1: mmc@4744000 {
>> +			compatible = "qcom,shikra-sdhci", "qcom,sdhci-msm-v5";
>> +
>> +			reg = <0x0 0x04744000 0x0 0x1000>,
>> +			      <0x0 0x04745000 0x0 0x1000>;
> [ ... ]
>
>> +			interconnects = <&system_noc MASTER_SDCC_1 QCOM_ICC_TAG_ALWAYS
>> +					&mc_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>,
>> +					<&mem_noc MASTER_AMPSS_M0 QCOM_ICC_TAG_ACTIVE_ONLY
>> +					&config_noc SLAVE_SDCC_1 QCOM_ICC_TAG_ACTIVE_ONLY>;
> Since the Shikra SoC utilizes an RPM-based architecture, should this node use
> interconnect tags from qcom,rpm-icc.h (such as RPM_ALWAYS_TAG) instead?
> Passing RPMh interconnect tags from qcom,icc.h to an RPM interconnect provider
> might result in unhandled parameters or silent bandwidth request failures.
>
>> +			interconnect-names = "sdhc-ddr",
>> +					     "cpu-sdhc";
>> +
>> +			power-domains = <&rpmpd RPMHPD_CX>;
> Does this power-domains assignment use an undeclared macro? RPMHPD_CX belongs
> to RPMh-based power domains, while this platform relies on RPMPD. This looks
> like it will cause a dtc syntax error during the build.
>
> [ ... ]
>
>> +		usb_1: usb@4e00000 {
>> +			compatible = "qcom,shikra-dwc3", "qcom,snps-dwc3";
>> +			reg = <0x0 0x04e00000 0x0 0xfc100>;
> [ ... ]
>
>> +			interrupts-extended = <&intc GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>,
>> +					      <&intc GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>,
>> +					      <&intc GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH>,
>> +					      <&intc GIC_SPI 254 IRQ_TYPE_LEVEL_HIGH>,
>> +					      <&intc GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "dwc_usb3",
>> +					  "pwr_event",
>> +					  "qusb2_phy",
>> +					  "hs_phy_irq",
>> +					  "ss_phy_irq";
> The mpm node correctly maps pin 12 to GIC SPI 422 for the DWC3 ss_phy_irq.
> Could routing this directly to the GIC here instead of hierarchically through
> the mpm (e.g., <&mpm 12 IRQ_TYPE_LEVEL_HIGH>) prevent the hardware from
> waking up from low-power states upon USB activity?
>
> [ ... ]
>
>> +	timer {
>> +		compatible = "arm,armv8-timer";
>> +
>> +		interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_PPI 2 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_PPI 3 IRQ_TYPE_LEVEL_LOW>,
>> +			     <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
>> +	};
> Are these PPI interrupt numbers correct for the ARMv8 generic timer? The
> architecturally defined numbers are typically 13 for Secure, 14 for Non-secure,
> 11 for Virtual, and 10 for Hypervisor. Using invalid hardware IRQ lines here
> could prevent the kernel from receiving scheduler ticks and stall the boot.

PPI numbers (1/2/3/0) are correct for Shikra. It uses the same older 
Qualcomm GIC implementation as sm6115, sm6375, sm6125, and sdm630, which 
all use these timer PPI numbers.

Thanks
Komal

>
>> +};


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

^ permalink raw reply

* Re: [PATCH 2/4] arm64: dts: qcom: Introduce Shikra SoC base dtsi
From: Komal Bajaj @ 2026-05-17 18:25 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
	Wesley Cheng, Ulf Hansson
  Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
	Imran Shaik, Krishna Kurapati, Monish Chunara, Rakesh Kota,
	Raviteja Laggyshetty, Sneh Mankad, Vishnu Santhosh, Xueyao An
In-Reply-To: <ced00124-9ba2-49ca-9200-62abd6ac3340@oss.qualcomm.com>

On 5/15/2026 4:07 PM, Konrad Dybcio wrote:
> On 5/12/26 6:08 AM, Komal Bajaj wrote:
>> Add initial device tree support for the Qualcomm Shikra SoC,
>> an IoT-focused platform built around a heterogeneous CPU cluster
>> (Cortex-A55 + Cortex-A78C) with RPM-based power and clock management.
> [...]
>
>> +	pmu {
>> +		compatible = "arm,armv8-pmuv3";
>> +		interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_HIGH>;
> Considering there's 2 core types, you may need something like:
>
> 9ce52e908bd5 ("arm64: dts: qcom: sm8650: switch to interrupt-cells 4 to add PPI partitions")
> 2c06e0797c32 ("arm64: dts: qcom: sm8650: add PPI interrupt partitions for the ARM PMUs")

Thanks for the pointer. I’ll update this to add the required PPI 
partitions, following the SM8650 approach.

>
>> +	};
>> +
>> +	psci: psci {
> Unused label, please drop

Sure, will drop it in next instance.

Thanks
Komal

>
> otherwise:
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> Konrad


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

^ permalink raw reply

* Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Dmitry Baryshkov @ 2026-05-17 17:18 UTC (permalink / raw)
  To: Kamal Wadhwa
  Cc: Rakesh Kota, linux-arm-msm, sashiko-reviews, Komal Bajaj, robh,
	linux-phy, neil.armstrong, vkoul, olteanv, krzk+dt, conor+dt,
	devicetree
In-Reply-To: <20260514103918.cs5m5kcxcrcgukvx@hu-kamalw-hyd.qualcomm.com>

On Thu, May 14, 2026 at 04:09:18PM +0530, Kamal Wadhwa wrote:
> On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
> > On 13/05/2026 17:29, Rakesh Kota wrote:
> > > On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
> > > > On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
> > > > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > > > - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
> > > > > - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
> > > > > --
> > > > > 
> > > > > commit 457abf6e7ac410430c866842e08e602ec3daaa51
> > > > > Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> > > > > 
> > > > > arm64: dts: qcom: Add Shikra CQM SoM platform
> > > > > 
> > > > > Add device tree include for the CQM variant of the Shikra System-on-Module.
> > > > > This variant integrates the Shikra SoC and PM4125 PMIC. The DTSI includes
> > > > > the common shikra.dtsi and adds PM4125 PMIC regulator definitions.
> > > > > 
> > > > > > diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > > new file mode 100644
> > > > > > index 0000000000000..401e71720519d
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > [ ... ]
> > > > > > +&rpm_requests {
> > > > > > +	regulators {
> > > > > > +		compatible = "qcom,rpm-pm2250-regulators";
> > > > > 
> > > > > Since the commit message indicates this is a PM4125 PMIC, should the
> > > > > compatible string include a specific identifier for the actual hardware?
> > > > > 
> > > > > Using a specific identifier like "qcom,rpm-pm4125-regulators" alongside
> > > > > the fallback allows the kernel to apply specific quirks if PM4125 errata
> > > > > are discovered later.
> > > > 
> > > > This can be ignored. The compat is a leftover from the historically
> > > > incorrect naming.
> > > > 
> > > > > 
> > > > > > +
> > > > > > +		pm4125_s2: s2 {
> > > > > > +			regulator-min-microvolt = <1000000>;
> > > > > > +			regulator-max-microvolt = <1200000>;
> > > > > > +		};
> > > > > 
> > > > > Do these regulators need to explicitly define their input supply dependencies
> > > > > such as vdd_s2-supply?
> > > > > 
> > > > > Without these properties, the regulator framework might be unaware that the
> > > > > PMIC regulators draw power from upstream supplies.
> > > > > 
> > > > > If the kernel dynamically manages the upstream supply and its reference count
> > > > > drops to zero, could it be disabled, causing an unexpected power loss for
> > > > > downstream components?
> > > > 
> > > > And this is a correct comment. Please provide missing supplies.
> > > > 
> > > As per the Qualcomm system design, the parent-child supply relationship
> > > is managed by the RPM firmware, not the Linux regulator framework. The
> > > RPM ensures the parent supply is never disabled until all subsystem
> > > votes are cleared.
> > 
> > How is this different from other, previous platforms?
> 
> This is not different. In the previous platforms too this is taken care from the
> RPM/RPMH firmware side, the only case where we may need explicit vote to parent
> is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
> may have a RPM/RPMH regulator as a parent.
> 
> Even on those previous targets the parent rail of all RPM/RPMH regulators are
> internally voted by RPM/RPMH FW at proper voltage with required headroom
> calculated based on the active child rails. This was done for all the
> subsystems (including APPS) regulators.
> 
> So no explicit handling from the APPS is required for parent supply.

You are explaining the driver behaviour. But the question is about the
hardware description. If there is no difference, please add necessary
supplies back.

-- 
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 1/4] dt-bindings: arm: qcom: Document Shikra and its EVK boards
From: Komal Bajaj @ 2026-05-17 17:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
	Ulf Hansson, linux-arm-msm, devicetree, linux-kernel, linux-phy,
	linux-mmc
In-Reply-To: <20260515-pumpkin-cuscus-of-expression-cec1ea@quoll>

On 5/15/2026 2:21 PM, Krzysztof Kozlowski wrote:
> On Tue, May 12, 2026 at 09:38:04AM +0530, Komal Bajaj wrote:
>> Shikra is a Qualcomm IoT SoC available in a System-on-Module (SoM)
>> form factor. The SoM integrates the Shikra SoC, PMICs, and essential
>> passives, and is designed to be mounted on carrier boards.
>>
>> One SoM variant is introduced:
>>    - CQM: retail variant with integrated modem (PM4125 PMIC)
>>
>> Two EVK boards are supported:
>>    - shikra-cqm-evk: pairs with the CQM SoM
>>    - shikra-cqs-evk: pairs with the CQM SoM, with no modem support
> s/CQM/CQS/

Right, will fix this typo.

>
> but anyway I would prefer to use full product names from
> include/dt-bindings/arm/qcom,ids.h (so CQXXXXM).

Sure, I'll update both CQM and CQS to their corresponding full name.

>
>> Each EVK provides debug UART, USB, and other peripheral interfaces.
>>
>> Add compatible strings for the CQM SoM variant and its two
>> corresponding EVK boards.
>>
>> Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>> ---
>>   Documentation/devicetree/bindings/arm/qcom.yaml | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>> index 2741c07e9f41..f041d71d7957 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>> @@ -963,6 +963,13 @@ properties:
>>             - const: qcom,qcs9100
>>             - const: qcom,sa8775p
>>   
>> +      - items:
>> +          - enum:
>> +              - qcom,shikra-cqm-evk
>> +              - qcom,shikra-cqs-evk
>> +          - const: qcom,shikra-cqm-som
> This is not accurate. The SoM has physically different SoC - either CQM
> or CQS. You either need two lists, like Toradex is doing (e.g.
> toradex,verdin-imx95), or enum in the middle of the lists, like Renesas
> is doing (e.g. renesas,rzt2h-evk).
>
> I prefer the first option, so Toradex choice.
>
> But for sure you do not get the third solution... Really, you are not
> supposed to invent these things, but look at existing solutions (and I
> was mentioning Toradex during internal meetings whenever someone asked
> me how the SoM and carrier should be organized).

Sure, I’ll follow the Toradex approach and restructure this with 
separate compatible lists for each EVK/SoM combination, as below:

- items:
       - enum:
           - qcom,shikra-cqm-evk
       - const: qcom,shikra-cqm-som
       - const: qcom,shikra

   - items:
       - enum:
           - qcom,shikra-cqs-evk
       - const: qcom,shikra-cqs-som
       - const: qcom,shikra


Thanks
Komal

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

^ permalink raw reply

* Re: [PATCH v3 1/2] phy: rockchip: inno-hdmi: Add configure() and validate() ops
From: Vinod Koul @ 2026-05-17 16:54 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Neil Armstrong, Heiko Stuebner, linux-phy, linux-rockchip,
	linux-arm-kernel, linux-kernel
In-Reply-To: <d69b500b-0473-496d-b17e-c60ce9839e0c@kwiboo.se>

On 15-05-26, 23:04, Jonas Karlman wrote:
> Hi,
> 
> On 5/15/2026 9:55 PM, Jonas Karlman wrote:
> > The commit 10ed34d6eaaf ("phy: Add HDMI configuration options")
> > introduced a way for HDMI PHYs to be configured through the generic
> > phy_configure() function.
> > 
> > This driver derives the TMDS character rate from the pixel clock and the
> > PHY bus width setting. However, no in-tree consumer of this PHY has ever
> > called phy_set_bus_width() to change the TMDS character rate as only
> > 8-bit RGB output is supported by the HDMI display driver.
> > 
> > Add configure() and validate() ops to allow consumers to configure the
> > TMDS character rate using phy_configure(). Fallback to the deprecated
> > way of using the PHY bus width to configure the TMDS character rate.
> > 
> > A typical call chain during DRM modeset on a RK3328 device:
> > 
> >   dw_hdmi_rockchip_encoder_atomic_check():
> >   - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
> > 
> >   dw_hdmi_rockchip_encoder_atomic_mode_set():
> >   - inno_hdmi_phy_configure(): pixclock 148500000
> >     - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
> > 
> >   vop_crtc_atomic_enable():
> >   - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
> >   - inno_hdmi_phy_rk3328_clk_set_rate(): pixclock 594000000 tmdsclock 594000000
> >   - inno_hdmi_phy_rk3328_clk_recalc_rate(): pixclock 594000000 vco 594000000
> > 
> >   dw_hdmi_rockchip_encoder_enable():
> >   - inno_hdmi_phy_power_on(): Inno HDMI PHY Power On
> >   - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
> > 
> > Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> > ---
> > Changes in v3:
> > - Change validate() ops to only validate tmdsclock
> > - Add comments about expected consumer usage
> > - Update commit message with a typical call chain
> > Changes in v2:
> > - Add validate() ops to validate that the TMDS rate is supported
> > - Split out parts that remove the old workaround into a separate patch
> > 
> > Patch "drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set()"
> > at [1] adds phy_validate() and phy_configure() calls for this HDMI PHY.
> > 
> > [1] https://lore.kernel.org/dri-devel/20260510183114.1248840-10-jonas@kwiboo.se/
> > ---
> >  drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 60 ++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> [snip]
> 
> > +static int inno_hdmi_phy_validate(struct phy *phy, enum phy_mode mode,
> > +				  int submode, union phy_configure_opts *opts)
> > +{
> > +	const struct pre_pll_config *cfg = pre_pll_cfg_table;
> > +	unsigned long tmdsclock;
> > +
> > +	if (!(mode == PHY_MODE_HDMI && submode == PHY_HDMI_MODE_TMDS))
> > +		return -EINVAL;
> > +
> > +	if (!opts->hdmi.tmds_char_rate || opts->hdmi.tmds_char_rate > 594000000)
> > +		return -EINVAL;
> 
> Sashiko reasoning log pointed out that a consumer of phy_validate() or
> phy_configure() can make a call with opts=NULL, so I may likely send a
> v4 of this series to fix such possible NULL pointer dereference here.

Yes please

> 
> Or is that something that possible should be checked before phy core
> calls the .validate()/.configure() ops?

The opts is an optional argument depending upon the mode to be valid. So
we cant do that in the core...

> 
> Multiple other phy .configure() ops seem to dereference opts members
> without any type of opts NULL check. (next-20260508)

We should fix these


PS: Any reason why you are sending encypted emails?

-- 
~Vinod

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
From: Vinod Koul @ 2026-05-17 16:37 UTC (permalink / raw)
  To: Ioana Ciornei
  Cc: neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy,
	devicetree, linux-kernel
In-Reply-To: <20260516060309.2282592-3-ioana.ciornei@nxp.com>

On 16-05-26, 09:03, Ioana Ciornei wrote:
> Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
> Dual-Channel Retimer. The driver currently supports only 10G and 1G link
> speeds but it can easily extended to also cover other usecases.
> 
> Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
> does not name the registers, the name for the macros were determined by
> their usage pattern.
> 
> A PHY device is created for each of the two channels present on the
> retimer. This allows for independent configuration of the two channels.
> This capability is especially important on retimers which have more than
> 2 channels that can be, depending on the board design, connected in
> multiple different ways to the SerDes lanes.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v3:
> - Use reverse Christmas tree ordering
> - Print a symbolic description in case of error
> - Some words do not need to be capitalized
> - Remove duplicated exit code path
> - Return -EINVAL in case of unsupported submode received in .set_mode()
> - Add a .validate() callback
> - Remove comma after sentinel entry
> - Add a ds125df111_rmw() helper
> - Use read_poll_timeout() to wait for channel reset to complete
> 
> Changes in v2:
> - Explicitly include all the needed headers
> - Change ds125df111_xlate() so that it returns an error if args_count is
> not exactly 1
> - Add a MAINTAINERS entry
> ---
>  MAINTAINERS                     |   7 +
>  drivers/phy/ti/Kconfig          |  10 ++
>  drivers/phy/ti/Makefile         |   1 +
>  drivers/phy/ti/phy-ds125df111.c | 294 ++++++++++++++++++++++++++++++++
>  4 files changed, 312 insertions(+)
>  create mode 100644 drivers/phy/ti/phy-ds125df111.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f877e5aaf2c7..58f410b666e7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26781,6 +26781,13 @@ T:	git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
>  F:	drivers/media/platform/ti/davinci/
>  F:	include/media/davinci/
>  
> +TI DS125DF111 RETIMER PHY DRIVER
> +M:	Ioana Ciornei <ioana.ciornei@nxp.com>
> +L:	linux-phy@lists.infradead.org (moderated for non-subscribers)
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
> +F:	drivers/phy/ti/phy-ds125df111.c
> +
>  TI ENHANCED CAPTURE (eCAP) DRIVER
>  M:	Vignesh Raghavendra <vigneshr@ti.com>
>  R:	Julien Panis <jpanis@baylibre.com>
> diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
> index b40f28019131..475e80fcd52d 100644
> --- a/drivers/phy/ti/Kconfig
> +++ b/drivers/phy/ti/Kconfig
> @@ -111,3 +111,13 @@ config PHY_TI_GMII_SEL
>  	help
>  	  This driver supports configuring of the TI CPSW Port mode depending on
>  	  the Ethernet PHY connected to the CPSW Port.
> +
> +config PHY_TI_DS125DF111

This should be in alphabetical order, so I guess before PHY_TI_G...

> +	tristate "DS125DF111 2-Channel Retimer Driver"
> +	depends on OF && I2C
> +	select GENERIC_PHY
> +	help
> +	  Enable this to add support for configuration and runtime management
> +	  of the TI DS125DF111 Multi-Protocol 2-Channel Retimer.
> +	  The retimer is modeled as a Generic PHY and supports both 10G and 1G
> +	  link speeds.
> diff --git a/drivers/phy/ti/Makefile b/drivers/phy/ti/Makefile
> index dcba2571c9bd..e68445ddd848 100644
> --- a/drivers/phy/ti/Makefile
> +++ b/drivers/phy/ti/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
>  obj-$(CONFIG_PHY_AM654_SERDES)		+= phy-am654-serdes.o
>  obj-$(CONFIG_PHY_TI_GMII_SEL)		+= phy-gmii-sel.o
>  obj-$(CONFIG_PHY_J721E_WIZ)		+= phy-j721e-wiz.o
> +obj-$(CONFIG_PHY_TI_DS125DF111)		+= phy-ds125df111.o

Here as well

> diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> new file mode 100644
> index 000000000000..8788f340e3b0
> --- /dev/null
> +++ b/drivers/phy/ti/phy-ds125df111.c
> @@ -0,0 +1,294 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2026 NXP */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/i2c.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>
> +#include <linux/slab.h>
> +
> +#define DS125DF111_NUM_CH			2
> +#define DS125DF111_NUM_VCO_GROUP_REG		5
> +
> +#define DS125DF111_CH_SELECT			0xff
> +#define DS125DF111_CH_SELECT_TARGET_MASK	GENMASK(3, 0)
> +#define DS125DF111_CH_SELECT_EN			BIT(2)
> +
> +#define DS125DF111_CH_CTRL			0x00
> +#define DS125DF111_CH_CTRL_RESET		BIT(2) /* self clearing */
> +
> +#define DS125DF111_CH_RST_SLEEP_US		10
> +#define DS125DF111_CH_RST_TIMEOUT_US		10000
> +
> +#define DS125DF111_VCO_GROUP_BASE		0x60
> +
> +#define DS125DF111_RATIOS			0x2F

Lower case for the hex values please

> +#define DS125DF111_RATIOS_RATE_MASK		GENMASK(7, 6)
> +#define DS125DF111_RATIOS_SUBRATE_MASK		GENMASK(5, 4)
> +#define DS125DF111_RATIOS_MASK			GENMASK(7, 4)
> +
> +struct ds125df111_ch {
> +	struct phy *phy;
> +	struct ds125df111_priv *priv;
> +	int idx;
> +};
> +
> +struct ds125df111_priv {
> +	struct ds125df111_ch ch[DS125DF111_NUM_CH];
> +	struct i2c_client *client;
> +	struct mutex mutex; /* protects access to shared registers */
> +};
> +
> +enum ds125df111_mode {
> +	FREQ_1G,
> +	FREQ_10G,
> +};
> +
> +static const struct ds125df111_config {
> +	u8 vco_group[DS125DF111_NUM_VCO_GROUP_REG];
> +	u8 rate;
> +	u8 subrate;
> +} ds125df111_cfg[] = {
> +	[FREQ_1G] = {
> +		/* VCO group #0 = 10GHz, VCO group #1 = 10GHz */
> +		.vco_group = {0x00, 0xB2, 0x00, 0xB2, 0xCC},
> +		/* By using the following combination of rate and subrate we
> +		 * select divide ratios of 1, 2, 4, 8 on both groups
> +		 */
> +		.rate = 0x1,
> +		.subrate = 0x2,
> +	},
> +
> +	[FREQ_10G] = {
> +		/* VCO group #0 = 10.3125GHz, VCO group #1 = 10.3125GHz */
> +		.vco_group = {0x90, 0xB3, 0x90, 0xB3, 0xCD},
> +		/* By using the following combination of rate and subrate we
> +		 * select divide ratios of 1 on both groups
> +		 */
> +		.rate = 0x1,
> +		.subrate = 0x3,
> +	},
> +};
> +
> +static int ds125df111_rmw(struct ds125df111_priv *priv, u8 reg, u8 clr, u8 set)
> +{
> +	struct i2c_client *i2c = priv->client;
> +	int err;
> +	u8 val;
> +
> +	err = i2c_smbus_read_byte_data(i2c, reg);
> +	if (err < 0)
> +		return err;
> +
> +	val = (u8)err;
> +	val &= ~clr;
> +	val |= set;
> +
> +	err = i2c_smbus_write_byte_data(i2c, reg, val);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int ds125df111_configure(struct phy *phy,
> +				const struct ds125df111_config *cfg)

This should start at preceding line open braces (here and few other
places)

> +{
> +	struct ds125df111_ch *ch = phy_get_drvdata(phy);
> +	struct ds125df111_priv *priv = ch->priv;
> +	struct i2c_client *i2c = priv->client;
> +	struct device *dev = &phy->dev;
> +	u8 ratios_val;
> +	int err, i;
> +	int val;
> +
> +	mutex_lock(&priv->mutex);
> +
> +	/* Make sure that any subsequent read/write operation will be directed
> +	 * only to the registers of the selected channel
> +	 */
> +	err = ds125df111_rmw(priv, DS125DF111_CH_SELECT,
> +			     DS125DF111_CH_SELECT_TARGET_MASK,
> +			     DS125DF111_CH_SELECT_EN | ch->idx);
> +	if (err < 0) {
> +		dev_err(dev, "Unable to select channel: %pe\n", ERR_PTR(err));
> +		goto out;
> +	}
> +
> +	/* Reset channel registers and wait until the bit was cleared */
> +	err = ds125df111_rmw(priv, DS125DF111_CH_CTRL, 0,
> +			     DS125DF111_CH_CTRL_RESET);
> +	if (err < 0) {
> +		dev_err(dev, "Error resetting channel configuration: %pe\n",
> +			ERR_PTR(err));
> +		goto out;
> +	}
> +
> +	err = read_poll_timeout(i2c_smbus_read_byte_data, val,
> +				val < 0 || !(val & DS125DF111_CH_CTRL_RESET),
> +				DS125DF111_CH_RST_SLEEP_US,
> +				DS125DF111_CH_RST_TIMEOUT_US, false, i2c,
> +				DS125DF111_CH_CTRL);
> +	if (err) {
> +		dev_err(dev, "Timed out waiting for channel reset: %pe\n",
> +			ERR_PTR(err));
> +		goto out;
> +	}
> +
> +	if (val < 0) {
> +		dev_err(dev, "Error reading reset status: %pe\n", ERR_PTR(val));
> +		err = val;
> +		goto out;
> +	}
> +
> +	/* Program the VCO group frequencies */
> +	for (i = 0; i < DS125DF111_NUM_VCO_GROUP_REG; i++) {
> +		err = i2c_smbus_write_byte_data(i2c,
> +						DS125DF111_VCO_GROUP_BASE + i,
> +						cfg->vco_group[i]);
> +		if (err < 0) {
> +			dev_err(dev, "Error programming VCO group: %pe\n",
> +				ERR_PTR(err));
> +			goto out;
> +		}
> +	}
> +
> +	/* Set the divide ratios for the VCO groups */
> +	ratios_val = FIELD_PREP(DS125DF111_RATIOS_RATE_MASK, cfg->rate) |
> +		FIELD_PREP(DS125DF111_RATIOS_SUBRATE_MASK, cfg->subrate);
> +	err = ds125df111_rmw(priv, DS125DF111_RATIOS, DS125DF111_RATIOS_MASK,
> +			     ratios_val);
> +	if (err < 0) {
> +		dev_err(dev, "Error programming the divide ratios: %pe\n",
> +			ERR_PTR(err));
> +		goto out;
> +	}
> +
> +out:
> +	mutex_unlock(&priv->mutex);
> +
> +	return err;
> +}
> +
> +static int ds125df111_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> +{
> +	const struct ds125df111_config *cfg;
> +
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EINVAL;
> +
> +	switch (submode) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		cfg = &ds125df111_cfg[FREQ_10G];
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	case PHY_INTERFACE_MODE_SGMII:
> +		cfg = &ds125df111_cfg[FREQ_1G];
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ds125df111_configure(phy, cfg);
> +}
> +
> +static int ds125df111_validate(struct phy *phy, enum phy_mode mode, int submode,
> +			       union phy_configure_opts *opts __always_unused)
> +{
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EINVAL;
> +
> +	switch (submode) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	case PHY_INTERFACE_MODE_SGMII:
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct phy_ops ds125df111_ops = {
> +	.validate	= ds125df111_validate,
> +	.set_mode	= ds125df111_set_mode,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct phy *ds125df111_xlate(struct device *dev,
> +				    const struct of_phandle_args *args)
> +{
> +	struct ds125df111_priv *priv = dev_get_drvdata(dev);
> +	u32 idx;
> +
> +	if (args->args_count != 1)
> +		return ERR_PTR(-EINVAL);
> +
> +	idx = args->args[0];
> +	if (idx >= DS125DF111_NUM_CH) {
> +		dev_err(dev, "Maximum number of channels is %d\n",
> +			DS125DF111_NUM_CH);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return priv->ch[idx].phy;
> +}
> +
> +static int ds125df111_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct phy_provider *provider;
> +	struct ds125df111_priv *priv;
> +	int i, err;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	priv->client = client;
> +	err = devm_mutex_init(dev, &priv->mutex);
> +	if (err)
> +		return err;
> +
> +	i2c_set_clientdata(client, priv);
> +
> +	for (i = 0; i < DS125DF111_NUM_CH; i++) {
> +		struct ds125df111_ch *ch = &priv->ch[i];
> +		struct phy *phy;
> +
> +		phy = devm_phy_create(dev, NULL, &ds125df111_ops);
> +		if (IS_ERR(phy))
> +			return PTR_ERR(phy);
> +
> +		ch->idx = i;
> +		ch->priv = priv;
> +		ch->phy = phy;
> +
> +		phy_set_drvdata(phy, ch);
> +	}
> +
> +	provider = devm_of_phy_provider_register(dev, ds125df111_xlate);
> +
> +	return PTR_ERR_OR_ZERO(provider);
> +}
> +
> +static const struct of_device_id ds125df111_dt_ids[] = {
> +	{ .compatible = "ti,ds125df111", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ds125df111_dt_ids);
> +
> +static struct i2c_driver ds125df111_driver = {
> +	.driver = {
> +		.name = "ds125df111",
> +		.of_match_table = ds125df111_dt_ids,
> +	},
> +	.probe = ds125df111_probe,
> +};
> +module_i2c_driver(ds125df111_driver);
> +
> +MODULE_AUTHOR("Ioana Ciornei <ioana.ciornei@nxp.com>");
> +MODULE_DESCRIPTION("TI DS125DF111 Retimer driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1

-- 
~Vinod

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

^ permalink raw reply

* Re: [PATCH v3 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
From: Vladimir Oltean @ 2026-05-16 17:16 UTC (permalink / raw)
  To: Ioana Ciornei
  Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy,
	devicetree, linux-kernel
In-Reply-To: <20260516060309.2282592-3-ioana.ciornei@nxp.com>

On Sat, May 16, 2026 at 09:03:09AM +0300, Ioana Ciornei wrote:
> Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
> Dual-Channel Retimer. The driver currently supports only 10G and 1G link
> speeds but it can easily extended to also cover other usecases.
> 
> Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
> does not name the registers, the name for the macros were determined by
> their usage pattern.
> 
> A PHY device is created for each of the two channels present on the
> retimer. This allows for independent configuration of the two channels.
> This capability is especially important on retimers which have more than
> 2 channels that can be, depending on the board design, connected in
> multiple different ways to the SerDes lanes.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

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

^ permalink raw reply

* [PATCH v3 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
From: Ioana Ciornei @ 2026-05-16  6:03 UTC (permalink / raw)
  To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy
  Cc: devicetree, linux-kernel
In-Reply-To: <20260516060309.2282592-1-ioana.ciornei@nxp.com>

Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
Dual-Channel Retimer. The driver currently supports only 10G and 1G link
speeds but it can easily extended to also cover other usecases.

Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
does not name the registers, the name for the macros were determined by
their usage pattern.

A PHY device is created for each of the two channels present on the
retimer. This allows for independent configuration of the two channels.
This capability is especially important on retimers which have more than
2 channels that can be, depending on the board design, connected in
multiple different ways to the SerDes lanes.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- Use reverse Christmas tree ordering
- Print a symbolic description in case of error
- Some words do not need to be capitalized
- Remove duplicated exit code path
- Return -EINVAL in case of unsupported submode received in .set_mode()
- Add a .validate() callback
- Remove comma after sentinel entry
- Add a ds125df111_rmw() helper
- Use read_poll_timeout() to wait for channel reset to complete

Changes in v2:
- Explicitly include all the needed headers
- Change ds125df111_xlate() so that it returns an error if args_count is
not exactly 1
- Add a MAINTAINERS entry
---
 MAINTAINERS                     |   7 +
 drivers/phy/ti/Kconfig          |  10 ++
 drivers/phy/ti/Makefile         |   1 +
 drivers/phy/ti/phy-ds125df111.c | 294 ++++++++++++++++++++++++++++++++
 4 files changed, 312 insertions(+)
 create mode 100644 drivers/phy/ti/phy-ds125df111.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f877e5aaf2c7..58f410b666e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26781,6 +26781,13 @@ T:	git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
 F:	drivers/media/platform/ti/davinci/
 F:	include/media/davinci/
 
+TI DS125DF111 RETIMER PHY DRIVER
+M:	Ioana Ciornei <ioana.ciornei@nxp.com>
+L:	linux-phy@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
+F:	drivers/phy/ti/phy-ds125df111.c
+
 TI ENHANCED CAPTURE (eCAP) DRIVER
 M:	Vignesh Raghavendra <vigneshr@ti.com>
 R:	Julien Panis <jpanis@baylibre.com>
diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
index b40f28019131..475e80fcd52d 100644
--- a/drivers/phy/ti/Kconfig
+++ b/drivers/phy/ti/Kconfig
@@ -111,3 +111,13 @@ config PHY_TI_GMII_SEL
 	help
 	  This driver supports configuring of the TI CPSW Port mode depending on
 	  the Ethernet PHY connected to the CPSW Port.
+
+config PHY_TI_DS125DF111
+	tristate "DS125DF111 2-Channel Retimer Driver"
+	depends on OF && I2C
+	select GENERIC_PHY
+	help
+	  Enable this to add support for configuration and runtime management
+	  of the TI DS125DF111 Multi-Protocol 2-Channel Retimer.
+	  The retimer is modeled as a Generic PHY and supports both 10G and 1G
+	  link speeds.
diff --git a/drivers/phy/ti/Makefile b/drivers/phy/ti/Makefile
index dcba2571c9bd..e68445ddd848 100644
--- a/drivers/phy/ti/Makefile
+++ b/drivers/phy/ti/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
 obj-$(CONFIG_PHY_AM654_SERDES)		+= phy-am654-serdes.o
 obj-$(CONFIG_PHY_TI_GMII_SEL)		+= phy-gmii-sel.o
 obj-$(CONFIG_PHY_J721E_WIZ)		+= phy-j721e-wiz.o
+obj-$(CONFIG_PHY_TI_DS125DF111)		+= phy-ds125df111.o
diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
new file mode 100644
index 000000000000..8788f340e3b0
--- /dev/null
+++ b/drivers/phy/ti/phy-ds125df111.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2026 NXP */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/i2c.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/slab.h>
+
+#define DS125DF111_NUM_CH			2
+#define DS125DF111_NUM_VCO_GROUP_REG		5
+
+#define DS125DF111_CH_SELECT			0xff
+#define DS125DF111_CH_SELECT_TARGET_MASK	GENMASK(3, 0)
+#define DS125DF111_CH_SELECT_EN			BIT(2)
+
+#define DS125DF111_CH_CTRL			0x00
+#define DS125DF111_CH_CTRL_RESET		BIT(2) /* self clearing */
+
+#define DS125DF111_CH_RST_SLEEP_US		10
+#define DS125DF111_CH_RST_TIMEOUT_US		10000
+
+#define DS125DF111_VCO_GROUP_BASE		0x60
+
+#define DS125DF111_RATIOS			0x2F
+#define DS125DF111_RATIOS_RATE_MASK		GENMASK(7, 6)
+#define DS125DF111_RATIOS_SUBRATE_MASK		GENMASK(5, 4)
+#define DS125DF111_RATIOS_MASK			GENMASK(7, 4)
+
+struct ds125df111_ch {
+	struct phy *phy;
+	struct ds125df111_priv *priv;
+	int idx;
+};
+
+struct ds125df111_priv {
+	struct ds125df111_ch ch[DS125DF111_NUM_CH];
+	struct i2c_client *client;
+	struct mutex mutex; /* protects access to shared registers */
+};
+
+enum ds125df111_mode {
+	FREQ_1G,
+	FREQ_10G,
+};
+
+static const struct ds125df111_config {
+	u8 vco_group[DS125DF111_NUM_VCO_GROUP_REG];
+	u8 rate;
+	u8 subrate;
+} ds125df111_cfg[] = {
+	[FREQ_1G] = {
+		/* VCO group #0 = 10GHz, VCO group #1 = 10GHz */
+		.vco_group = {0x00, 0xB2, 0x00, 0xB2, 0xCC},
+		/* By using the following combination of rate and subrate we
+		 * select divide ratios of 1, 2, 4, 8 on both groups
+		 */
+		.rate = 0x1,
+		.subrate = 0x2,
+	},
+
+	[FREQ_10G] = {
+		/* VCO group #0 = 10.3125GHz, VCO group #1 = 10.3125GHz */
+		.vco_group = {0x90, 0xB3, 0x90, 0xB3, 0xCD},
+		/* By using the following combination of rate and subrate we
+		 * select divide ratios of 1 on both groups
+		 */
+		.rate = 0x1,
+		.subrate = 0x3,
+	},
+};
+
+static int ds125df111_rmw(struct ds125df111_priv *priv, u8 reg, u8 clr, u8 set)
+{
+	struct i2c_client *i2c = priv->client;
+	int err;
+	u8 val;
+
+	err = i2c_smbus_read_byte_data(i2c, reg);
+	if (err < 0)
+		return err;
+
+	val = (u8)err;
+	val &= ~clr;
+	val |= set;
+
+	err = i2c_smbus_write_byte_data(i2c, reg, val);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int ds125df111_configure(struct phy *phy,
+				const struct ds125df111_config *cfg)
+{
+	struct ds125df111_ch *ch = phy_get_drvdata(phy);
+	struct ds125df111_priv *priv = ch->priv;
+	struct i2c_client *i2c = priv->client;
+	struct device *dev = &phy->dev;
+	u8 ratios_val;
+	int err, i;
+	int val;
+
+	mutex_lock(&priv->mutex);
+
+	/* Make sure that any subsequent read/write operation will be directed
+	 * only to the registers of the selected channel
+	 */
+	err = ds125df111_rmw(priv, DS125DF111_CH_SELECT,
+			     DS125DF111_CH_SELECT_TARGET_MASK,
+			     DS125DF111_CH_SELECT_EN | ch->idx);
+	if (err < 0) {
+		dev_err(dev, "Unable to select channel: %pe\n", ERR_PTR(err));
+		goto out;
+	}
+
+	/* Reset channel registers and wait until the bit was cleared */
+	err = ds125df111_rmw(priv, DS125DF111_CH_CTRL, 0,
+			     DS125DF111_CH_CTRL_RESET);
+	if (err < 0) {
+		dev_err(dev, "Error resetting channel configuration: %pe\n",
+			ERR_PTR(err));
+		goto out;
+	}
+
+	err = read_poll_timeout(i2c_smbus_read_byte_data, val,
+				val < 0 || !(val & DS125DF111_CH_CTRL_RESET),
+				DS125DF111_CH_RST_SLEEP_US,
+				DS125DF111_CH_RST_TIMEOUT_US, false, i2c,
+				DS125DF111_CH_CTRL);
+	if (err) {
+		dev_err(dev, "Timed out waiting for channel reset: %pe\n",
+			ERR_PTR(err));
+		goto out;
+	}
+
+	if (val < 0) {
+		dev_err(dev, "Error reading reset status: %pe\n", ERR_PTR(val));
+		err = val;
+		goto out;
+	}
+
+	/* Program the VCO group frequencies */
+	for (i = 0; i < DS125DF111_NUM_VCO_GROUP_REG; i++) {
+		err = i2c_smbus_write_byte_data(i2c,
+						DS125DF111_VCO_GROUP_BASE + i,
+						cfg->vco_group[i]);
+		if (err < 0) {
+			dev_err(dev, "Error programming VCO group: %pe\n",
+				ERR_PTR(err));
+			goto out;
+		}
+	}
+
+	/* Set the divide ratios for the VCO groups */
+	ratios_val = FIELD_PREP(DS125DF111_RATIOS_RATE_MASK, cfg->rate) |
+		FIELD_PREP(DS125DF111_RATIOS_SUBRATE_MASK, cfg->subrate);
+	err = ds125df111_rmw(priv, DS125DF111_RATIOS, DS125DF111_RATIOS_MASK,
+			     ratios_val);
+	if (err < 0) {
+		dev_err(dev, "Error programming the divide ratios: %pe\n",
+			ERR_PTR(err));
+		goto out;
+	}
+
+out:
+	mutex_unlock(&priv->mutex);
+
+	return err;
+}
+
+static int ds125df111_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+	const struct ds125df111_config *cfg;
+
+	if (mode != PHY_MODE_ETHERNET)
+		return -EINVAL;
+
+	switch (submode) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		cfg = &ds125df111_cfg[FREQ_10G];
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_SGMII:
+		cfg = &ds125df111_cfg[FREQ_1G];
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ds125df111_configure(phy, cfg);
+}
+
+static int ds125df111_validate(struct phy *phy, enum phy_mode mode, int submode,
+			       union phy_configure_opts *opts __always_unused)
+{
+	if (mode != PHY_MODE_ETHERNET)
+		return -EINVAL;
+
+	switch (submode) {
+	case PHY_INTERFACE_MODE_10GBASER:
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_SGMII:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct phy_ops ds125df111_ops = {
+	.validate	= ds125df111_validate,
+	.set_mode	= ds125df111_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *ds125df111_xlate(struct device *dev,
+				    const struct of_phandle_args *args)
+{
+	struct ds125df111_priv *priv = dev_get_drvdata(dev);
+	u32 idx;
+
+	if (args->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	idx = args->args[0];
+	if (idx >= DS125DF111_NUM_CH) {
+		dev_err(dev, "Maximum number of channels is %d\n",
+			DS125DF111_NUM_CH);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return priv->ch[idx].phy;
+}
+
+static int ds125df111_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct phy_provider *provider;
+	struct ds125df111_priv *priv;
+	int i, err;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->client = client;
+	err = devm_mutex_init(dev, &priv->mutex);
+	if (err)
+		return err;
+
+	i2c_set_clientdata(client, priv);
+
+	for (i = 0; i < DS125DF111_NUM_CH; i++) {
+		struct ds125df111_ch *ch = &priv->ch[i];
+		struct phy *phy;
+
+		phy = devm_phy_create(dev, NULL, &ds125df111_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		ch->idx = i;
+		ch->priv = priv;
+		ch->phy = phy;
+
+		phy_set_drvdata(phy, ch);
+	}
+
+	provider = devm_of_phy_provider_register(dev, ds125df111_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id ds125df111_dt_ids[] = {
+	{ .compatible = "ti,ds125df111", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ds125df111_dt_ids);
+
+static struct i2c_driver ds125df111_driver = {
+	.driver = {
+		.name = "ds125df111",
+		.of_match_table = ds125df111_dt_ids,
+	},
+	.probe = ds125df111_probe,
+};
+module_i2c_driver(ds125df111_driver);
+
+MODULE_AUTHOR("Ioana Ciornei <ioana.ciornei@nxp.com>");
+MODULE_DESCRIPTION("TI DS125DF111 Retimer driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 1/2] dt-bindings: phy: add PHY bindings for the TI DS125DF111 Retimer PHY
From: Ioana Ciornei @ 2026-05-16  6:03 UTC (permalink / raw)
  To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy
  Cc: devicetree, linux-kernel
In-Reply-To: <20260516060309.2282592-1-ioana.ciornei@nxp.com>

Add device tree binding for the TI DS125DF111 Retimer PHY.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
Changes in v3:
- none
Changes in v2:
- Remove the label from the example
- Rename the node from 'retimer' to 'phy'
---
 .../bindings/phy/ti,ds125df111.yaml           | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/ti,ds125df111.yaml

diff --git a/Documentation/devicetree/bindings/phy/ti,ds125df111.yaml b/Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
new file mode 100644
index 000000000000..ca4605f1d664
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/ti,ds125df111.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI DS125DF111 Retimer PHY
+
+description:
+  This binding describes the TI DS125DF111 multi-protocol Retimer PHY.
+
+maintainers:
+  - Ioana Ciornei <ioana.ciornei@nxp.com>
+
+properties:
+  compatible:
+    const: ti,ds125df111
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 1
+    description: |
+      The phandle's argument in the PHY specifier selects one of the two
+      channels of the retimer
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      phy@18 {
+        compatible = "ti,ds125df111";
+        reg = <0x18>;
+        #phy-cells = <1>;
+      };
+    };
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 0/2] phy: ti: add driver for TI DS125DF111 Dual-Channel Retimer
From: Ioana Ciornei @ 2026-05-16  6:03 UTC (permalink / raw)
  To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy
  Cc: devicetree, linux-kernel

This patch set adds a generic PHY driver and the corresponding DT
binding for the TI DS125DF111 Dual-Channel retimer. The datasheet on
which this driver was based on can be found at -
https://www.ti.com/lit/gpn/DS125DF111.

A separate generic PHY is registered for each of the two channels of the
retimer, so consumers can drive each channel independently. This allows
for independent control of the channels, which is especially important
since each channel can be routed to different SerDes lanes and it is not
guaranteed that the same retimer will do both directions of SerDes lane.

This was tested on a LS1088ARDB board with the Lynx10G SerDes PHY driver
yet to be submitted.

Changes in v3:
- Use reverse Christmas tree ordering
- Print a symbolic description in case of error
- Some words do not need to be capitalized
- Remove duplicated exit code path
- Return -EINVAL in case of unsupported submode received in .set_mode()
- Add a .validate() callback
- Remove comma after sentinel entry
- Add a ds125df111_rmw() helper
- Use read_poll_timeout() to wait for channel reset to complete
- Link to v2: https://lore.kernel.org/all/20260515110145.1925579-1-ioana.ciornei@nxp.com/

Changes in v2:
- Remove the label from the example
- Rename the node from 'retimer' to 'phy'
- Explicitly include all the needed headers
- Change ds125df111_xlate() so that it returns an error if args_count is
not exactly 1
- Add a MAINTAINERS entry
- Link to v1: https://lore.kernel.org/all/20260513185103.1371809-1-ioana.ciornei@nxp.com/

Ioana Ciornei (2):
  dt-bindings: phy: add PHY bindings for the TI DS125DF111 Retimer PHY
  phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer

 .../bindings/phy/ti,ds125df111.yaml           |  46 +++
 MAINTAINERS                                   |   7 +
 drivers/phy/ti/Kconfig                        |  10 +
 drivers/phy/ti/Makefile                       |   1 +
 drivers/phy/ti/phy-ds125df111.c               | 294 ++++++++++++++++++
 5 files changed, 358 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
 create mode 100644 drivers/phy/ti/phy-ds125df111.c

-- 
2.25.1


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

^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox