Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 phy-next 12/16] phy: lynx-28g: optimize read-modify-write operation
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

It is unnecessary to rewrite a register if the masked field already
contains the desired value upon reading. The hardware behaviour does not
depend upon register writes with identical values.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v3: none
---
 drivers/phy/freescale/phy-fsl-lynx-core.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
index d82e529fa65a..3d9508dfb2c1 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.h
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
@@ -93,7 +93,8 @@ static inline void lynx_rmw(struct lynx_priv *priv, unsigned long off, u32 val,
 	orig = lynx_read(priv, off);
 	tmp = orig & ~mask;
 	tmp |= val;
-	lynx_write(priv, off, tmp);
+	if (orig != tmp)
+		lynx_write(priv, off, tmp);
 }
 
 #define lynx_lane_rmw(lane, reg, val, mask)	\
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 11/16] phy: lynx-28g: add support for big endian register maps
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

Some 10G Lynx SerDes blocks are big endian and require byte swapping
because the CPUs are little endian armv8 (LS1046A). Parse the
"big-endian" device tree property, and modify the base lynx_read() and
lynx_write() accessors to test this property before issuing either the
ioread32() or ioread32be() variants (as per
Documentation/driver-api/device-io.rst).

All other accessors - lynx_rmw(), lynx_lane_read(), lynx_lane_write(),
lynx_lane_rmw(), lynx_pll_read() - need to go through these endian-aware
helpers.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: none
v1->v2:
- none. Deliberately ignoring this Sashiko feedback:
  https://lore.kernel.org/linux-phy/20260529120005.icj44ffdvdk25fjm@skbuf/
---
 drivers/phy/freescale/phy-fsl-lynx-core.c |  1 +
 drivers/phy/freescale/phy-fsl-lynx-core.h | 36 ++++++++++++++++-------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.c b/drivers/phy/freescale/phy-fsl-lynx-core.c
index 3fb89bb4b0d6..226b2af2b599 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.c
@@ -295,6 +295,7 @@ int lynx_probe(struct platform_device *pdev, const struct lynx_info *info,
 
 	priv->dev = dev;
 	priv->info = info;
+	priv->big_endian = device_property_read_bool(dev, "big-endian");
 	dev_set_drvdata(dev, priv);
 	spin_lock_init(&priv->pcc_lock);
 	INIT_DELAYED_WORK(&priv->cdr_check, lynx_cdr_lock_check);
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
index e8b280cc9b38..d82e529fa65a 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.h
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
@@ -58,36 +58,52 @@ struct lynx_priv {
 	 * like PCCn
 	 */
 	spinlock_t pcc_lock;
+	bool big_endian;
 	struct lynx_pll pll[LYNX_NUM_PLL];
 	struct lynx_lane *lane;
 
 	struct delayed_work cdr_check;
 };
 
+static inline u32 lynx_read(struct lynx_priv *priv, unsigned long off)
+{
+	void __iomem *reg = priv->base + off;
+
+	if (priv->big_endian)
+		return ioread32be(reg);
+
+	return ioread32(reg);
+}
+
+static inline void lynx_write(struct lynx_priv *priv, unsigned long off, u32 val)
+{
+	void __iomem *reg = priv->base + off;
+
+	if (priv->big_endian)
+		return iowrite32be(val, reg);
+
+	return iowrite32(val, reg);
+}
+
 static inline void lynx_rmw(struct lynx_priv *priv, unsigned long off, u32 val,
 			    u32 mask)
 {
-	void __iomem *reg = priv->base + off;
 	u32 orig, tmp;
 
-	orig = ioread32(reg);
+	orig = lynx_read(priv, off);
 	tmp = orig & ~mask;
 	tmp |= val;
-	iowrite32(tmp, reg);
+	lynx_write(priv, off, tmp);
 }
 
-#define lynx_read(priv, off) \
-	ioread32((priv)->base + (off))
-#define lynx_write(priv, off, val) \
-	iowrite32(val, (priv)->base + (off))
 #define lynx_lane_rmw(lane, reg, val, mask)	\
 	lynx_rmw((lane)->priv, reg(lane->id), val, mask)
 #define lynx_lane_read(lane, reg)			\
-	ioread32((lane)->priv->base + reg((lane)->id))
+	lynx_read((lane)->priv, reg((lane)->id))
 #define lynx_lane_write(lane, reg, val)		\
-	iowrite32(val, (lane)->priv->base + reg((lane)->id))
+	lynx_write((lane)->priv, reg((lane)->id), val)
 #define lynx_pll_read(pll, reg)			\
-	ioread32((pll)->priv->base + reg((pll)->id))
+	lynx_read((pll)->priv, reg((pll)->id))
 
 int lynx_probe(struct platform_device *pdev, const struct lynx_info *info,
 	       const struct phy_ops *phy_ops);
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 10/16] phy: lynx-28g: common probe() and remove()
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

Factor the device-agnostic logic from lynx_28g_probe() and
lynx_28g_remove() into lynx_probe() and lynx_remove() inside
phy-fsl-lynx-core.c. These will be shared with the 10G Lynx driver.

Since the PLL configuration, lane configuration and CDR lock detection
procedure are going to be different, introduce lynx_info function
pointers so that this code remains in the 28G Lynx driver.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3:
- adapt to lynx_28g_xlate() change to return ERR_PTR(-ENODEV)
v1->v2:
- adapt to the addition of a NULL check for the "info" pointer. Note
  that lynx_28g_probe() does not need to test for NULL because it only
  compares the pointer to a value.
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c  | 225 +++++-----------------
 drivers/phy/freescale/phy-fsl-lynx-core.c | 170 ++++++++++++++++
 drivers/phy/freescale/phy-fsl-lynx-core.h |  12 +-
 3 files changed, 227 insertions(+), 180 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index dbbadaa08cb6..50b991870edb 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -12,7 +12,6 @@
 #include "phy-fsl-lynx-core.h"
 
 #define LYNX_28G_NUM_LANE			8
-#define LYNX_28G_NUM_PLL			LYNX_NUM_PLL
 
 /* SoC IP wrapper for protocol converters */
 #define PCC8					0x10a0
@@ -781,6 +780,30 @@ static bool lynx_28g_compat_lane_supports_mode(int lane,
 	}
 }
 
+static void lynx_28g_cdr_lock_check(struct lynx_lane *lane)
+{
+	u32 rrstctl;
+	int err;
+
+	rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
+	if (!!(rrstctl & LNaRRSTCTL_CDR_LOCK))
+		return;
+
+	lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_RST_REQ,
+			  LNaRRSTCTL_RST_REQ);
+
+	err = read_poll_timeout(lynx_28g_lane_read, rrstctl,
+				!!(rrstctl & LNaRRSTCTL_RST_DONE),
+				LYNX_28G_LANE_RESET_SLEEP_US,
+				LYNX_28G_LANE_RESET_TIMEOUT_US,
+				false, lane, LNaRRSTCTL);
+	if (err) {
+		dev_warn_once(&lane->phy->dev,
+			      "Lane %c receiver reset failed: %pe\n",
+			      'A' + lane->id, ERR_PTR(err));
+	}
+}
+
 static void lynx_28g_lane_remap_pll(struct lynx_lane *lane,
 				    enum lynx_lane_mode lane_mode)
 {
@@ -1088,50 +1111,6 @@ static void lynx_28g_pll_read_configuration(struct lynx_pll *pll)
 	}
 }
 
-#define work_to_lynx(w) container_of((w), struct lynx_28g_priv, cdr_check.work)
-
-static void lynx_28g_cdr_lock_check(struct work_struct *work)
-{
-	struct lynx_28g_priv *priv = work_to_lynx(work);
-	struct lynx_28g_lane *lane;
-	u32 rrstctl;
-	int err, i;
-
-	for (i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
-		lane = &priv->lane[i];
-		if (!lane->phy)
-			continue;
-
-		mutex_lock(&lane->phy->mutex);
-
-		if (!lane->init || !lane->powered_up) {
-			mutex_unlock(&lane->phy->mutex);
-			continue;
-		}
-
-		rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
-		if (!(rrstctl & LNaRRSTCTL_CDR_LOCK)) {
-			lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_RST_REQ,
-					  LNaRRSTCTL_RST_REQ);
-
-			err = read_poll_timeout(lynx_28g_lane_read, rrstctl,
-						!!(rrstctl & LNaRRSTCTL_RST_DONE),
-						LYNX_28G_LANE_RESET_SLEEP_US,
-						LYNX_28G_LANE_RESET_TIMEOUT_US,
-						false, lane, LNaRRSTCTL);
-			if (err) {
-				dev_warn_once(&lane->phy->dev,
-					      "Lane %c receiver reset failed: %pe\n",
-					      'A' + lane->id, ERR_PTR(err));
-			}
-		}
-
-		mutex_unlock(&lane->phy->mutex);
-	}
-	queue_delayed_work(system_power_efficient_wq, &priv->cdr_check,
-			   msecs_to_jiffies(1000));
-}
-
 static void lynx_28g_lane_read_configuration(struct lynx_28g_lane *lane)
 {
 	u32 pccr, pss, protocol;
@@ -1157,49 +1136,13 @@ static void lynx_28g_lane_read_configuration(struct lynx_28g_lane *lane)
 	}
 }
 
-static struct phy *lynx_28g_xlate(struct device *dev,
-				  const struct of_phandle_args *args)
-{
-	struct lynx_28g_priv *priv = dev_get_drvdata(dev);
-	int idx;
-
-	if (args->args_count == 0)
-		return of_phy_simple_xlate(dev, args);
-	else if (args->args_count != 1)
-		return ERR_PTR(-ENODEV);
-
-	idx = args->args[0];
-
-	if (WARN_ON(idx >= LYNX_28G_NUM_LANE ||
-		    idx < priv->info->first_lane))
-		return ERR_PTR(-EINVAL);
-
-	return priv->lane[idx].phy ?: ERR_PTR(-ENODEV);
-}
-
-static int lynx_28g_probe_lane(struct lynx_28g_priv *priv, int id,
-			       struct device_node *dn)
-{
-	struct lynx_28g_lane *lane = &priv->lane[id];
-	struct phy *phy;
-
-	phy = devm_phy_create(priv->dev, dn, &lynx_28g_ops);
-	if (IS_ERR(phy))
-		return PTR_ERR(phy);
-
-	lane->priv = priv;
-	lane->phy = phy;
-	lane->id = id;
-	phy_set_drvdata(phy, lane);
-	lynx_28g_lane_read_configuration(lane);
-
-	return 0;
-}
-
 static const struct lynx_info lynx_info_compat = {
 	.get_pccr = lynx_28g_get_pccr,
 	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
+	.pll_read_configuration = lynx_28g_pll_read_configuration,
+	.lane_read_configuration = lynx_28g_lane_read_configuration,
+	.cdr_lock_check = lynx_28g_cdr_lock_check,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
@@ -1207,6 +1150,9 @@ static const struct lynx_info lynx_info_lx2160a_serdes1 = {
 	.get_pccr = lynx_28g_get_pccr,
 	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
+	.pll_read_configuration = lynx_28g_pll_read_configuration,
+	.lane_read_configuration = lynx_28g_lane_read_configuration,
+	.cdr_lock_check = lynx_28g_cdr_lock_check,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
@@ -1214,6 +1160,9 @@ static const struct lynx_info lynx_info_lx2160a_serdes2 = {
 	.get_pccr = lynx_28g_get_pccr,
 	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
+	.pll_read_configuration = lynx_28g_pll_read_configuration,
+	.lane_read_configuration = lynx_28g_lane_read_configuration,
+	.cdr_lock_check = lynx_28g_cdr_lock_check,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
@@ -1221,6 +1170,9 @@ static const struct lynx_info lynx_info_lx2160a_serdes3 = {
 	.get_pccr = lynx_28g_get_pccr,
 	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
+	.pll_read_configuration = lynx_28g_pll_read_configuration,
+	.lane_read_configuration = lynx_28g_lane_read_configuration,
+	.cdr_lock_check = lynx_28g_cdr_lock_check,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
@@ -1228,6 +1180,9 @@ static const struct lynx_info lynx_info_lx2162a_serdes1 = {
 	.get_pccr = lynx_28g_get_pccr,
 	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
+	.pll_read_configuration = lynx_28g_pll_read_configuration,
+	.lane_read_configuration = lynx_28g_lane_read_configuration,
+	.cdr_lock_check = lynx_28g_cdr_lock_check,
 	.first_lane = 4,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
@@ -1236,112 +1191,26 @@ static const struct lynx_info lynx_info_lx2162a_serdes2 = {
 	.get_pccr = lynx_28g_get_pccr,
 	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
+	.pll_read_configuration = lynx_28g_pll_read_configuration,
+	.lane_read_configuration = lynx_28g_lane_read_configuration,
+	.cdr_lock_check = lynx_28g_cdr_lock_check,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static int lynx_28g_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct phy_provider *provider;
-	struct lynx_28g_priv *priv;
-	struct device_node *dn;
-	int err;
-
-	dn = dev_of_node(dev);
-	if (!dn) {
-		dev_err(dev, "Device requires an OF node\n");
-		return -EINVAL;
-	}
-
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	priv->dev = dev;
-	priv->info = of_device_get_match_data(dev);
-	if (!priv->info)
-		return -ENODEV;
-
-	dev_set_drvdata(dev, priv);
-	spin_lock_init(&priv->pcc_lock);
-	INIT_DELAYED_WORK(&priv->cdr_check, lynx_28g_cdr_lock_check);
+	const struct lynx_info *info;
 
 	/*
 	 * If we get here it means we probed on a device tree where
 	 * "fsl,lynx-28g" wasn't the fallback, but the sole compatible string.
 	 */
-	if (priv->info == &lynx_info_compat)
+	info = of_device_get_match_data(dev);
+	if (info == &lynx_info_compat)
 		dev_warn(dev, "Please update device tree to use per-device compatible strings\n");
 
-	priv->lane = devm_kcalloc(dev, priv->info->num_lanes,
-				  sizeof(*priv->lane), GFP_KERNEL);
-	if (!priv->lane)
-		return -ENOMEM;
-
-	priv->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(priv->base))
-		return PTR_ERR(priv->base);
-
-	for (int i = 0; i < LYNX_28G_NUM_PLL; i++) {
-		struct lynx_28g_pll *pll = &priv->pll[i];
-
-		pll->priv = priv;
-		pll->id = i;
-		lynx_28g_pll_read_configuration(pll);
-	}
-
-	if (of_get_child_count(dn)) {
-		struct device_node *child;
-
-		for_each_available_child_of_node(dn, child) {
-			u32 reg;
-
-			/* PHY subnode name must be 'phy'. */
-			if (!(of_node_name_eq(child, "phy")))
-				continue;
-
-			if (of_property_read_u32(child, "reg", &reg)) {
-				dev_err(dev, "No \"reg\" property for %pOF\n", child);
-				of_node_put(child);
-				return -EINVAL;
-			}
-
-			if (reg < priv->info->first_lane || reg >= LYNX_28G_NUM_LANE) {
-				dev_err(dev, "\"reg\" property out of range for %pOF\n", child);
-				of_node_put(child);
-				return -EINVAL;
-			}
-
-			err = lynx_28g_probe_lane(priv, reg, child);
-			if (err) {
-				of_node_put(child);
-				return err;
-			}
-		}
-	} else {
-		for (int i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
-			err = lynx_28g_probe_lane(priv, i, NULL);
-			if (err)
-				return err;
-		}
-	}
-
-	provider = devm_of_phy_provider_register(dev, lynx_28g_xlate);
-	if (IS_ERR(provider))
-		return PTR_ERR(provider);
-
-	queue_delayed_work(system_power_efficient_wq, &priv->cdr_check,
-			   msecs_to_jiffies(1000));
-
-	return 0;
-}
-
-static void lynx_28g_remove(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	struct lynx_28g_priv *priv = dev_get_drvdata(dev);
-
-	cancel_delayed_work_sync(&priv->cdr_check);
+	return lynx_probe(pdev, info, &lynx_28g_ops);
 }
 
 static const struct of_device_id lynx_28g_of_match_table[] = {
@@ -1357,7 +1226,7 @@ MODULE_DEVICE_TABLE(of, lynx_28g_of_match_table);
 
 static struct platform_driver lynx_28g_driver = {
 	.probe = lynx_28g_probe,
-	.remove = lynx_28g_remove,
+	.remove = lynx_remove,
 	.driver = {
 		.name = "lynx-28g",
 		.of_match_table = lynx_28g_of_match_table,
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.c b/drivers/phy/freescale/phy-fsl-lynx-core.c
index 802e32dc6dca..3fb89bb4b0d6 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.c
@@ -2,6 +2,7 @@
 /* Copyright 2025-2026 NXP */
 
 #include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include "phy-fsl-lynx-core.h"
 
@@ -202,5 +203,174 @@ int lynx_pcvt_rmw(struct lynx_lane *lane, enum lynx_lane_mode mode, int cr,
 }
 EXPORT_SYMBOL_NS_GPL(lynx_pcvt_rmw, "PHY_FSL_LYNX");
 
+#define work_to_lynx(w) container_of((w), struct lynx_priv, cdr_check.work)
+
+static void lynx_cdr_lock_check(struct work_struct *work)
+{
+	struct lynx_priv *priv = work_to_lynx(work);
+	struct lynx_lane *lane;
+
+	for (int i = priv->info->first_lane; i < priv->info->num_lanes; i++) {
+		lane = &priv->lane[i];
+		if (!lane->phy)
+			continue;
+
+		mutex_lock(&lane->phy->mutex);
+
+		if (!lane->init || !lane->powered_up) {
+			mutex_unlock(&lane->phy->mutex);
+			continue;
+		}
+
+		priv->info->cdr_lock_check(lane);
+
+		mutex_unlock(&lane->phy->mutex);
+	}
+
+	queue_delayed_work(system_power_efficient_wq, &priv->cdr_check,
+			   msecs_to_jiffies(1000));
+}
+
+static struct phy *lynx_xlate(struct device *dev,
+			      const struct of_phandle_args *args)
+{
+	struct lynx_priv *priv = dev_get_drvdata(dev);
+	int idx;
+
+	if (args->args_count == 0)
+		return of_phy_simple_xlate(dev, args);
+	else if (args->args_count != 1)
+		return ERR_PTR(-ENODEV);
+
+	idx = args->args[0];
+
+	if (WARN_ON(idx >= priv->info->num_lanes ||
+		    idx < priv->info->first_lane))
+		return ERR_PTR(-EINVAL);
+
+	return priv->lane[idx].phy ?: ERR_PTR(-ENODEV);
+}
+
+static int lynx_probe_lane(struct lynx_priv *priv, int id,
+			   struct device_node *dn,
+			   const struct phy_ops *phy_ops)
+{
+	struct lynx_lane *lane = &priv->lane[id];
+	struct phy *phy;
+
+	phy = devm_phy_create(priv->dev, dn, phy_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	lane->priv = priv;
+	lane->phy = phy;
+	lane->id = id;
+	phy_set_drvdata(phy, lane);
+	priv->info->lane_read_configuration(lane);
+
+	return 0;
+}
+
+int lynx_probe(struct platform_device *pdev, const struct lynx_info *info,
+	       const struct phy_ops *phy_ops)
+{
+	struct device *dev = &pdev->dev;
+	struct phy_provider *provider;
+	struct device_node *dn;
+	struct lynx_priv *priv;
+	int err;
+
+	dn = dev_of_node(dev);
+	if (!dn) {
+		dev_err(dev, "Device requires an OF node\n");
+		return -EINVAL;
+	}
+
+	if (!info)
+		return -ENODEV;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->info = info;
+	dev_set_drvdata(dev, priv);
+	spin_lock_init(&priv->pcc_lock);
+	INIT_DELAYED_WORK(&priv->cdr_check, lynx_cdr_lock_check);
+
+	priv->lane = devm_kcalloc(dev, priv->info->num_lanes,
+				  sizeof(*priv->lane), GFP_KERNEL);
+	if (!priv->lane)
+		return -ENOMEM;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	for (int i = 0; i < LYNX_NUM_PLL; i++) {
+		struct lynx_pll *pll = &priv->pll[i];
+
+		pll->priv = priv;
+		pll->id = i;
+		priv->info->pll_read_configuration(pll);
+	}
+
+	if (of_get_child_count(dn)) {
+		struct device_node *child;
+
+		for_each_available_child_of_node(dn, child) {
+			u32 reg;
+
+			/* PHY subnode name must be 'phy'. */
+			if (!(of_node_name_eq(child, "phy")))
+				continue;
+
+			if (of_property_read_u32(child, "reg", &reg)) {
+				dev_err(dev, "No \"reg\" property for %pOF\n", child);
+				of_node_put(child);
+				return -EINVAL;
+			}
+
+			if (reg < priv->info->first_lane || reg >= priv->info->num_lanes) {
+				dev_err(dev, "\"reg\" property out of range for %pOF\n", child);
+				of_node_put(child);
+				return -EINVAL;
+			}
+
+			err = lynx_probe_lane(priv, reg, child, phy_ops);
+			if (err) {
+				of_node_put(child);
+				return err;
+			}
+		}
+	} else {
+		for (int i = priv->info->first_lane; i < priv->info->num_lanes; i++) {
+			err = lynx_probe_lane(priv, i, NULL, phy_ops);
+			if (err)
+				return err;
+		}
+	}
+
+	provider = devm_of_phy_provider_register(dev, lynx_xlate);
+	if (IS_ERR(provider))
+		return PTR_ERR(provider);
+
+	queue_delayed_work(system_power_efficient_wq, &priv->cdr_check,
+			   msecs_to_jiffies(1000));
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(lynx_probe, "PHY_FSL_LYNX");
+
+void lynx_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct lynx_priv *priv = dev_get_drvdata(dev);
+
+	cancel_delayed_work_sync(&priv->cdr_check);
+}
+EXPORT_SYMBOL_NS_GPL(lynx_remove, "PHY_FSL_LYNX");
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Freescale Lynx SerDes core functionality");
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
index 5cd86c9543cb..e8b280cc9b38 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.h
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
@@ -10,14 +10,15 @@
 
 #define LYNX_NUM_PLL				2
 
+struct lynx_priv;
+struct lynx_lane;
+
 struct lynx_pccr {
 	int offset;
 	int width;
 	int shift;
 };
 
-struct lynx_priv;
-
 struct lynx_pll {
 	struct lynx_priv *priv;
 	int id;
@@ -42,6 +43,9 @@ struct lynx_info {
 			struct lynx_pccr *pccr);
 	int (*get_pcvt_offset)(int lane, enum lynx_lane_mode mode);
 	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
+	void (*pll_read_configuration)(struct lynx_pll *pll);
+	void (*lane_read_configuration)(struct lynx_lane *lane);
+	void (*cdr_lock_check)(struct lynx_lane *lane);
 	int first_lane;
 	int num_lanes;
 };
@@ -85,6 +89,10 @@ static inline void lynx_rmw(struct lynx_priv *priv, unsigned long off, u32 val,
 #define lynx_pll_read(pll, reg)			\
 	ioread32((pll)->priv->base + reg((pll)->id))
 
+int lynx_probe(struct platform_device *pdev, const struct lynx_info *info,
+	       const struct phy_ops *phy_ops);
+void lynx_remove(struct platform_device *pdev);
+
 const char *lynx_lane_mode_str(enum lynx_lane_mode lane_mode);
 enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf);
 bool lynx_lane_supports_mode(struct lynx_lane *lane, enum lynx_lane_mode mode);
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 09/16] phy: lynx-28g: make lynx_28g_pll_read_configuration() callable per PLL
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

In a future change, lynx_28g_pll_read_configuration() and
lynx_28g_lane_read_configuration() will be made methods of struct
lynx_info.

There is no functional reason, but lynx_28g_lane_read_configuration() is
called per lane and lynx_28g_pll_read_configuration() iterates over PLLs
internally. So the API exported by the lynx_info structure would not be
uniform. Change lynx_28g_pll_read_configuration() to also permit reading
the PLL configuration individually, and move the for loop at the call
site.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: none
v1->v2: fix typo in commit message (spotted by Sashiko)
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 73 ++++++++++++------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index a3fbd31d4dbf..dbbadaa08cb6 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -272,7 +272,6 @@
 #define lynx_28g_lane_rmw			lynx_lane_rmw
 #define lynx_28g_lane_read			lynx_lane_read
 #define lynx_28g_lane_write			lynx_lane_write
-#define lynx_28g_pll_read			lynx_pll_read
 
 #define lynx_28g_priv				lynx_priv
 #define lynx_28g_lane				lynx_lane
@@ -1051,49 +1050,41 @@ static const struct phy_ops lynx_28g_ops = {
 	.owner		= THIS_MODULE,
 };
 
-static void lynx_28g_pll_read_configuration(struct lynx_28g_priv *priv)
+static void lynx_28g_pll_read_configuration(struct lynx_pll *pll)
 {
-	struct lynx_28g_pll *pll;
 	u32 val;
-	int i;
 
-	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
-		pll = &priv->pll[i];
-		pll->priv = priv;
-		pll->id = i;
+	val = lynx_pll_read(pll, PLLnRSTCTL);
+	pll->enabled = !(val & PLLnRSTCTL_DIS);
+	pll->locked = !!(val & PLLnRSTCTL_LOCK);
 
-		val = lynx_28g_pll_read(pll, PLLnRSTCTL);
-		pll->enabled = !(val & PLLnRSTCTL_DIS);
-		pll->locked = !!(val & PLLnRSTCTL_LOCK);
+	val = lynx_pll_read(pll, PLLnCR0);
+	pll->refclk_sel = FIELD_GET(PLLnCR0_REFCLK_SEL, val);
 
-		val = lynx_28g_pll_read(pll, PLLnCR0);
-		pll->refclk_sel = FIELD_GET(PLLnCR0_REFCLK_SEL, val);
+	val = lynx_pll_read(pll, PLLnCR1);
+	pll->frate_sel = FIELD_GET(PLLnCR1_FRATE_SEL, val);
 
-		val = lynx_28g_pll_read(pll, PLLnCR1);
-		pll->frate_sel = FIELD_GET(PLLnCR1_FRATE_SEL, val);
-
-		if (!pll->enabled)
-			continue;
+	if (!pll->enabled)
+		return;
 
-		switch (pll->frate_sel) {
-		case PLLnCR1_FRATE_5G_10GVCO:
-		case PLLnCR1_FRATE_5G_25GVCO:
-			/* 5GHz clock net */
-			__set_bit(LANE_MODE_1000BASEX_SGMII, pll->supported);
-			break;
-		case PLLnCR1_FRATE_10G_20GVCO:
-			/* 10.3125GHz clock net */
-			__set_bit(LANE_MODE_10GBASER, pll->supported);
-			__set_bit(LANE_MODE_USXGMII, pll->supported);
-			break;
-		case PLLnCR1_FRATE_12G_25GVCO:
-			/* 12.890625GHz clock net */
-			__set_bit(LANE_MODE_25GBASER, pll->supported);
-			break;
-		default:
-			/* 6GHz, 8GHz */
-			break;
-		}
+	switch (pll->frate_sel) {
+	case PLLnCR1_FRATE_5G_10GVCO:
+	case PLLnCR1_FRATE_5G_25GVCO:
+		/* 5GHz clock net */
+		__set_bit(LANE_MODE_1000BASEX_SGMII, pll->supported);
+		break;
+	case PLLnCR1_FRATE_10G_20GVCO:
+		/* 10.3125GHz clock net */
+		__set_bit(LANE_MODE_10GBASER, pll->supported);
+		__set_bit(LANE_MODE_USXGMII, pll->supported);
+		break;
+	case PLLnCR1_FRATE_12G_25GVCO:
+		/* 12.890625GHz clock net */
+		__set_bit(LANE_MODE_25GBASER, pll->supported);
+		break;
+	default:
+		/* 6GHz, 8GHz */
+		break;
 	}
 }
 
@@ -1291,7 +1282,13 @@ static int lynx_28g_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	lynx_28g_pll_read_configuration(priv);
+	for (int i = 0; i < LYNX_28G_NUM_PLL; i++) {
+		struct lynx_28g_pll *pll = &priv->pll[i];
+
+		pll->priv = priv;
+		pll->id = i;
+		lynx_28g_pll_read_configuration(pll);
+	}
 
 	if (of_get_child_count(dn)) {
 		struct device_node *child;
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 08/16] phy: lynx-28g: move struct lynx_info definitions downwards
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

We need to be able to reference more function pointers in upcoming
patches. The struct lynx_info definitions are currently placed a bit up
in lynx-28g.c in order to be able to do that without function prototype
forward declarations, so move them downward to avoid that situation.

No functional change intended.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: none
v1->v2: adapt to lynx_28g_lane_remap_pll() prototype change in context
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 86 ++++++++++++------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 1de663283faf..a3fbd31d4dbf 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -782,49 +782,6 @@ static bool lynx_28g_compat_lane_supports_mode(int lane,
 	}
 }
 
-static const struct lynx_info lynx_info_compat = {
-	.get_pccr = lynx_28g_get_pccr,
-	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
-	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
-	.num_lanes = LYNX_28G_NUM_LANE,
-};
-
-static const struct lynx_info lynx_info_lx2160a_serdes1 = {
-	.get_pccr = lynx_28g_get_pccr,
-	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
-	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
-	.num_lanes = LYNX_28G_NUM_LANE,
-};
-
-static const struct lynx_info lynx_info_lx2160a_serdes2 = {
-	.get_pccr = lynx_28g_get_pccr,
-	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
-	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
-	.num_lanes = LYNX_28G_NUM_LANE,
-};
-
-static const struct lynx_info lynx_info_lx2160a_serdes3 = {
-	.get_pccr = lynx_28g_get_pccr,
-	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
-	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
-	.num_lanes = LYNX_28G_NUM_LANE,
-};
-
-static const struct lynx_info lynx_info_lx2162a_serdes1 = {
-	.get_pccr = lynx_28g_get_pccr,
-	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
-	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
-	.first_lane = 4,
-	.num_lanes = LYNX_28G_NUM_LANE,
-};
-
-static const struct lynx_info lynx_info_lx2162a_serdes2 = {
-	.get_pccr = lynx_28g_get_pccr,
-	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
-	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
-	.num_lanes = LYNX_28G_NUM_LANE,
-};
-
 static void lynx_28g_lane_remap_pll(struct lynx_lane *lane,
 				    enum lynx_lane_mode lane_mode)
 {
@@ -1248,6 +1205,49 @@ static int lynx_28g_probe_lane(struct lynx_28g_priv *priv, int id,
 	return 0;
 }
 
+static const struct lynx_info lynx_info_compat = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
+	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
+};
+
+static const struct lynx_info lynx_info_lx2160a_serdes1 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
+	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
+};
+
+static const struct lynx_info lynx_info_lx2160a_serdes2 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
+	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
+};
+
+static const struct lynx_info lynx_info_lx2160a_serdes3 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
+	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
+};
+
+static const struct lynx_info lynx_info_lx2162a_serdes1 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
+	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
+	.first_lane = 4,
+	.num_lanes = LYNX_28G_NUM_LANE,
+};
+
+static const struct lynx_info lynx_info_lx2162a_serdes2 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
+	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
+};
+
 static int lynx_28g_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 07/16] phy: lynx-28g: provide default lynx_lane_supports_mode() implementation
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

For the 28G Lynx, there are situations where a protocol is not supported
on a lane despite there being a PCCR register and protocol converter
available:
- LX2160A SerDes 1: reference manual documents PCCD fields E25GC_CFG and
  E25GD_CFG and protocol converter registers E25GCCR1..E25GCCR3 /
  E25GDCR1..E25GDCR3, but nonetheless, Table 289. SerDes 1 protocol
  mapping shows no RCW[SRDS_PRTCL_S1] value for which lanes C and D
  support 25G
- when using the "fsl,lynx-28g" fallback compatible string, we don't
  want to offer 25GbE because we don't know if the lane supports it,
  even though we know how to reach the PCCR and protocol converter
  registers for it.

But for the upcoming 10G Lynx SerDes, the above situations don't exist.
There, if we know how to reach the PCCR and protocol converter
registers on a lane, we implicitly know that the protocol is supported
there, so implementing priv->info->lane_supports_mode() would be
redundant.

Implement lynx_lane_supports_mode_default() which decides whether a lane
mode is supported just based on priv->info->get_pccr() and
priv->info->get_pcvt_offset().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v3: none
---
 drivers/phy/freescale/phy-fsl-lynx-core.c | 27 ++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.c b/drivers/phy/freescale/phy-fsl-lynx-core.c
index f49d594622cb..802e32dc6dca 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.c
@@ -40,6 +40,27 @@ enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf)
 }
 EXPORT_SYMBOL_NS_GPL(phy_interface_to_lane_mode, "PHY_FSL_LYNX");
 
+/* By default, assume that if we know how to get the PCCR register and
+ * protocol converter for a lane, that protocol is supported.
+ */
+static bool lynx_lane_supports_mode_default(struct lynx_lane *lane,
+					    enum lynx_lane_mode mode)
+{
+	struct lynx_priv *priv = lane->priv;
+	struct lynx_pccr pccr;
+
+	if (!priv->info->get_pccr || !priv->info->get_pcvt_offset)
+		return false;
+
+	if (priv->info->get_pccr(mode, lane->id, &pccr) < 0)
+		return false;
+
+	if (priv->info->get_pcvt_offset(lane->id, mode) < 0)
+		return false;
+
+	return true;
+}
+
 /* A lane mode is supported if we have a PLL that can provide its required
  * clock net, and if there is a protocol converter for that mode on that lane.
  */
@@ -48,8 +69,12 @@ bool lynx_lane_supports_mode(struct lynx_lane *lane, enum lynx_lane_mode mode)
 	struct lynx_priv *priv = lane->priv;
 	int i;
 
-	if (!priv->info->lane_supports_mode(lane->id, mode))
+	if (priv->info->lane_supports_mode) {
+		if (!priv->info->lane_supports_mode(lane->id, mode))
+			return false;
+	} else if (!lynx_lane_supports_mode_default(lane, mode)) {
 		return false;
+	}
 
 	for (i = 0; i < LYNX_NUM_PLL; i++) {
 		if (!priv->pll[i].enabled)
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 06/16] phy: lynx-28g: generalize protocol converter accessors
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

The protocol converters on the 10G Lynx are architecturally similar, but
different in layout from the 28G Lynx ones.

Move lynx_pccr_read(), lynx_pccr_write(), lynx_pcvt_read() and
lynx_pcvt_write() from the 28G Lynx driver to the common module, and
permit each SerDes driver to provide just its own bits in order to use
this common API.

Currently, that just means that the direct calls to
lynx_28g_get_pcvt_offset() are modified to go through the
lynx->info->get_pcvt_offset() indirect function call, and similarly,
lynx_28g_get_pccr() through lynx->info->get_pccr().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: none
v1->v2: adapt to lynx_28g_lane_remap_pll() prototype change in context
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c  | 102 +++-------------------
 drivers/phy/freescale/phy-fsl-lynx-core.c |  90 +++++++++++++++++++
 drivers/phy/freescale/phy-fsl-lynx-core.h |  13 +++
 3 files changed, 115 insertions(+), 90 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 5c473d6c233e..1de663283faf 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -269,8 +269,6 @@
 #define LYNX_28G_LANE_STOP_SLEEP_US		100
 #define LYNX_28G_LANE_STOP_TIMEOUT_US		1000000
 
-#define lynx_28g_read				lynx_read
-#define lynx_28g_write				lynx_write
 #define lynx_28g_lane_rmw			lynx_lane_rmw
 #define lynx_28g_lane_read			lynx_lane_read
 #define lynx_28g_lane_write			lynx_lane_write
@@ -785,124 +783,48 @@ static bool lynx_28g_compat_lane_supports_mode(int lane,
 }
 
 static const struct lynx_info lynx_info_compat = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2160a_serdes1 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2160a_serdes2 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2160a_serdes3 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2162a_serdes1 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
 	.first_lane = 4,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2162a_serdes2 = {
+	.get_pccr = lynx_28g_get_pccr,
+	.get_pcvt_offset = lynx_28g_get_pcvt_offset,
 	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
 	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
-static int lynx_pccr_read(struct lynx_28g_lane *lane, enum lynx_lane_mode mode,
-			  u32 *val)
-{
-	struct lynx_28g_priv *priv = lane->priv;
-	struct lynx_pccr pccr;
-	u32 tmp;
-	int err;
-
-	err = lynx_28g_get_pccr(mode, lane->id, &pccr);
-	if (err)
-		return err;
-
-	tmp = lynx_28g_read(priv, pccr.offset);
-	*val = (tmp >> pccr.shift) & GENMASK(pccr.width - 1, 0);
-
-	return 0;
-}
-
-static int lynx_pccr_write(struct lynx_28g_lane *lane,
-			   enum lynx_lane_mode lane_mode, u32 val)
-{
-	struct lynx_28g_priv *priv = lane->priv;
-	struct lynx_pccr pccr;
-	u32 old, tmp, mask;
-	int err;
-
-	err = lynx_28g_get_pccr(lane_mode, lane->id, &pccr);
-	if (err)
-		return err;
-
-	old = lynx_28g_read(priv, pccr.offset);
-	mask = GENMASK(pccr.width - 1, 0) << pccr.shift;
-	tmp = (old & ~mask) | (val << pccr.shift);
-	lynx_28g_write(priv, pccr.offset, tmp);
-
-	dev_dbg(&lane->phy->dev, "PCCR@0x%x: 0x%x -> 0x%x\n",
-		pccr.offset, old, tmp);
-
-	return 0;
-}
-
-static int lynx_pcvt_read(struct lynx_28g_lane *lane,
-			  enum lynx_lane_mode lane_mode, int cr, u32 *val)
-{
-	struct lynx_28g_priv *priv = lane->priv;
-	int offset;
-
-	offset = lynx_28g_get_pcvt_offset(lane->id, lane_mode);
-	if (offset < 0)
-		return offset;
-
-	*val = lynx_28g_read(priv, offset + cr);
-
-	return 0;
-}
-
-static int lynx_pcvt_write(struct lynx_28g_lane *lane,
-			   enum lynx_lane_mode lane_mode, int cr, u32 val)
-{
-	struct lynx_28g_priv *priv = lane->priv;
-	int offset;
-
-	offset = lynx_28g_get_pcvt_offset(lane->id, lane_mode);
-	if (offset < 0)
-		return offset;
-
-	lynx_28g_write(priv, offset + cr, val);
-
-	return 0;
-}
-
-static int lynx_pcvt_rmw(struct lynx_28g_lane *lane,
-			 enum lynx_lane_mode lane_mode,
-			 int cr, u32 val, u32 mask)
-{
-	int err;
-	u32 tmp;
-
-	err = lynx_pcvt_read(lane, lane_mode, cr, &tmp);
-	if (err)
-		return err;
-
-	tmp &= ~mask;
-	tmp |= val;
-
-	return lynx_pcvt_write(lane, lane_mode, cr, tmp);
-}
-
 static void lynx_28g_lane_remap_pll(struct lynx_lane *lane,
 				    enum lynx_lane_mode lane_mode)
 {
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.c b/drivers/phy/freescale/phy-fsl-lynx-core.c
index 5e5bcaa54d09..f49d594622cb 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.c
@@ -87,5 +87,95 @@ struct lynx_pll *lynx_pll_get(struct lynx_priv *priv, enum lynx_lane_mode mode)
 }
 EXPORT_SYMBOL_NS_GPL(lynx_pll_get, "PHY_FSL_LYNX");
 
+int lynx_pccr_read(struct lynx_lane *lane, enum lynx_lane_mode mode, u32 *val)
+{
+	struct lynx_priv *priv = lane->priv;
+	struct lynx_pccr pccr;
+	u32 tmp;
+	int err;
+
+	err = priv->info->get_pccr(mode, lane->id, &pccr);
+	if (err)
+		return err;
+
+	tmp = lynx_read(priv, pccr.offset);
+	*val = (tmp >> pccr.shift) & GENMASK(pccr.width - 1, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(lynx_pccr_read, "PHY_FSL_LYNX");
+
+int lynx_pccr_write(struct lynx_lane *lane, enum lynx_lane_mode mode, u32 val)
+{
+	struct lynx_priv *priv = lane->priv;
+	struct lynx_pccr pccr;
+	u32 old, tmp, mask;
+	int err;
+
+	err = priv->info->get_pccr(mode, lane->id, &pccr);
+	if (err)
+		return err;
+
+	old = lynx_read(priv, pccr.offset);
+	mask = GENMASK(pccr.width - 1, 0) << pccr.shift;
+	tmp = (old & ~mask) | (val << pccr.shift);
+	lynx_write(priv, pccr.offset, tmp);
+
+	dev_dbg(&lane->phy->dev, "PCCR@0x%x: 0x%x -> 0x%x\n",
+		pccr.offset, old, tmp);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(lynx_pccr_write, "PHY_FSL_LYNX");
+
+int lynx_pcvt_read(struct lynx_lane *lane, enum lynx_lane_mode mode, int cr,
+		   u32 *val)
+{
+	struct lynx_priv *priv = lane->priv;
+	int offset;
+
+	offset = priv->info->get_pcvt_offset(lane->id, mode);
+	if (offset < 0)
+		return offset;
+
+	*val = lynx_read(priv, offset + cr);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(lynx_pcvt_read, "PHY_FSL_LYNX");
+
+int lynx_pcvt_write(struct lynx_lane *lane, enum lynx_lane_mode mode, int cr,
+		    u32 val)
+{
+	struct lynx_priv *priv = lane->priv;
+	int offset;
+
+	offset = priv->info->get_pcvt_offset(lane->id, mode);
+	if (offset < 0)
+		return offset;
+
+	lynx_write(priv, offset + cr, val);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(lynx_pcvt_write, "PHY_FSL_LYNX");
+
+int lynx_pcvt_rmw(struct lynx_lane *lane, enum lynx_lane_mode mode, int cr,
+		  u32 val, u32 mask)
+{
+	int err;
+	u32 tmp;
+
+	err = lynx_pcvt_read(lane, mode, cr, &tmp);
+	if (err)
+		return err;
+
+	tmp &= ~mask;
+	tmp |= val;
+
+	return lynx_pcvt_write(lane, mode, cr, tmp);
+}
+EXPORT_SYMBOL_NS_GPL(lynx_pcvt_rmw, "PHY_FSL_LYNX");
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Freescale Lynx SerDes core functionality");
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
index b726ff21972b..5cd86c9543cb 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.h
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
@@ -4,6 +4,7 @@
 #ifndef _PHY_FSL_LYNX_CORE_H
 #define _PHY_FSL_LYNX_CORE_H
 
+#include <linux/phy/phy.h>
 #include <linux/phy.h>
 #include <soc/fsl/phy-fsl-lynx.h>
 
@@ -37,6 +38,9 @@ struct lynx_lane {
 };
 
 struct lynx_info {
+	int (*get_pccr)(enum lynx_lane_mode lane_mode, int lane,
+			struct lynx_pccr *pccr);
+	int (*get_pcvt_offset)(int lane, enum lynx_lane_mode mode);
 	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
 	int first_lane;
 	int num_lanes;
@@ -87,4 +91,13 @@ bool lynx_lane_supports_mode(struct lynx_lane *lane, enum lynx_lane_mode mode);
 
 struct lynx_pll *lynx_pll_get(struct lynx_priv *priv, enum lynx_lane_mode mode);
 
+int lynx_pccr_read(struct lynx_lane *lane, enum lynx_lane_mode mode, u32 *val);
+int lynx_pccr_write(struct lynx_lane *lane, enum lynx_lane_mode mode, u32 val);
+int lynx_pcvt_read(struct lynx_lane *lane, enum lynx_lane_mode mode, int cr,
+		   u32 *val);
+int lynx_pcvt_write(struct lynx_lane *lane, enum lynx_lane_mode mode, int cr,
+		    u32 val);
+int lynx_pcvt_rmw(struct lynx_lane *lane, enum lynx_lane_mode mode, int cr,
+		  u32 val, u32 mask);
+
 #endif
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 05/16] phy: lynx-28g: common lynx_pll_get()
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

The logic should be absolutely unchanged in the new 10G Lynx SerDes
driver, so let's move this to phy-fsl-lynx-core.c and update the 28G
Lynx driver to use the common variant.

While at it, update the call site, lynx_28g_lane_remap_pll(), to use the
new data structures, and refactor the NULL pll pointer check (the
current form triggers a checkpatch CHECK).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: none
v1->v2: add the drive-by refactoring
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c  | 34 ++++-------------------
 drivers/phy/freescale/phy-fsl-lynx-core.c | 24 ++++++++++++++++
 drivers/phy/freescale/phy-fsl-lynx-core.h |  2 ++
 3 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 570fa74daba0..5c473d6c233e 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -467,30 +467,6 @@ static const struct lynx_28g_proto_conf lynx_28g_proto_conf[LANE_MODE_MAX] = {
 	},
 };
 
-static struct lynx_28g_pll *lynx_28g_pll_get(struct lynx_28g_priv *priv,
-					     enum lynx_lane_mode mode)
-{
-	struct lynx_28g_pll *pll;
-	int i;
-
-	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
-		pll = &priv->pll[i];
-
-		if (!pll->enabled)
-			continue;
-
-		if (test_bit(mode, pll->supported))
-			return pll;
-	}
-
-	/* no pll supports requested mode, either caller forgot to check
-	 * lynx_lane_supports_mode(), or this is a bug.
-	 */
-	dev_WARN_ONCE(priv->dev, 1, "no pll for lane mode %s\n",
-		      lynx_lane_mode_str(mode));
-	return NULL;
-}
-
 static void lynx_28g_lane_set_nrate(struct lynx_28g_lane *lane,
 				    struct lynx_28g_pll *pll,
 				    enum lynx_lane_mode lane_mode)
@@ -927,15 +903,15 @@ static int lynx_pcvt_rmw(struct lynx_28g_lane *lane,
 	return lynx_pcvt_write(lane, lane_mode, cr, tmp);
 }
 
-static void lynx_28g_lane_remap_pll(struct lynx_28g_lane *lane,
+static void lynx_28g_lane_remap_pll(struct lynx_lane *lane,
 				    enum lynx_lane_mode lane_mode)
 {
-	struct lynx_28g_priv *priv = lane->priv;
-	struct lynx_28g_pll *pll;
+	struct lynx_priv *priv = lane->priv;
+	struct lynx_pll *pll;
 
 	/* Switch to the PLL that works with this interface type */
-	pll = lynx_28g_pll_get(priv, lane_mode);
-	if (unlikely(pll == NULL))
+	pll = lynx_pll_get(priv, lane_mode);
+	if (unlikely(!pll))
 		return;
 
 	lynx_28g_lane_set_pll(lane, pll);
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.c b/drivers/phy/freescale/phy-fsl-lynx-core.c
index de45b14d3fb6..5e5bcaa54d09 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.c
@@ -63,5 +63,29 @@ bool lynx_lane_supports_mode(struct lynx_lane *lane, enum lynx_lane_mode mode)
 }
 EXPORT_SYMBOL_NS_GPL(lynx_lane_supports_mode, "PHY_FSL_LYNX");
 
+struct lynx_pll *lynx_pll_get(struct lynx_priv *priv, enum lynx_lane_mode mode)
+{
+	struct lynx_pll *pll;
+	int i;
+
+	for (i = 0; i < LYNX_NUM_PLL; i++) {
+		pll = &priv->pll[i];
+
+		if (!pll->enabled)
+			continue;
+
+		if (test_bit(mode, pll->supported))
+			return pll;
+	}
+
+	/* no pll supports requested mode, either caller forgot to check
+	 * lynx_lane_supports_mode(), or this is a bug.
+	 */
+	dev_WARN_ONCE(priv->dev, 1, "no pll for lane mode %s\n",
+		      lynx_lane_mode_str(mode));
+	return NULL;
+}
+EXPORT_SYMBOL_NS_GPL(lynx_pll_get, "PHY_FSL_LYNX");
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Freescale Lynx SerDes core functionality");
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
index f0cb3e805235..b726ff21972b 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.h
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
@@ -85,4 +85,6 @@ const char *lynx_lane_mode_str(enum lynx_lane_mode lane_mode);
 enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf);
 bool lynx_lane_supports_mode(struct lynx_lane *lane, enum lynx_lane_mode mode);
 
+struct lynx_pll *lynx_pll_get(struct lynx_priv *priv, enum lynx_lane_mode mode);
+
 #endif
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 04/16] phy: lynx-28g: move data structures to core
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

The goal is to avoid duplicating the core data structures when
introducing the new lynx-10g driver.

We move the following to phy-fsl-lynx-core:
- struct lynx_28g_pll -> struct lynx_pll. This has some
  hardware-specific register fields which need to become hardware
  agnostic (the PLL register layout is different for Lynx 10G), So:
  - PLLnRSTCTL_DIS(pll->rstctl) becomes !pll->enabled
  - PLLnRSTCTL_LOCK(pll->rstctl) becomes pll->locked
  - FIELD_GET(PLLnCR1_FRATE_SEL, pll->cr1) becomes pll->frate_sel
  - FIELD_GET(PLLnCR0_REFCLK_SEL, pll->cr0) becomes pll->refclk_sel
- struct lynx_28g_lane -> struct lynx_lane
- struct lynx_28g_priv -> struct lynx_priv
  - field lane[LYNX_28G_NUM_LANE] has to be dynamically allocated. Not
    all Lynx 10G SerDes blocks have 8 lanes.
- LYNX_28G_NUM_PLL -> LYNX_NUM_PLL. This is an architectural constant
  which is the same for Lynx 10G as well.

To avoid major noise in the lynx-28g driver, we keep compatibility shims
(for now) where the old lynx_28g names are preserved, but translate to
the common data structures.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v3: none
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c  | 135 +++++++---------------
 drivers/phy/freescale/phy-fsl-lynx-core.c |  23 ++++
 drivers/phy/freescale/phy-fsl-lynx-core.h |  64 ++++++++++
 3 files changed, 129 insertions(+), 93 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index d3f49d96340f..570fa74daba0 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -12,7 +12,7 @@
 #include "phy-fsl-lynx-core.h"
 
 #define LYNX_28G_NUM_LANE			8
-#define LYNX_28G_NUM_PLL			2
+#define LYNX_28G_NUM_PLL			LYNX_NUM_PLL
 
 /* SoC IP wrapper for protocol converters */
 #define PCC8					0x10a0
@@ -43,8 +43,8 @@
 
 /* Per PLL registers */
 #define PLLnRSTCTL(pll)				(0x400 + (pll) * 0x100 + 0x0)
-#define PLLnRSTCTL_DIS(rstctl)			(((rstctl) & BIT(24)) >> 24)
-#define PLLnRSTCTL_LOCK(rstctl)			(((rstctl) & BIT(23)) >> 23)
+#define PLLnRSTCTL_DIS				BIT(24)
+#define PLLnRSTCTL_LOCK				BIT(23)
 
 #define PLLnCR0(pll)				(0x400 + (pll) * 0x100 + 0x4)
 #define PLLnCR0_REFCLK_SEL			GENMASK(20, 16)
@@ -269,6 +269,17 @@
 #define LYNX_28G_LANE_STOP_SLEEP_US		100
 #define LYNX_28G_LANE_STOP_TIMEOUT_US		1000000
 
+#define lynx_28g_read				lynx_read
+#define lynx_28g_write				lynx_write
+#define lynx_28g_lane_rmw			lynx_lane_rmw
+#define lynx_28g_lane_read			lynx_lane_read
+#define lynx_28g_lane_write			lynx_lane_write
+#define lynx_28g_pll_read			lynx_pll_read
+
+#define lynx_28g_priv				lynx_priv
+#define lynx_28g_lane				lynx_lane
+#define lynx_28g_pll				lynx_pll
+
 enum lynx_28g_eq_type {
 	EQ_TYPE_NO_EQ = 0,
 	EQ_TYPE_2TAP = 1,
@@ -456,86 +467,6 @@ static const struct lynx_28g_proto_conf lynx_28g_proto_conf[LANE_MODE_MAX] = {
 	},
 };
 
-struct lynx_28g_priv;
-
-struct lynx_28g_pll {
-	struct lynx_28g_priv *priv;
-	u32 rstctl, cr0, cr1;
-	int id;
-	DECLARE_BITMAP(supported, LANE_MODE_MAX);
-};
-
-struct lynx_28g_lane {
-	struct lynx_28g_priv *priv;
-	struct phy *phy;
-	bool powered_up;
-	bool init;
-	unsigned int id;
-	enum lynx_lane_mode mode;
-};
-
-struct lynx_28g_priv {
-	void __iomem *base;
-	struct device *dev;
-	const struct lynx_info *info;
-	/* Serialize concurrent access to registers shared between lanes,
-	 * like PCCn
-	 */
-	spinlock_t pcc_lock;
-	struct lynx_28g_pll pll[LYNX_28G_NUM_PLL];
-	struct lynx_28g_lane lane[LYNX_28G_NUM_LANE];
-
-	struct delayed_work cdr_check;
-};
-
-static void lynx_28g_rmw(struct lynx_28g_priv *priv, unsigned long off,
-			 u32 val, u32 mask)
-{
-	void __iomem *reg = priv->base + off;
-	u32 orig, tmp;
-
-	orig = ioread32(reg);
-	tmp = orig & ~mask;
-	tmp |= val;
-	iowrite32(tmp, reg);
-}
-
-#define lynx_28g_read(priv, off) \
-	ioread32((priv)->base + (off))
-#define lynx_28g_write(priv, off, val) \
-	iowrite32(val, (priv)->base + (off))
-#define lynx_28g_lane_rmw(lane, reg, val, mask)	\
-	lynx_28g_rmw((lane)->priv, reg(lane->id), val, mask)
-#define lynx_28g_lane_read(lane, reg)			\
-	ioread32((lane)->priv->base + reg((lane)->id))
-#define lynx_28g_lane_write(lane, reg, val)		\
-	iowrite32(val, (lane)->priv->base + reg((lane)->id))
-#define lynx_28g_pll_read(pll, reg)			\
-	ioread32((pll)->priv->base + reg((pll)->id))
-
-/* A lane mode is supported if we have a PLL that can provide its required
- * clock net, and if there is a protocol converter for that mode on that lane.
- */
-static bool lynx_28g_supports_lane_mode(struct lynx_28g_lane *lane,
-					enum lynx_lane_mode mode)
-{
-	struct lynx_28g_priv *priv = lane->priv;
-	int i;
-
-	if (!priv->info->lane_supports_mode(lane->id, mode))
-		return false;
-
-	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
-		if (PLLnRSTCTL_DIS(priv->pll[i].rstctl))
-			continue;
-
-		if (test_bit(mode, priv->pll[i].supported))
-			return true;
-	}
-
-	return false;
-}
-
 static struct lynx_28g_pll *lynx_28g_pll_get(struct lynx_28g_priv *priv,
 					     enum lynx_lane_mode mode)
 {
@@ -545,7 +476,7 @@ static struct lynx_28g_pll *lynx_28g_pll_get(struct lynx_28g_priv *priv,
 	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
 		pll = &priv->pll[i];
 
-		if (PLLnRSTCTL_DIS(pll->rstctl))
+		if (!pll->enabled)
 			continue;
 
 		if (test_bit(mode, pll->supported))
@@ -553,7 +484,7 @@ static struct lynx_28g_pll *lynx_28g_pll_get(struct lynx_28g_priv *priv,
 	}
 
 	/* no pll supports requested mode, either caller forgot to check
-	 * lynx_28g_supports_lane_mode, or this is a bug.
+	 * lynx_lane_supports_mode(), or this is a bug.
 	 */
 	dev_WARN_ONCE(priv->dev, 1, "no pll for lane mode %s\n",
 		      lynx_lane_mode_str(mode));
@@ -564,7 +495,7 @@ static void lynx_28g_lane_set_nrate(struct lynx_28g_lane *lane,
 				    struct lynx_28g_pll *pll,
 				    enum lynx_lane_mode lane_mode)
 {
-	switch (FIELD_GET(PLLnCR1_FRATE_SEL, pll->cr1)) {
+	switch (pll->frate_sel) {
 	case PLLnCR1_FRATE_5G_10GVCO:
 	case PLLnCR1_FRATE_5G_25GVCO:
 		switch (lane_mode) {
@@ -879,27 +810,33 @@ static bool lynx_28g_compat_lane_supports_mode(int lane,
 
 static const struct lynx_info lynx_info_compat = {
 	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2160a_serdes1 = {
 	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2160a_serdes2 = {
 	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2160a_serdes3 = {
 	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2162a_serdes1 = {
 	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
 	.first_lane = 4,
+	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static const struct lynx_info lynx_info_lx2162a_serdes2 = {
 	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
+	.num_lanes = LYNX_28G_NUM_LANE,
 };
 
 static int lynx_pccr_read(struct lynx_28g_lane *lane, enum lynx_lane_mode mode,
@@ -1168,7 +1105,7 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 		return -EOPNOTSUPP;
 
 	lane_mode = phy_interface_to_lane_mode(submode);
-	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
+	if (!lynx_lane_supports_mode(lane, lane_mode))
 		return -EOPNOTSUPP;
 
 	if (lane_mode == lane->mode)
@@ -1210,7 +1147,7 @@ static int lynx_28g_validate(struct phy *phy, enum phy_mode mode, int submode,
 		return -EOPNOTSUPP;
 
 	lane_mode = phy_interface_to_lane_mode(submode);
-	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
+	if (!lynx_lane_supports_mode(lane, lane_mode))
 		return -EOPNOTSUPP;
 
 	return 0;
@@ -1262,6 +1199,7 @@ static const struct phy_ops lynx_28g_ops = {
 static void lynx_28g_pll_read_configuration(struct lynx_28g_priv *priv)
 {
 	struct lynx_28g_pll *pll;
+	u32 val;
 	int i;
 
 	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
@@ -1269,14 +1207,20 @@ static void lynx_28g_pll_read_configuration(struct lynx_28g_priv *priv)
 		pll->priv = priv;
 		pll->id = i;
 
-		pll->rstctl = lynx_28g_pll_read(pll, PLLnRSTCTL);
-		pll->cr0 = lynx_28g_pll_read(pll, PLLnCR0);
-		pll->cr1 = lynx_28g_pll_read(pll, PLLnCR1);
+		val = lynx_28g_pll_read(pll, PLLnRSTCTL);
+		pll->enabled = !(val & PLLnRSTCTL_DIS);
+		pll->locked = !!(val & PLLnRSTCTL_LOCK);
 
-		if (PLLnRSTCTL_DIS(pll->rstctl))
+		val = lynx_28g_pll_read(pll, PLLnCR0);
+		pll->refclk_sel = FIELD_GET(PLLnCR0_REFCLK_SEL, val);
+
+		val = lynx_28g_pll_read(pll, PLLnCR1);
+		pll->frate_sel = FIELD_GET(PLLnCR1_FRATE_SEL, val);
+
+		if (!pll->enabled)
 			continue;
 
-		switch (FIELD_GET(PLLnCR1_FRATE_SEL, pll->cr1)) {
+		switch (pll->frate_sel) {
 		case PLLnCR1_FRATE_5G_10GVCO:
 		case PLLnCR1_FRATE_5G_25GVCO:
 			/* 5GHz clock net */
@@ -1440,6 +1384,11 @@ static int lynx_28g_probe(struct platform_device *pdev)
 	if (priv->info == &lynx_info_compat)
 		dev_warn(dev, "Please update device tree to use per-device compatible strings\n");
 
+	priv->lane = devm_kcalloc(dev, priv->info->num_lanes,
+				  sizeof(*priv->lane), GFP_KERNEL);
+	if (!priv->lane)
+		return -ENOMEM;
+
 	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.c b/drivers/phy/freescale/phy-fsl-lynx-core.c
index d56f189c162d..de45b14d3fb6 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.c
@@ -40,5 +40,28 @@ enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf)
 }
 EXPORT_SYMBOL_NS_GPL(phy_interface_to_lane_mode, "PHY_FSL_LYNX");
 
+/* A lane mode is supported if we have a PLL that can provide its required
+ * clock net, and if there is a protocol converter for that mode on that lane.
+ */
+bool lynx_lane_supports_mode(struct lynx_lane *lane, enum lynx_lane_mode mode)
+{
+	struct lynx_priv *priv = lane->priv;
+	int i;
+
+	if (!priv->info->lane_supports_mode(lane->id, mode))
+		return false;
+
+	for (i = 0; i < LYNX_NUM_PLL; i++) {
+		if (!priv->pll[i].enabled)
+			continue;
+
+		if (test_bit(mode, priv->pll[i].supported))
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_NS_GPL(lynx_lane_supports_mode, "PHY_FSL_LYNX");
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Freescale Lynx SerDes core functionality");
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
index fe15986482b0..f0cb3e805235 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-core.h
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
@@ -7,18 +7,82 @@
 #include <linux/phy.h>
 #include <soc/fsl/phy-fsl-lynx.h>
 
+#define LYNX_NUM_PLL				2
+
 struct lynx_pccr {
 	int offset;
 	int width;
 	int shift;
 };
 
+struct lynx_priv;
+
+struct lynx_pll {
+	struct lynx_priv *priv;
+	int id;
+	int refclk_sel;
+	int frate_sel;
+	bool enabled;
+	bool locked;
+	DECLARE_BITMAP(supported, LANE_MODE_MAX);
+};
+
+struct lynx_lane {
+	struct lynx_priv *priv;
+	struct phy *phy;
+	bool powered_up;
+	bool init;
+	unsigned int id;
+	enum lynx_lane_mode mode;
+};
+
 struct lynx_info {
 	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
 	int first_lane;
+	int num_lanes;
 };
 
+struct lynx_priv {
+	void __iomem *base;
+	struct device *dev;
+	const struct lynx_info *info;
+	/* Serialize concurrent access to registers shared between lanes,
+	 * like PCCn
+	 */
+	spinlock_t pcc_lock;
+	struct lynx_pll pll[LYNX_NUM_PLL];
+	struct lynx_lane *lane;
+
+	struct delayed_work cdr_check;
+};
+
+static inline void lynx_rmw(struct lynx_priv *priv, unsigned long off, u32 val,
+			    u32 mask)
+{
+	void __iomem *reg = priv->base + off;
+	u32 orig, tmp;
+
+	orig = ioread32(reg);
+	tmp = orig & ~mask;
+	tmp |= val;
+	iowrite32(tmp, reg);
+}
+
+#define lynx_read(priv, off) \
+	ioread32((priv)->base + (off))
+#define lynx_write(priv, off, val) \
+	iowrite32(val, (priv)->base + (off))
+#define lynx_lane_rmw(lane, reg, val, mask)	\
+	lynx_rmw((lane)->priv, reg(lane->id), val, mask)
+#define lynx_lane_read(lane, reg)			\
+	ioread32((lane)->priv->base + reg((lane)->id))
+#define lynx_lane_write(lane, reg, val)		\
+	iowrite32(val, (lane)->priv->base + reg((lane)->id))
+#define lynx_pll_read(pll, reg)			\
+	ioread32((pll)->priv->base + reg((pll)->id))
+
 const char *lynx_lane_mode_str(enum lynx_lane_mode lane_mode);
 enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf);
+bool lynx_lane_supports_mode(struct lynx_lane *lane, enum lynx_lane_mode mode);
 
 #endif
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 03/16] phy: lynx-28g: move lane mode helpers to new core module
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

Do some preparation work for the introduction of the lynx-10g driver,
which will share a common backbone with the 28G Lynx SerDes.

This is just trivial stuff which can be moved without any surgery, and
is easy to follow but otherwise pollutes more serious changes.

The lane modes themselves are exported to a public header, because on
the 10G Lynx, the hardware requires implementing a procedure called
"RCW override". This requires coordination with drivers/soc/fsl/guts.c
to tell it that a SerDes lane needs to be switched to a different
protocol (enum lynx_lane_mode).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: remove help text for hidden Kconfig symbol
v1->v2: none
---
 drivers/phy/freescale/Kconfig             |  4 ++
 drivers/phy/freescale/Makefile            |  1 +
 drivers/phy/freescale/phy-fsl-lynx-28g.c  | 56 ++---------------------
 drivers/phy/freescale/phy-fsl-lynx-core.c | 44 ++++++++++++++++++
 drivers/phy/freescale/phy-fsl-lynx-core.h | 24 ++++++++++
 include/soc/fsl/phy-fsl-lynx.h            | 16 +++++++
 6 files changed, 92 insertions(+), 53 deletions(-)
 create mode 100644 drivers/phy/freescale/phy-fsl-lynx-core.c
 create mode 100644 drivers/phy/freescale/phy-fsl-lynx-core.h
 create mode 100644 include/soc/fsl/phy-fsl-lynx.h

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index 81f53564ee15..ac575d531db7 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -51,11 +51,15 @@ config PHY_FSL_SAMSUNG_HDMI_PHY
 	  Enable this to add support for the Samsung HDMI PHY in i.MX8MP.
 endif
 
+config PHY_FSL_LYNX_CORE
+	tristate
+
 config PHY_FSL_LYNX_28G
 	tristate "Freescale Layerscape Lynx 28G SerDes PHY support"
 	depends on OF
 	depends on ARCH_LAYERSCAPE || COMPILE_TEST
 	select GENERIC_PHY
+	select PHY_FSL_LYNX_CORE
 	help
 	  Enable this to add support for the Lynx SerDes 28G PHY as
 	  found on NXP's Layerscape platforms such as LX2160A.
diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index 658eac7d0a62..d7aa62cdeb39 100644
--- a/drivers/phy/freescale/Makefile
+++ b/drivers/phy/freescale/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_PHY_MIXEL_LVDS_PHY)	+= phy-fsl-imx8qm-lvds-phy.o
 obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)	+= phy-fsl-imx8-mipi-dphy.o
 obj-$(CONFIG_PHY_FSL_IMX8M_PCIE)	+= phy-fsl-imx8m-pcie.o
 obj-$(CONFIG_PHY_FSL_IMX8QM_HSIO)	+= phy-fsl-imx8qm-hsio.o
+obj-$(CONFIG_PHY_FSL_LYNX_CORE)		+= phy-fsl-lynx-core.o
 obj-$(CONFIG_PHY_FSL_LYNX_28G)		+= phy-fsl-lynx-28g.o
 obj-$(CONFIG_PHY_FSL_SAMSUNG_HDMI_PHY)	+= phy-fsl-samsung-hdmi.o
diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 1f5cb02931f5..d3f49d96340f 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -9,6 +9,8 @@
 #include <linux/platform_device.h>
 #include <linux/workqueue.h>
 
+#include "phy-fsl-lynx-core.h"
+
 #define LYNX_28G_NUM_LANE			8
 #define LYNX_28G_NUM_PLL			2
 
@@ -282,15 +284,6 @@ enum lynx_28g_proto_sel {
 	PROTO_SEL_25G_50G_100G = 0x1a,
 };
 
-enum lynx_lane_mode {
-	LANE_MODE_UNKNOWN,
-	LANE_MODE_1000BASEX_SGMII,
-	LANE_MODE_10GBASER,
-	LANE_MODE_USXGMII,
-	LANE_MODE_25GBASER,
-	LANE_MODE_MAX,
-};
-
 struct lynx_28g_proto_conf {
 	/* LNaGCR0 */
 	int proto_sel;
@@ -463,12 +456,6 @@ static const struct lynx_28g_proto_conf lynx_28g_proto_conf[LANE_MODE_MAX] = {
 	},
 };
 
-struct lynx_pccr {
-	int offset;
-	int width;
-	int shift;
-};
-
 struct lynx_28g_priv;
 
 struct lynx_28g_pll {
@@ -487,11 +474,6 @@ struct lynx_28g_lane {
 	enum lynx_lane_mode mode;
 };
 
-struct lynx_info {
-	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
-	int first_lane;
-};
-
 struct lynx_28g_priv {
 	void __iomem *base;
 	struct device *dev;
@@ -531,39 +513,6 @@ static void lynx_28g_rmw(struct lynx_28g_priv *priv, unsigned long off,
 #define lynx_28g_pll_read(pll, reg)			\
 	ioread32((pll)->priv->base + reg((pll)->id))
 
-static const char *lynx_lane_mode_str(enum lynx_lane_mode lane_mode)
-{
-	switch (lane_mode) {
-	case LANE_MODE_1000BASEX_SGMII:
-		return "1000Base-X/SGMII";
-	case LANE_MODE_10GBASER:
-		return "10GBase-R";
-	case LANE_MODE_USXGMII:
-		return "USXGMII";
-	case LANE_MODE_25GBASER:
-		return "25GBase-R";
-	default:
-		return "unknown";
-	}
-}
-
-static enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf)
-{
-	switch (intf) {
-	case PHY_INTERFACE_MODE_SGMII:
-	case PHY_INTERFACE_MODE_1000BASEX:
-		return LANE_MODE_1000BASEX_SGMII;
-	case PHY_INTERFACE_MODE_10GBASER:
-		return LANE_MODE_10GBASER;
-	case PHY_INTERFACE_MODE_USXGMII:
-		return LANE_MODE_USXGMII;
-	case PHY_INTERFACE_MODE_25GBASER:
-		return LANE_MODE_25GBASER;
-	default:
-		return LANE_MODE_UNKNOWN;
-	}
-}
-
 /* A lane mode is supported if we have a PLL that can provide its required
  * clock net, and if there is a protocol converter for that mode on that lane.
  */
@@ -1572,6 +1521,7 @@ static struct platform_driver lynx_28g_driver = {
 };
 module_platform_driver(lynx_28g_driver);
 
+MODULE_IMPORT_NS("PHY_FSL_LYNX");
 MODULE_AUTHOR("Ioana Ciornei <ioana.ciornei@nxp.com>");
 MODULE_DESCRIPTION("Lynx 28G SerDes PHY driver for Layerscape SoCs");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.c b/drivers/phy/freescale/phy-fsl-lynx-core.c
new file mode 100644
index 000000000000..d56f189c162d
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright 2025-2026 NXP */
+
+#include <linux/module.h>
+
+#include "phy-fsl-lynx-core.h"
+
+const char *lynx_lane_mode_str(enum lynx_lane_mode lane_mode)
+{
+	switch (lane_mode) {
+	case LANE_MODE_1000BASEX_SGMII:
+		return "1000Base-X/SGMII";
+	case LANE_MODE_10GBASER:
+		return "10GBase-R";
+	case LANE_MODE_USXGMII:
+		return "USXGMII";
+	case LANE_MODE_25GBASER:
+		return "25GBase-R";
+	default:
+		return "unknown";
+	}
+}
+EXPORT_SYMBOL_NS_GPL(lynx_lane_mode_str, "PHY_FSL_LYNX");
+
+enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf)
+{
+	switch (intf) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
+		return LANE_MODE_1000BASEX_SGMII;
+	case PHY_INTERFACE_MODE_10GBASER:
+		return LANE_MODE_10GBASER;
+	case PHY_INTERFACE_MODE_USXGMII:
+		return LANE_MODE_USXGMII;
+	case PHY_INTERFACE_MODE_25GBASER:
+		return LANE_MODE_25GBASER;
+	default:
+		return LANE_MODE_UNKNOWN;
+	}
+}
+EXPORT_SYMBOL_NS_GPL(phy_interface_to_lane_mode, "PHY_FSL_LYNX");
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Freescale Lynx SerDes core functionality");
diff --git a/drivers/phy/freescale/phy-fsl-lynx-core.h b/drivers/phy/freescale/phy-fsl-lynx-core.h
new file mode 100644
index 000000000000..fe15986482b0
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-lynx-core.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright 2025-2026 NXP */
+
+#ifndef _PHY_FSL_LYNX_CORE_H
+#define _PHY_FSL_LYNX_CORE_H
+
+#include <linux/phy.h>
+#include <soc/fsl/phy-fsl-lynx.h>
+
+struct lynx_pccr {
+	int offset;
+	int width;
+	int shift;
+};
+
+struct lynx_info {
+	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
+	int first_lane;
+};
+
+const char *lynx_lane_mode_str(enum lynx_lane_mode lane_mode);
+enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf);
+
+#endif
diff --git a/include/soc/fsl/phy-fsl-lynx.h b/include/soc/fsl/phy-fsl-lynx.h
new file mode 100644
index 000000000000..92e8272d5ae1
--- /dev/null
+++ b/include/soc/fsl/phy-fsl-lynx.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright 2023-2026 NXP */
+
+#ifndef __PHY_FSL_LYNX_H_
+#define __PHY_FSL_LYNX_H_
+
+enum lynx_lane_mode {
+	LANE_MODE_UNKNOWN,
+	LANE_MODE_1000BASEX_SGMII,
+	LANE_MODE_10GBASER,
+	LANE_MODE_USXGMII,
+	LANE_MODE_25GBASER,
+	LANE_MODE_MAX,
+};
+
+#endif /* __PHY_FSL_LYNX_H_ */
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 02/16] phy: lynx-28g: reject probing on devices with unsupported OF nodes
From: Vladimir Oltean @ 2026-06-03 13:19 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

It is possible to bind the lynx-28g driver to an arbitrary device with
an OF node, using the driver_override mechanism that is available for
the platform bus, and trigger a crash this way:

$ echo 1ea0000.serdes > /sys/bus/platform/drivers/lynx-10g/unbind
$ echo lynx-28g > /sys/bus/platform/devices/1ea0000.serdes/driver_override
$ echo 1ea0000.serdes > /sys/bus/platform/drivers/lynx-28g/bind
Internal error: Oops: 0000000096000004 [#1]  SMP
Hardware name: LS1028A RDB Board (DT)
pc : lynx_probe+0x118/0x4fc
lr : lynx_probe+0x110/0x4fc
Call trace:
 lynx_probe+0x118/0x4fc (P)
 lynx_28g_probe+0x54/0x7c
 platform_probe+0x68/0xa4
 really_probe+0x14c/0x2ec
 __driver_probe_device+0xc8/0x170
 device_driver_attach+0x58/0xa8
 bind_store+0xd8/0x118
 drv_attr_store+0x24/0x38

The crash is caused by the fact that of_device_get_match_data() returns
NULL (the bound device has a different compatible string) and this is
not checked.

There was a previous attempt to avoid this in commit c9d80e861034 ("phy:
lynx-28g: require an OF node to probe"), but the mechanism was not fully
understood and it only covered the case where the driver was bound to a
device with no OF node.

The issue was found during Sashiko review. Elevated privilege is
required to override the driver for a device, so the real life impact of
the issue should not be very high.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: none
v1->v2: patch is new
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index cacc128dc96a..1f5cb02931f5 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -1477,6 +1477,9 @@ static int lynx_28g_probe(struct platform_device *pdev)
 
 	priv->dev = dev;
 	priv->info = of_device_get_match_data(dev);
+	if (!priv->info)
+		return -ENODEV;
+
 	dev_set_drvdata(dev, priv);
 	spin_lock_init(&priv->pcc_lock);
 	INIT_DELAYED_WORK(&priv->cdr_check, lynx_28g_cdr_lock_check);
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 01/16] phy: lynx-28g: avoid returning NULL in of_xlate() function
From: Vladimir Oltean @ 2026-06-03 13:18 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel
In-Reply-To: <20260603131914.503053-1-vladimir.oltean@nxp.com>

Sashiko points out that _of_phy_get() does not support a NULL returned
output from phy_provider->of_xlate(), just a valid pointer or a
pointer-encoded error.

When lynx_28g_probe() -> for_each_available_child_of_node() skips
over lanes which have OF nodes with status = "disabled", the
priv->lane[idx].phy pointer will remain NULL.

This NULL pointer may be propagated to lynx_28g_xlate() if the device
tree contains a phandle to the disabled lane AND fw_devlink did not
block probing for the consumer. In that case, the PHY core will crash
when trying to dereference the NULL phy pointer.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3: patch is new
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 92bfc5f65e0b..cacc128dc96a 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -1435,7 +1435,7 @@ static struct phy *lynx_28g_xlate(struct device *dev,
 		    idx < priv->info->first_lane))
 		return ERR_PTR(-EINVAL);
 
-	return priv->lane[idx].phy;
+	return priv->lane[idx].phy ?: ERR_PTR(-ENODEV);
 }
 
 static int lynx_28g_probe_lane(struct lynx_28g_priv *priv, int id,
-- 
2.34.1


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

^ permalink raw reply related

* [PATCH v3 phy-next 00/16] New Generic PHY driver for Lynx 10G SerDes
From: Vladimir Oltean @ 2026-06-03 13:18 UTC (permalink / raw)
  To: linux-phy
  Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
	linux-kernel

The 10G Lynx SerDes is present on NXP LS1028A, LS1046A, LS1088A,
LS2088A and other SoCs (unsupported here). It is an older generation of
the 28G Lynx present in LX2160A and LX2162A.

Modify the Generic PHY driver for lynx-28g to create a common portion
(lynx-common) and add a new driver for lynx-10g and DT bindings.

Main use case is networking - dynamic SerDes protocol changing - but
initial support is limited to minor changes (1GbE <-> 2.5GbE) due to
lack of support for the RCW override procedure necessary for major
protocol changes (1GbE <-> 10GbE). This is the next step once the base
driver is available.

Full change log in individual patches, summary below.

Changes since v2:
- 1 new patch (01/16) made in response to Sashiko to fix an existing
  issue
- fix lynx_10g_power_on() procedure
- include <linux/of.h> instead of <linux/of_device.h>
- fix build warning introduced in v2 in lynx_10g_lane_set_nrate()
- move fsl,lynx-10g compatible comment to commit message from schema
  property description
- make big-endian required for LS1046A in schema
- remove help text for hidden Kconfig symbol

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

Changes since v1:
- 2 new patches (01/15 and 12/15) made in response to Sashiko
- minor drive-by refactoring in patch 04/15
- fix typo in commit message of 08/15 (Sashiko)
- minor cleanups in lynx-10g driver (14/15)

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

Vladimir Oltean (16):
  phy: lynx-28g: avoid returning NULL in of_xlate() function
  phy: lynx-28g: reject probing on devices with unsupported OF nodes
  phy: lynx-28g: move lane mode helpers to new core module
  phy: lynx-28g: move data structures to core
  phy: lynx-28g: common lynx_pll_get()
  phy: lynx-28g: generalize protocol converter accessors
  phy: lynx-28g: provide default lynx_lane_supports_mode()
    implementation
  phy: lynx-28g: move struct lynx_info definitions downwards
  phy: lynx-28g: make lynx_28g_pll_read_configuration() callable per PLL
  phy: lynx-28g: common probe() and remove()
  phy: lynx-28g: add support for big endian register maps
  phy: lynx-28g: optimize read-modify-write operation
  phy: lynx-28g: improve phy_validate() procedure
  dt-bindings: phy: lynx-10g: initial document
  phy: lynx-10g: new driver
  MAINTAINERS: expand Lynx 28G entry to cover Lynx 10G SerDes

 .../devicetree/bindings/phy/fsl,lynx-10g.yaml |  136 ++
 MAINTAINERS                                   |    8 +-
 drivers/phy/freescale/Kconfig                 |   14 +
 drivers/phy/freescale/Makefile                |    2 +
 drivers/phy/freescale/phy-fsl-lynx-10g.c      | 1278 +++++++++++++++++
 drivers/phy/freescale/phy-fsl-lynx-28g.c      |  622 ++------
 drivers/phy/freescale/phy-fsl-lynx-core.c     |  445 ++++++
 drivers/phy/freescale/phy-fsl-lynx-core.h     |  134 ++
 include/soc/fsl/phy-fsl-lynx.h                |   43 +
 9 files changed, 2195 insertions(+), 487 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
 create mode 100644 drivers/phy/freescale/phy-fsl-lynx-10g.c
 create mode 100644 drivers/phy/freescale/phy-fsl-lynx-core.c
 create mode 100644 drivers/phy/freescale/phy-fsl-lynx-core.h
 create mode 100644 include/soc/fsl/phy-fsl-lynx.h

-- 
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 v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-06-03 12:57 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
	devicetree, linux-kernel
In-Reply-To: <v4vz7cistjb2iuzha4oykglar7duw4y2uuyhumzs33yvpwrxcu@i5tsg4uzpuwc>

On 03/06/2026 13:40, Dmitry Baryshkov wrote:
>> Are you sure about that ?
> Yes.
> 
>> ipcat I thought designated lane 7 specifically as clk-lane i.e. named it
>> CLK_LN of some description.
> Split configurations explicitly use other lanes for clocks. E.g. check
> the RB5 Navigation schematics, CAM0B connector.

Can you please check:

CSI_3PHASE_COMMON.CSI_COMMON_CTRL5

0 LN0_PWRDN_B Lane 0
...
7 LNCK_PWRDN_B Clock Lane

... just a badly name field

CSI_2PHASE_CTRL10

Bit[2] = IS_CLKLANE

Right so CSI_2PHASE_CTRL10 controls lane mode, indeed. Thanks for checking.

---
bod

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

^ permalink raw reply

* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Dmitry Baryshkov @ 2026-06-03 12:40 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
	devicetree, linux-kernel
In-Reply-To: <f6c91099-0002-4580-a5e8-5611b089024b@linaro.org>

On Wed, Jun 03, 2026 at 01:22:11PM +0100, Bryan O'Donoghue wrote:
> On 03/06/2026 13:10, Dmitry Baryshkov wrote:
> > > Documentation shows clock lane at lane 7.
> > > 
> > > Truthfully it makes no sense that the clock lane would genuinely be locked
> > > to lane 7 but the documentation does seem to suggest it.
> > > 
> > > Yes in fact I agree. clock-lanes can be reintroduced if someone can show
> > > hardware that supports/depends on it.
> > Konrad and I checked, Hamoa supports using other lanes as a clock lane.
> 
> Are you sure about that ?

Yes.

> 
> ipcat I thought designated lane 7 specifically as clk-lane i.e. named it
> CLK_LN of some description.

Split configurations explicitly use other lanes for clocks. E.g. check
the RB5 Navigation schematics, CAM0B connector.


-- 
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 v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-06-03 12:22 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
	devicetree, linux-kernel
In-Reply-To: <c6aetoiz3dcedlxwjmt5cqh2mngswtmanf6p4s2molemnviwdc@btotpaqwcsoy>

On 03/06/2026 13:10, Dmitry Baryshkov wrote:
>> Documentation shows clock lane at lane 7.
>>
>> Truthfully it makes no sense that the clock lane would genuinely be locked
>> to lane 7 but the documentation does seem to suggest it.
>>
>> Yes in fact I agree. clock-lanes can be reintroduced if someone can show
>> hardware that supports/depends on it.
> Konrad and I checked, Hamoa supports using other lanes as a clock lane.

Are you sure about that ?

ipcat I thought designated lane 7 specifically as clk-lane i.e. named it 
CLK_LN of some description.

---
bod

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

^ permalink raw reply

* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Dmitry Baryshkov @ 2026-06-03 12:10 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Bryan O'Donoghue, linux-arm-msm, linux-phy, linux-media,
	devicetree, linux-kernel
In-Reply-To: <be3e1abe-5148-4247-930b-2e23164eea73@linaro.org>

On Tue, Jun 02, 2026 at 11:22:41PM +0100, Bryan O'Donoghue wrote:
> On 02/06/2026 23:07, Vladimir Zapolskiy wrote:
> > > +    ret = fwnode_property_read_u32(ep, "clock-lanes", &clock_lane);
> > > +    if (ret) {
> > > +        clock_lane = CSI2_DEFAULT_CLK_LN;
> > > +        dev_info(dev, "Using default clock-lane %d\n",
> > > +             CSI2_DEFAULT_CLK_LN);
> > 
> > Why CSI2_DEFAULT_CLK_LN is set to 7, what does it mean and how is it used?
> > 
> > Since "7" is a meaningless number in the context, I believe it's
> > practically
> > not used at all, and if so, 'clock-lanes' property should be just removed.
> 
> Documentation shows clock lane at lane 7.
> 
> Truthfully it makes no sense that the clock lane would genuinely be locked
> to lane 7 but the documentation does seem to suggest it.
> 
> Yes in fact I agree. clock-lanes can be reintroduced if someone can show
> hardware that supports/depends on it.

Konrad and I checked, Hamoa supports using other lanes as a clock lane.

-- 
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 phy-next 13/15] dt-bindings: phy: lynx-10g: initial document
From: Vladimir Oltean @ 2026-06-03 11:56 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-phy, Ioana Ciornei, Vinod Koul, Neil Armstrong,
	Tanjeff Moos, linux-kernel, devicetree, Conor Dooley,
	Krzysztof Kozlowski, Rob Herring
In-Reply-To: <20260602-reviving-aging-a7d066d2448b@spud>

On Tue, Jun 02, 2026 at 06:10:30PM +0100, Conor Dooley wrote:
> On Fri, May 29, 2026 at 08:15:07PM +0300, Vladimir Oltean wrote:
> > Add a schema for the 10G Lynx SerDes. This is very similar to the modern
> > form of the 28G Lynx SerDes, which is very much the intention.
> > 
> > We allow both forms of #phy-cells = <1> in the top-level provider
> > and #phy-cells = <0> in the per-lane provider for more flexibility to
> > consumers, and because the kernel code is shared with the 28G Lynx which
> > already has that support for compatibility reasons.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> > Cc: devicetree@vger.kernel.org
> > Cc: Conor Dooley <conor+dt@kernel.org>
> > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > Cc: Rob Herring <robh@kernel.org>
> > 
> > v1->v2:
> > - move patch later in series, right before driver
> > - deliberately ignoring this Sashiko feedback:
> >   https://lore.kernel.org/linux-phy/20260529125017.ifqunh52gdzhthdg@skbuf/
> > ---
> >  .../devicetree/bindings/phy/fsl,lynx-10g.yaml | 131 ++++++++++++++++++
> >  1 file changed, 131 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > new file mode 100644
> > index 000000000000..993f076bba4e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > @@ -0,0 +1,131 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale Lynx 10G SerDes PHY
> > +
> > +maintainers:
> > +  - Vladimir Oltean <vladimir.oltean@nxp.com>
> > +
> > +description:
> > +  The 10G Lynx is a multi-protocol SerDes block which handles networking, PCIe,
> > +  SATA and other high-speed interfaces. It is present on most QorIQ and
> > +  Layerscape SoCs. The register map is common, but the integration is
> > +  SoC-specific, with the differences consisting in register endianness, the
> > +  number of lanes, protocol converters available per lane and their location in
> > +  the PCCR registers. Some SoCs have multiple SerDes blocks and those differ in
> > +  their protocol capabilities per lane.
> > +
> > +properties:
> > +  compatible:
> > +    description:
> > +      There is intentionally no generic fsl,lynx-10g compatible string due to
> > +      the hardware inability to report its capabilities, despite having a
> > +      common register map.
> 
> I think you can probably drop this from the diff, and put it in the
> commit message.

Ok.

> > +    enum:
> > +      - fsl,ls1028a-serdes
> > +      - fsl,ls1046a-serdes1
> > +      - fsl,ls1046a-serdes2
> > +      - fsl,ls1088a-serdes1
> > +      - fsl,ls1088a-serdes2
> > +      - fsl,ls2088a-serdes1
> > +      - fsl,ls2088a-serdes2
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  big-endian: true
> 
> This isn't a required property, but should it be made required for the
> specific compatibles that are big endian? Or are we not that lucky, and
> devices can be either?

I can make big-endian a required property for fsl,ls1046a-serdes1 and
fsl,ls1046a-serdes2.

Thanks for the review!

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

^ permalink raw reply

* Re: [PATCH v2 phy-next 14/15] phy: lynx-10g: new driver
From: Vladimir Oltean @ 2026-06-03 11:53 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: conor+dt, devicetree, vkoul, neil.armstrong, linux-phy, robh,
	olteanv
In-Reply-To: <20260529182118.007AC1F00893@smtp.kernel.org>

On Fri, May 29, 2026 at 06:21:17PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Incorrect restoration of the PCCR mux configuration because `0` is treated as uninitialized.
> - [High] Potential timing violation when bringing lanes out of reset and power down simultaneously.
> 
> Pre-existing issues:
> - [High] lynx_xlate can return a NULL pointer for unprobed PHY lanes, leading to a NULL pointer dereference in the PHY core framework.
> --
> 
> commit 851e6824db7b03958722b9808f3549609168de99
> Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> phy: lynx-10g: new driver
>     
> Introduce a driver for the networking lanes of the 10G Lynx SerDes
> block, present on the majority of Layerscape and QorIQ (Freescale/NXP)
> SoCs.
> 
> > diff --git a/drivers/phy/freescale/phy-fsl-lynx-10g.c b/drivers/phy/freescale/phy-fsl-lynx-10g.c
> > index 0000000000000..9b04d6a4b825a
> > --- /dev/null
> > +++ b/drivers/phy/freescale/phy-fsl-lynx-10g.c
> [ ... ]
> > +static int lynx_10g_lane_enable_pcvt(struct lynx_lane *lane,
> > +				     enum lynx_lane_mode mode)
> > +{
> > +	struct lynx_priv *priv = lane->priv;
> > +	u32 val;
> > +	int err;
> > +
> > +	spin_lock(&priv->pcc_lock);
> [ ... ]
> > +	if (lane->default_pccr[mode]) {
> [Severity: High]
> Can this condition incorrectly fall through to hardcoded defaults if a valid
> mux configuration of 0 is backed up?
> 
> Since default_pccr is zero-initialized by devm_kcalloc(), a backed-up value
> of 0 evaluates to false. This skips the restoration and allows the driver to
> apply hardcoded defaults later, which could permanently reroute the hardware
> lanes and break the network interface.

No it cannot. default_pccr[mode] == 0 is not a valid mux configuration,
it universally means the protocol converter is disabled.

> > +		err = lynx_pccr_write(lane, mode, lane->default_pccr[mode]);
> > +		goto out;
> > +	}
> > +
> > +	val = 0;
> [ ... ]
> > +static int lynx_10g_power_on(struct phy *phy)
> > +{
> > +	struct lynx_lane *lane = phy_get_drvdata(phy);
> > +
> > +	if (lane->powered_up)
> > +		return 0;
> > +
> > +	/* The RM says to wait for at least 120ns between per lane setting have
> > +	 * been changed and the lane being taken out of reset
> > +	 */
> > +	usleep_range(1, 2);
> > +
> > +	lynx_lane_rmw(lane, LNaGCR0, LNaGCR0_RRST_OFF | LNaGCR0_TRST_OFF,
> > +		      LNaGCR0_RRST | LNaGCR0_TRST |
> > +		      LNaGCR0_RX_PD | LNaGCR0_TX_PD);
> [Severity: High]
> Does this code violate typical PHY hardware sequencing by clearing both the
> power-down bits and digital reset bits simultaneously?
> 
> Releasing the digital reset at the same time as activating the analog
> components doesn't leave time for the analog circuitry to stabilize. The
> prior usleep_range() occurs before power-up is initiated, which might not
> prevent hardware lockups or unreliable behavior on link up.

This is actually a great comment. Re-reading the hardware documentation,
the power up sequence is indeed messed up. Will fix for v3.

> > +
> > +	lane->powered_up = true;
> > +
> > +	return 0;
> > +}
> [ ... ]
> > +static int lynx_10g_probe(struct platform_device *pdev)
> > +{
> > +	return lynx_probe(pdev, of_device_get_match_data(&pdev->dev),
> > +			  &lynx_10g_ops);
> > +}
> [Severity: High]
> This is a pre-existing issue, but does the core PHY framework crash if
> lynx_xlate() returns a NULL pointer for an unprobed PHY lane?
> 
> If a PHY lane is requested via device tree but the driver did not probe it,
> priv->lane[idx].phy will be NULL. If lynx_xlate() returns this NULL pointer,
> the core PHY framework interprets it as a successful retrieval of an optional
> PHY and might attempt to dereference it. Would it be safer to explicitly
> return ERR_PTR(-ENODEV) in lynx_xlate() when the phy pointer is NULL?

Will fix in v3.

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

^ permalink raw reply

* Re: [PATCH v2 0/6] phy: rockchip: samsung-hdptx: Clock fixes and API transition cleanups
From: Cristian Ciocaltea @ 2026-06-03 10:27 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Algea Cao,
	Dmitry Baryshkov
  Cc: kernel, linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
	Thomas Niederprüm, Simon Wright
In-Reply-To: <e0eeb794-11b9-4ba6-bee2-f6cc607d3a16@collabora.com>

On 5/20/26 10:05 PM, Cristian Ciocaltea wrote:
> Hi Vinod,
> 
> On 5/11/26 9:21 PM, Cristian Ciocaltea wrote:
>> This series provides a set of bug fixes and cleanups for the Rockchip
>> Samsung HDPTX PHY driver.
>>
>> The first part of the series (i.e. PATCH 1 & 2) addresses clock rate
>> calculation and synchronization issues.  Specifically, it fixes edge
>> cases where the PHY PLL is pre-programmed by an external component (like
>> a bootloader) or when changing the color depth (bpc) while keeping the
>> modeline constant.  Because the Common Clock Framework .set_rate()
>> callback might not be invoked if the pixel clock remains unchanged, this
>> previously led to out-of-sync states between CCF and the actual HDMI PHY
>> configuration.
>>
>> The second part focuses on code cleanups and modernizing the register
>> access.  Now that dw_hdmi_qp driver has fully switched to using
>> phy_configure(), we can drop the deprecated TMDS rate setup workarounds
>> and the restrict_rate_change flag logic.  Finally, it refactors the
>> driver to consistently use standard bitfield macros.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> Changes in v2:
>> - Collected Tested-by tags from Thomas and Simon
>> - Fixed a typo in commit description of patch 1
>> - Added a comment in patch 2 explaining why PLL config errors are
>>   ignored for rk_hdptx_phy_consumer_get()
>> - Added a missed FIELD_GET conversion for lcpll_hw.pms_sdiv in patch 6
>> - Rebased onto latest phy/fixes
>> - Link to v1: https://lore.kernel.org/r/20260227-hdptx-clk-fixes-v1-0-f998f2762d0f@collabora.com
> 
> In case you missed my comments from last week on the Sashiko AI review findings
> - in short, I don't think there is anything to worry about and the series should
> be fine to apply as-is.  Please let me know if you would still prefer a new
> revision.
Kind reminder..

Regards,
Cristian

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

^ permalink raw reply

* Re: [PATCH v8 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Loic Poulain @ 2026-06-03 10:10 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Bryan O'Donoghue, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Vladimir Zapolskiy, linux-arm-msm, linux-phy, linux-media,
	devicetree, linux-kernel
In-Reply-To: <47a00bf8-dad3-449b-8881-93606fe33c81@kernel.org>

On Tue, Jun 2, 2026 at 3:58 PM Bryan O'Donoghue <bod@kernel.org> wrote:
>
> On 02/06/2026 09:18, Loic Poulain wrote:
> > Hi Bryan,
> >
> > On Sat, May 23, 2026 at 4:53 AM Bryan O'Donoghue
> > <bryan.odonoghue@linaro.org> wrote:
> >>
> >> Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
> >> existing CAMSS CSI PHY init sequences are imported in order to save time
> >> and effort in later patches.
> >>
> >> The following devices are supported in this drop:
> >> "qcom,x1e80100-csi2-phy"
> >>
> >> In-line with other PHY drivers the process node is included in the name.
> >> Data-lane and clock lane positioning and polarity selection via newly
> >> amended struct phy_configure_opts_mipi_dphy{} is supported.
> >>
> >> The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
> >> DPHY is supported.
> >>
> >> In porting some of the logic over from camss-csiphy*.c to here its also
> >> possible to rationalise some of the code.
> >>
> >> In particular use of regulator_bulk and clk_bulk as well as dropping the
> >> seemingly useless and unused interrupt handler.
> >>
> >> The PHY sequences and a lot of the logic that goes with them are well
> >> proven in CAMSS and mature so the main thing to watch out for here is how
> >> to get the right sequencing of regulators, clocks and register-writes.
> >>
> >> The register init sequence table is imported verbatim from the existing
> >> CAMSS csiphy driver. A follow-up series will rework the table to extract
> >> the repetitive per-lane pattern into a loop.
> >>
> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> >> ---
> >>   MAINTAINERS                                        |  10 +
> >>   drivers/phy/qualcomm/Kconfig                       |  14 +
> >>   drivers/phy/qualcomm/Makefile                      |   5 +
> >>   drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 376 +++++++++++++++++++
> >>   drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c     | 402 +++++++++++++++++++++
> >>   drivers/phy/qualcomm/phy-qcom-mipi-csi2.h          |  95 +++++
> >>   6 files changed, 902 insertions(+)
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 63389fea5d150..3b5da8a40383f 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -22018,6 +22018,16 @@ S:     Maintained
> >>   F:     Documentation/devicetree/bindings/media/qcom,*-iris.yaml
> >>   F:     drivers/media/platform/qcom/iris/
> >>
> >> +QUALCOMM MIPI CSI2 PHY DRIVER
> >> +M:     Bryan O'Donoghue <bod@kernel.org>
> >> +L:     linux-phy@lists.infradead.org
> >> +L:     linux-media@vger.kernel.org
> >> +L:     linux-arm-msm@vger.kernel.org
> >> +S:     Maintained
> >> +F:     Documentation/devicetree/bindings/phy/qcom,*-csi2-phy.yaml
> >> +F:     drivers/phy/qualcomm/phy-qcom-mipi-csi2*.c
> >> +F:     drivers/phy/qualcomm/phy-qcom-mipi-csi2*.h
> >> +
> >>   QUALCOMM NAND CONTROLLER DRIVER
> >>   M:     Manivannan Sadhasivam <mani@kernel.org>
> >>   L:     linux-mtd@lists.infradead.org
> >> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> >> index 60a0ead127fa9..779a3511ba852 100644
> >> --- a/drivers/phy/qualcomm/Kconfig
> >> +++ b/drivers/phy/qualcomm/Kconfig
> >> @@ -28,6 +28,20 @@ config PHY_QCOM_EDP
> >>            Enable this driver to support the Qualcomm eDP PHY found in various
> >>            Qualcomm chipsets.
> >>
> >> +config PHY_QCOM_MIPI_CSI2
> >> +       tristate "Qualcomm MIPI CSI2 PHY driver"
> >> +       depends on ARCH_QCOM || COMPILE_TEST
> >> +       depends on OF
> >> +       depends on PM
> >> +       depends on COMMON_CLK
> >> +       select GENERIC_PHY
> >> +       select GENERIC_PHY_MIPI_DPHY
> >> +       help
> >> +         Enable this to support the MIPI CSI2 PHY driver found in various
> >> +         Qualcomm chipsets. This PHY is used to connect MIPI CSI2
> >> +         camera sensors to the CSI Decoder in the Qualcomm Camera Subsystem
> >> +         CAMSS.
> >> +
> >>   config PHY_QCOM_IPQ4019_USB
> >>          tristate "Qualcomm IPQ4019 USB PHY driver"
> >>          depends on OF && (ARCH_QCOM || COMPILE_TEST)
> >> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> >> index b71a6a0bed3f1..382cb594b06b6 100644
> >> --- a/drivers/phy/qualcomm/Makefile
> >> +++ b/drivers/phy/qualcomm/Makefile
> >> @@ -6,6 +6,11 @@ obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)     += phy-qcom-ipq4019-usb.o
> >>   obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)    += phy-qcom-ipq806x-sata.o
> >>   obj-$(CONFIG_PHY_QCOM_M31_USB)         += phy-qcom-m31.o
> >>   obj-$(CONFIG_PHY_QCOM_M31_EUSB)                += phy-qcom-m31-eusb2.o
> >> +
> >> +phy-qcom-mipi-csi2-objs                        += phy-qcom-mipi-csi2-core.o \
> >> +                                          phy-qcom-mipi-csi2-3ph-dphy.o
> >> +obj-$(CONFIG_PHY_QCOM_MIPI_CSI2)       += phy-qcom-mipi-csi2.o
> >> +
> >>   obj-$(CONFIG_PHY_QCOM_PCIE2)           += phy-qcom-pcie2.o
> >>
> >>   obj-$(CONFIG_PHY_QCOM_QMP_COMBO)       += phy-qcom-qmp-combo.o phy-qcom-qmp-usbc.o
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> >> new file mode 100644
> >> index 0000000000000..86ec405820e62
> >> --- /dev/null
> >> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
> >> @@ -0,0 +1,376 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v1.0
> >> + *
> >> + * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
> >> + * Copyright (C) 2016-2025 Linaro Ltd.
> >> + */
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/io.h>
> >> +#include <linux/time64.h>
> >> +
> >> +#include "phy-qcom-mipi-csi2.h"
> >> +
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n)     ((offset) + 0x4 * (n))
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET   BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE     BIT(7)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID    BIT(1)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n)   ((offset) + 0xb0 + 0x4 * (n))
> >> +
> >> +#define CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(n)             ((0x200 * (n)) + 0x24)
> >> +
> >> +/*
> >> + * 3 phase CSI has 19 common status regs with only 0-10 being used
> >> + * and 11-18 being reserved.
> >> + */
> >> +#define CSI_COMMON_STATUS_NUM                          11
> >> +/*
> >> + * There are a number of common control registers
> >> + * The offset to clear the CSIPHY IRQ status starts @ 22
> >> + * So to clear CSI_COMMON_STATUS0 this is CSI_COMMON_CONTROL22, STATUS1 is
> >> + * CONTROL23 and so on
> >> + */
> >> +#define CSI_CTRL_STATUS_INDEX                          22
> >> +
> >> +/*
> >> + * There are 43 COMMON_CTRL registers with regs after # 33 being reserved
> >> + */
> >> +#define CSI_CTRL_MAX                                   33
> >> +
> >> +#define CSIPHY_DEFAULT_PARAMS                          0
> >> +#define CSIPHY_SETTLE_CNT_LOWER_BYTE                   2
> >> +#define CSIPHY_SKEW_CAL                                        7
> >> +
> >> +/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
> >> +static const struct
> >> +mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
> >> +       /* Power up lanes 2ph mode */
> >> +       {.reg_addr = 0x1014, .reg_data = 0xd5, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +
> >> +       {.reg_addr = 0x0094, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x00a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0090, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0098, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0094, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0030, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0000, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0038, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x002c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0034, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x001c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0014, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x003c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0004, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0020, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0008, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> >> +       {.reg_addr = 0x0010, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0094, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x005c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0060, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0064, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> >> +
> >> +       {.reg_addr = 0x0e94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0ea0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e28, .reg_data = 0x04, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e00, .reg_data = 0x80, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e0c, .reg_data = 0xff, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e38, .reg_data = 0x1f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0e08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> >> +       {.reg_addr = 0x0e10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +
> >> +       {.reg_addr = 0x0494, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x04a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0490, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0498, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0494, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0430, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0400, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0438, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x042c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0434, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x041c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0414, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x043c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0404, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0420, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0408, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> >> +       {.reg_addr = 0x0410, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0494, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x045c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0460, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0464, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> >> +
> >> +       {.reg_addr = 0x0894, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x08a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0890, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0898, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0894, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0830, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0800, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0838, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x082c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0834, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x081c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0814, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x083c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0804, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0820, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0808, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> >> +       {.reg_addr = 0x0810, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0894, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x085c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0860, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0864, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> >> +
> >> +       {.reg_addr = 0x0c94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0ca0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c94, .reg_data = 0x07, .delay_us =  0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c00, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c38, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
> >> +       {.reg_addr = 0x0c10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
> >> +       {.reg_addr = 0x0c94, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0c5c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0c60, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
> >> +       {.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
> >> +};
> >> +
> >> +static inline const struct mipi_csi2phy_device_regs *
> >> +csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
> >> +{
> >> +       return &csi2phy->soc_cfg->reg_info;
> >> +}
> >> +
> >> +static void phy_qcom_mipi_csi2_hw_version_read(struct mipi_csi2phy_device *csi2phy)
> >> +{
> >> +       const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> >> +       u32 tmp;
> >> +
> >> +       writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> >> +
> >> +       tmp = readl_relaxed(csi2phy->base +
> >> +                           CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 12));
> >> +       csi2phy->hw_version = tmp;
> >> +
> >> +       tmp = readl_relaxed(csi2phy->base +
> >> +                           CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 13));
> >> +       csi2phy->hw_version |= (tmp << 8) & 0xFF00;
> >> +
> >> +       tmp = readl_relaxed(csi2phy->base +
> >> +                           CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 14));
> >> +       csi2phy->hw_version |= (tmp << 16) & 0xFF0000;
> >> +
> >> +       tmp = readl_relaxed(csi2phy->base +
> >> +                           CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 15));
> >> +       csi2phy->hw_version |= (tmp << 24) & 0xFF000000;
> >> +
> >> +       dev_dbg_once(csi2phy->dev, "CSIPHY 3PH HW Version = 0x%08x\n", csi2phy->hw_version);
> >> +}
> >> +
> >> +/*
> >> + * phy_qcom_mipi_csi2_reset - Perform software reset on CSIPHY module
> >> + * @phy_qcom_mipi_csi2: CSIPHY device
> >> + */
> >> +static void phy_qcom_mipi_csi2_reset(struct mipi_csi2phy_device *csi2phy)
> >> +{
> >> +       const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> >> +
> >> +       writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET,
> >> +              csi2phy->base + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> >> +       usleep_range(5000, 8000);
> >> +       writel(0x0, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> >> +}
> >> +
> >> +/*
> >> + * phy_qcom_mipi_csi2_settle_cnt_calc - Calculate settle count value
> >> + *
> >> + * Helper function to calculate settle count value. This is
> >> + * based on the CSI2 T_hs_settle parameter which in turn
> >> + * is calculated based on the CSI2 transmitter link frequency.
> >> + *
> >> + * Return settle count value or 0 if the CSI2 link frequency
> >> + * is not available
> >> + */
> >> +static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
> >> +{
> >> +       u32 t_hs_prepare_max_ps;
> >> +       u32 timer_period_ps;
> >> +       u32 t_hs_settle_ps;
> >> +       u8 settle_cnt;
> >> +       u32 ui_ps;
> >> +
> >> +       if (link_freq <= 0)
> >> +               return 0;
> >> +
> >> +       ui_ps = div_u64(PSEC_PER_SEC, link_freq);
> >> +       ui_ps /= 2;
> >> +       t_hs_prepare_max_ps = 85000 + 6 * ui_ps;
> >> +       t_hs_settle_ps = t_hs_prepare_max_ps;
> >> +
> >> +       timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
> >> +       settle_cnt = t_hs_settle_ps / timer_period_ps - 6;
> >> +
> >> +       return settle_cnt;
> >> +}
> >> +
> >> +static void
> >> +phy_qcom_mipi_csi2_gen2_config_lanes(struct mipi_csi2phy_device *csi2phy,
> >> +                                    u8 settle_cnt)
> >> +{
> >> +       const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> >> +       const struct mipi_csi2phy_lane_regs *r = regs->init_seq;
> >> +       int i, array_size = regs->lane_array_size;
> >> +       u32 val;
> >> +
> >> +       for (i = 0; i < array_size; i++, r++) {
> >> +               switch (r->param_type) {
> >> +               case CSIPHY_SETTLE_CNT_LOWER_BYTE:
> >> +                       val = settle_cnt & 0xff;
> >> +                       break;
> >> +               case CSIPHY_SKEW_CAL:
> >> +                       /* TODO: support application of skew from dt flag */
> >> +                       continue;
> >> +               default:
> >> +                       val = r->reg_data;
> >> +                       break;
> >> +               }
> >> +               writel(val, csi2phy->base + r->reg_addr);
> >> +               if (r->delay_us)
> >> +                       udelay(r->delay_us);
> >> +       }
> >> +}
> >> +
> >> +static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
> >> +                                          struct mipi_csi2phy_stream_cfg *cfg)
> >> +{
> >> +       const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> >> +       struct mipi_csi2phy_lanes_cfg *lane_cfg = &cfg->lane_cfg;
> >> +       u8 settle_cnt;
> >> +       u8 val;
> >> +       int i;
> >> +
> >> +       settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
> >> +
> >> +       /* Lane position enable in common reg offset */
> >> +       val = BIT(lane_cfg->clk.pos);
> >> +       for (i = 0; i < cfg->num_data_lanes; i++)
> >> +               val |= BIT(lane_cfg->data[i].pos);
> >> +
> >> +       writel(val, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> >> +
> >> +       /* Lane configuration for polarity @ CSIPHY-base + CTRL9 */
> >> +       for (i = 0; i < cfg->num_data_lanes; i++) {
> >> +               if (lane_cfg->data[i].pol) {
> >> +                       u8 pos = lane_cfg->data[i].pos;
> >> +
> >> +                       writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(pos));
> >> +               }
> >> +       }
> >> +
> >> +       if (lane_cfg->clk.pol)
> >> +               writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRL9n(lane_cfg->clk.pos));
> >> +
> >> +       val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
> >> +       writel(val, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> >> +
> >> +       val = 0x02;
> >> +       writel(val, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 7));
> >> +
> >> +       val = 0x00;
> >> +       writel(val, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
> >> +
> >> +       phy_qcom_mipi_csi2_gen2_config_lanes(csi2phy, settle_cnt);
> >> +
> >> +       /* IRQ_MASK registers - disable all interrupts */
> >> +       for (i = CSI_COMMON_STATUS_NUM; i < CSI_CTRL_STATUS_INDEX; i++) {
> >> +               writel(0, csi2phy->base +
> >> +                      CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, i));
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static void
> >> +phy_qcom_mipi_csi2_lanes_disable(struct mipi_csi2phy_device *csi2phy,
> >> +                                struct mipi_csi2phy_stream_cfg *cfg)
> >> +{
> >> +       const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
> >> +
> >> +       writel(0, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
> >> +
> >> +       writel(0, csi2phy->base +
> >> +              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
> >> +}
> >> +
> >> +static const struct mipi_csi2phy_hw_ops phy_qcom_mipi_csi2_ops_3ph_1_0 = {
> >> +       .hw_version_read = phy_qcom_mipi_csi2_hw_version_read,
> >> +       .reset = phy_qcom_mipi_csi2_reset,
> >> +       .lanes_enable = phy_qcom_mipi_csi2_lanes_enable,
> >> +       .lanes_disable = phy_qcom_mipi_csi2_lanes_disable,
> >> +};
> >> +
> >> +static const char * const x1e_clks[] = {
> >> +       "core",
> >> +       "timer"
> >> +};
> >> +
> >> +static const char * const x1e_supplies[] = {
> >> +       "vdda-0p9",
> >> +       "vdda-1p2"
> >> +};
> >> +
> >> +static const char * const x1e_genpd_names[] = {
> >> +       "mmcx",
> >> +       "mx",
> >> +};
> >> +
> >> +const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
> >> +       .ops = &phy_qcom_mipi_csi2_ops_3ph_1_0,
> >> +       .reg_info = {
> >> +               .init_seq = lane_regs_x1e80100,
> >> +               .lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
> >> +               .common_regs_offset = 0x1000,
> >> +       },
> >> +       .supply_names = (const char **)x1e_supplies,
> >> +       .num_supplies = ARRAY_SIZE(x1e_supplies),
> >> +       .clk_names = (const char **)x1e_clks,
> >> +       .num_clk = ARRAY_SIZE(x1e_clks),
> >> +       .opp_clk = x1e_clks[0],
> >> +       .timer_clk = x1e_clks[1],
> >> +       .genpd_names = (const char **)x1e_genpd_names,
> >> +       .num_genpd_names = ARRAY_SIZE(x1e_genpd_names),
> >> +};
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> >> new file mode 100644
> >> index 0000000000000..dfeff863a406f
> >> --- /dev/null
> >> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> >> @@ -0,0 +1,402 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (c) 2025, Linaro Ltd.
> >> + */
> >> +#include <dt-bindings/phy/phy.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/err.h>
> >> +#include <linux/io.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/pm_opp.h>
> >> +#include <linux/phy/phy.h>
> >> +#include <linux/phy/phy-mipi-dphy.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/pm_domain.h>
> >> +#include <linux/pm_runtime.h>
> >> +#include <linux/regmap.h>
> >> +#include <linux/regulator/consumer.h>
> >> +#include <linux/reset.h>
> >> +#include <linux/slab.h>
> >> +
> >> +#include "phy-qcom-mipi-csi2.h"
> >> +
> >> +static int
> >> +phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
> >> +                                  s64 link_freq)
> >> +{
> >> +       struct device *dev = csi2phy->dev;
> >> +       unsigned long opp_rate = link_freq / 4;
> >> +       struct dev_pm_opp *opp;
> >> +       long timer_rate;
> >> +       int i, ret;
> >> +
> >> +       opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
> >> +       if (IS_ERR(opp)) {
> >> +               dev_err(csi2phy->dev, "Couldn't find ceiling for %lld Hz\n",
> >> +                       link_freq);
> >> +               return PTR_ERR(opp);
> >> +       }
> >> +
> >> +       for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
> >> +               unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
> >> +
> >> +               ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
> >> +               if (ret) {
> >> +                       dev_err(csi2phy->dev, "Couldn't set perf state %u\n",
> >> +                               perf);
> >> +                       dev_pm_opp_put(opp);
> >> +                       goto unset_pstate;
> >> +               }
> >> +       }
> >> +       dev_pm_opp_put(opp);
> >> +
> >> +       ret = dev_pm_opp_set_rate(dev, opp_rate);
> >> +       if (ret) {
> >> +               dev_err(csi2phy->dev, "dev_pm_opp_set_rate() fail\n");
> >> +               goto unset_opp_rate;
> >> +       }
> >> +
> >> +       timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
> >> +       if (timer_rate <= 0) {
> >> +               ret = -ENODEV;
> >> +               goto unset_opp_rate;
> >> +       }
> >> +
> >> +       ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
> >> +       if (ret)
> >> +               goto unset_opp_rate;
> >> +
> >> +       csi2phy->timer_clk_rate = timer_rate;
> >> +
> >> +       return 0;
> >> +
> >> +unset_opp_rate:
> >> +       dev_pm_opp_set_rate(dev, 0);
> >> +
> >> +unset_pstate:
> >> +       while (i--)
> >> +               dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int phy_qcom_mipi_csi2_configure(struct phy *phy,
> >> +                                       union phy_configure_opts *opts)
> >> +{
> >> +       struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> >> +       struct phy_configure_opts_mipi_dphy *dphy_cfg = &opts->mipi_dphy;
> >> +       struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
> >> +       int ret;
> >> +
> >> +       ret = phy_mipi_dphy_config_validate(dphy_cfg);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       if (dphy_cfg->lanes < 1 || dphy_cfg->lanes > CSI2_MAX_DATA_LANES)
> >> +               return -EINVAL;
> >> +
> >> +       stream_cfg->link_freq = dphy_cfg->hs_clk_rate;
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
> >> +{
> >> +       struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> >> +       const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
> >> +       struct device *dev = &phy->dev;
> >> +       int i, ret;
> >> +
> >> +       ret = regulator_bulk_enable(csi2phy->soc_cfg->num_supplies,
> >> +                                   csi2phy->supplies);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = pm_runtime_resume_and_get(csi2phy->dev);
> >> +       if (ret < 0)
> >> +               goto disable_regulators;
> >> +
> >> +       ret = phy_qcom_mipi_csi2_set_clock_rates(csi2phy, csi2phy->stream_cfg.link_freq);
> >> +       if (ret)
> >> +               goto poweroff_phy;
> >> +
> >> +       ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
> >> +                                     csi2phy->clks);
> >> +       if (ret) {
> >> +               dev_err(dev, "failed to enable clocks, %d\n", ret);
> >> +               goto unset_rate;
> >> +       }
> >> +
> >> +       ops->reset(csi2phy);
> >> +
> >> +       ops->hw_version_read(csi2phy);
> >> +
> >> +       return ops->lanes_enable(csi2phy, &csi2phy->stream_cfg);
> >> +
> >> +unset_rate:
> >> +       for (i = 0; i < csi2phy->pd_list->num_pds; i++)
> >> +               dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> >> +
> >> +       dev_pm_opp_set_rate(csi2phy->dev, 0);
> >> +
> >> +poweroff_phy:
> >> +       pm_runtime_put_sync(csi2phy->dev);
> >> +
> >> +disable_regulators:
> >> +       regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> >> +                              csi2phy->supplies);
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
> >> +{
> >> +       struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
> >> +       const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
> >> +       int i;
> >> +
> >> +       ops->lanes_disable(csi2phy, &csi2phy->stream_cfg);
> >> +
> >> +       clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
> >> +                                  csi2phy->clks);
> >> +
> >> +       for (i = 0; i < csi2phy->pd_list->num_pds; i++)
> >> +               dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
> >> +
> >> +       dev_pm_opp_set_rate(csi2phy->dev, 0);
> >> +
> >> +       pm_runtime_put_sync(csi2phy->dev);
> >> +
> >> +       regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
> >> +                              csi2phy->supplies);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static const struct phy_ops phy_qcom_mipi_csi2_ops = {
> >> +       .configure      = phy_qcom_mipi_csi2_configure,
> >> +       .power_on       = phy_qcom_mipi_csi2_power_on,
> >> +       .power_off      = phy_qcom_mipi_csi2_power_off,
> >> +       .owner          = THIS_MODULE,
> >> +};
> >> +
> >> +static struct phy *qcom_csi2_phy_xlate(struct device *dev,
> >> +                                      const struct of_phandle_args *args)
> >> +{
> >> +       struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
> >> +
> >> +       if (args->args[0] != PHY_TYPE_DPHY) {
> >> +               dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
> >> +               return ERR_PTR(-EOPNOTSUPP);
> >> +       }
> >> +
> >> +       csi2phy->phy_mode = args->args[0];
> >> +
> >> +       return csi2phy->phy;
> >> +}
> >> +
> >> +static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2phy)
> >> +{
> >> +       const struct dev_pm_domain_attach_data pd_data = {
> >> +               .pd_names = csi2phy->soc_cfg->genpd_names,
> >> +               .num_pd_names = csi2phy->soc_cfg->num_genpd_names,
> >> +       };
> >> +
> >> +       return devm_pm_domain_attach_list(csi2phy->dev, &pd_data, &csi2phy->pd_list);
> >
> > If strict domain/name checking isn’t required (is there a reason it
> > would be?), we could simplify the soc_cfg struct and pass NULL instead
> > of pd_data in the above call.
>
> Naming is a nice feature as it means you can mix RPMPD and GDSC
> power-domains attaching opps to RPMPD only.
>
> >
> >> +}
> >> +
> >> +static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
> >> +{
> >> +       struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
> >> +       u32 lane_polarities[CSI2_MAX_DATA_LANES + 1];
> >> +       u32 data_lanes[CSI2_MAX_DATA_LANES];
> >> +       struct device *dev = csi2phy->dev;
> >> +       struct fwnode_handle *ep;
> >> +       int num_polarities;
> >> +       int num_data_lanes;
> >> +       u32 clock_lane;
> >> +       int i, ret;
> >> +
> >> +       ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 1, 0,
> >> +                                            FWNODE_GRAPH_ENDPOINT_NEXT);
> >> +       if (ep) {
> >> +               fwnode_handle_put(ep);
> >> +               dev_err(dev, "DPHY split mode is not supported\n");
> >> +               return -EOPNOTSUPP;
> >> +       }
> >> +
> >> +       ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
> >> +       if (!ep) {
> >> +               dev_err(dev, "Missing port@0\n");
> >> +               return -ENODEV;
> >> +       }
> >> +
> >> +       num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
> >> +       if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
> >> +               ret = -EINVAL;
> >> +               dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
> >> +               goto out_put;
> >> +       }
> >> +       stream_cfg->num_data_lanes = num_data_lanes;
> >> +
> >> +       ret = fwnode_property_read_u32_array(ep, "data-lanes", data_lanes,
> >> +                                            stream_cfg->num_data_lanes);
> >> +       if (ret) {
> >> +               dev_err(dev, "Failed to read data-lanes: %d\n", ret);
> >> +               goto out_put;
> >> +       }
> >> +
> >> +       ret = fwnode_property_read_u32(ep, "clock-lanes", &clock_lane);
> >> +       if (ret) {
> >> +               clock_lane = CSI2_DEFAULT_CLK_LN;
> >> +               dev_info(dev, "Using default clock-lane %d\n",
> >> +                        CSI2_DEFAULT_CLK_LN);
> >> +       }
> >> +
> >> +       /* lane-polarities: optional, up to num_data_lanes + 1 entries */
> >> +       memset(lane_polarities, 0x00, sizeof(lane_polarities));
> >> +       num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
> >> +       if (num_polarities > 0) {
> >> +               if (num_polarities != stream_cfg->num_data_lanes + 1) {
> >> +                       ret = -EINVAL;
> >> +                       dev_err(dev, "clock+data-lane %d/polarities %d mismatch\n",
> >> +                               stream_cfg->num_data_lanes + 1, num_polarities);
> >> +                       goto out_put;
> >> +               }
> >> +
> >> +               ret = fwnode_property_read_u32_array(ep, "lane-polarities", lane_polarities,
> >> +                                                    num_polarities);
> >> +               if (ret) {
> >> +                       dev_err(dev, "Failed to read lane-polarities: %d\n", ret);
> >> +                       goto out_put;
> >> +               }
> >> +       }
> >> +
> >> +       for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
> >> +               csi2phy->stream_cfg.lane_cfg.data[i].pos = data_lanes[i];
> >> +               csi2phy->stream_cfg.lane_cfg.data[i].pol = lane_polarities[i + 1];
> >> +       }
> >> +       csi2phy->stream_cfg.lane_cfg.clk.pos = clock_lane;
> >> +       csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
> >> +
> >> +       ret = 0;
> >> +
> >> +out_put:
> >> +       fwnode_handle_put(ep);
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
> >> +{
> >> +       unsigned int i, num_clk, num_supplies;
> >> +       struct mipi_csi2phy_device *csi2phy;
> >> +       struct phy_provider *phy_provider;
> >> +       struct device *dev = &pdev->dev;
> >> +       struct phy *generic_phy;
> >> +       int ret;
> >> +
> >> +       csi2phy = devm_kzalloc(dev, sizeof(*csi2phy), GFP_KERNEL);
> >> +       if (!csi2phy)
> >> +               return -ENOMEM;
> >> +
> >> +       csi2phy->dev = dev;
> >> +       dev_set_drvdata(dev, csi2phy);
> >> +
> >> +       csi2phy->soc_cfg = device_get_match_data(&pdev->dev);
> >> +
> >> +       if (!csi2phy->soc_cfg)
> >> +               return -EINVAL;
> >> +
> >> +       num_clk = csi2phy->soc_cfg->num_clk;
> >> +       csi2phy->clks = devm_kzalloc(dev, sizeof(*csi2phy->clks) * num_clk, GFP_KERNEL);
> >> +       if (!csi2phy->clks)
> >> +               return -ENOMEM;
> >> +
> >> +       ret = phy_qcom_mipi_csi2_parse_routing(csi2phy);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = phy_qcom_mipi_csi2_attach_pm_domains(csi2phy);
> >> +       if (ret < 0)
> >> +               return dev_err_probe(dev, ret, "Failed to attach power-domain list\n");
> >> +
> >> +       devm_pm_runtime_enable(dev);
> >> +
> >> +       for (i = 0; i < num_clk; i++)
> >> +               csi2phy->clks[i].id = csi2phy->soc_cfg->clk_names[i];
> >> +
> >> +       ret = devm_clk_bulk_get(dev, num_clk, csi2phy->clks);
> >> +       if (ret)
> >> +               return dev_err_probe(dev, ret, "Failed to get clocks\n");
> >
> > Maybe it would be simpler to use devm_pm_clk_create +
> > of_pm_clk_add_clks ? then the clocks would be automatically handled
> > from the PM core on suspend/resume. And you wouldn't have to specify
> > and handle per-platform specific clock names/count (if such strict
> > checking is not necessary).
>
> I think TBH, I'd rather keep control of the ordering of voting /
> clock-enables in the driver.
>
> >> +
> >> +       csi2phy->timer_clk = devm_clk_get(dev, csi2phy->soc_cfg->timer_clk);
> >> +       if (IS_ERR(csi2phy->timer_clk)) {
> >> +               return dev_err_probe(dev, PTR_ERR(csi2phy->timer_clk),
> >> +                                    "Failed to get timer clock\n");
> >> +       }
> >> +
> >> +       ret = devm_pm_opp_set_clkname(dev, csi2phy->soc_cfg->opp_clk);
> >
> > Is there any reason for the clock name to differ from "core"? Since
> > you're introducing a fresh driver and binding, it might be better to
> > avoid making the clock naming explicitly dependent on the SoC or
> > platform.
>
> This is the correct call though. The YAML mandates the name, so
> hard-coding in the driver is just an expression of that mandate or
> rather a restatement of it.
>
> I'll use core and timer directly.

Yes, that’s what I meant, this can be hardcoded here, as SoC-specific
clock naming will likely never be needed.

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

^ permalink raw reply

* Re: [PATCH v3 4/5] phy: fsl-imx8mq-usb: add control register regmap
From: Bough Chen @ 2026-06-03  7:21 UTC (permalink / raw)
  To: Xu Yang
  Cc: Vinod Koul, Neil Armstrong, Frank Li, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Jun Li, linux-phy, imx,
	linux-arm-kernel, linux-kernel, Xu Yang
In-Reply-To: <20260603-imx8mp-usb-phy-improvement-v3-4-7afb8f89abc6@nxp.com>

On Wed, Jun 03, 2026 at 01:37:17PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
> 
> The CR port is a simple 16-bit data/address parallel port that is
> provided for on-chip access to the control registers inside the
> USB 3.0 femtoPHY. Add control register regmap and export these
> registers by debugfs to help PHY's diagnostic.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v3:
>  - drop Frank's tag because it includes other changes
>  - new patch
> ---
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b0092c34416e..cda88ea1f12d 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -1,5 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0+
> -/* Copyright (c) 2017 NXP. */
> +/* Copyright 2017-2026 NXP. */

Should be : Copyright 2017, 2026 NXP.

>  
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
> @@ -11,6 +11,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
>  #include <linux/usb/typec_mux.h>
>  
>  #define PHY_CTRL0			0x0
> @@ -56,6 +57,8 @@
>  #define PHY_CTRL6_ALT_CLK_EN		BIT(1)
>  #define PHY_CTRL6_ALT_CLK_SEL		BIT(0)
>  
> +#define PHY_CRCTL			0x30
> +
>  #define PHY_TUNE_DEFAULT		0xffffffff
>  
>  #define TCA_CLK_RST			0x00
> @@ -119,6 +122,7 @@ struct imx8mq_usb_phy {
>  	void __iomem *base;
>  	struct regulator *vbus;
>  	struct tca_blk *tca;
> +	struct regmap *cr_regmap;
>  	u32 pcs_tx_swing_full;
>  	u32 pcs_tx_deemph_3p5db;
>  	u32 tx_vref_tune;
> @@ -665,6 +669,14 @@ static const struct of_device_id imx8mq_usb_phy_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
>  
> +static const struct regmap_config imx_cr_regmap_config = {
> +	.name = "cr",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0x7,
> +};
> +

Your commit message says: The CR port is a simple 16-bit data/address parallel port
But here you use 32 bit mmio, which is a bit confuse to reader. 
Maybe you can add the following comment before the regmap config:

/*
 * CR Port MMIO Register Map
 * 
 * The CR (Control Register) port uses a 16-bit data/address protocol,
 * but is accessed through 32-bit MMIO registers:
 * 
 * Offset 0x0: CR Control Register
 *   [31:16] - Control/Status flags
 *   [15:0]  - 16-bit CR Address
 * 
 * Offset 0x4: CR Data Register
 *   [31:16] - Reserved/Status
 *   [15:0]  - 16-bit CR Data
 */

Or change your commit log like this:

The CR port is a 16-bit data/address parallel port protocol that is
accessed through 32-bit MMIO registers for on-chip access to the
control registers inside the USB 3.0 femtoPHY. Add control register
regmap and export these registers by debugfs to help PHY's diagnostic.

Regards
Haibo Chen

>  static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  {
>  	struct phy_provider *phy_provider;
> @@ -694,6 +706,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(imx_phy->base))
>  		return PTR_ERR(imx_phy->base);
>  
> +	imx_phy->cr_regmap = devm_regmap_init_mmio(dev, imx_phy->base + PHY_CRCTL,
> +						   &imx_cr_regmap_config);
> +	if (IS_ERR(imx_phy->cr_regmap)) {
> +		dev_warn(dev, "Fail to init debug register regmap\n");
> +		imx_phy->cr_regmap = NULL;
> +	}
> +
>  	ret = devm_pm_runtime_set_active_enabled(dev);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> @@ -729,6 +748,9 @@ static int imx8mq_usb_phy_runtime_suspend(struct device *dev)
>  {
>  	struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
>  
> +	if (imx_phy->cr_regmap)
> +		regcache_cache_only(imx_phy->cr_regmap, true);
> +
>  	clk_disable_unprepare(imx_phy->alt_clk);
>  	clk_disable_unprepare(imx_phy->clk);
>  
> @@ -750,6 +772,9 @@ static int imx8mq_usb_phy_runtime_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	if (imx_phy->cr_regmap)
> +		regcache_cache_only(imx_phy->cr_regmap, false);
> +
>  	return 0;
>  }
>  
> 
> -- 
> 2.34.1
> 

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

^ permalink raw reply

* Re: [PATCH v2 phy-next 13/15] dt-bindings: phy: lynx-10g: initial document
From: Alexander Stein @ 2026-06-03  6:14 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-phy, Ioana Ciornei, Vinod Koul, Neil Armstrong,
	Tanjeff Moos, linux-kernel, devicetree, Conor Dooley,
	Krzysztof Kozlowski, Rob Herring
In-Reply-To: <20260602090356.ewl5bezjxyqys6ee@skbuf>

Hi Valdimir,

Am Dienstag, 2. Juni 2026, 11:03:56 CEST schrieb Vladimir Oltean:
> Hi Alexander,
> 
> On Mon, Jun 01, 2026 at 08:34:25AM +0200, Alexander Stein wrote:
> > Hi,
> > 
> > Am Freitag, 29. Mai 2026, 19:15:07 CEST schrieb Vladimir Oltean:
> > > Add a schema for the 10G Lynx SerDes. This is very similar to the modern
> > > form of the 28G Lynx SerDes, which is very much the intention.
> > > 
> > > We allow both forms of #phy-cells = <1> in the top-level provider
> > > and #phy-cells = <0> in the per-lane provider for more flexibility to
> > > consumers, and because the kernel code is shared with the 28G Lynx which
> > > already has that support for compatibility reasons.
> > > 
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---
> > > Cc: devicetree@vger.kernel.org
> > > Cc: Conor Dooley <conor+dt@kernel.org>
> > > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > > Cc: Rob Herring <robh@kernel.org>
> > > 
> > > v1->v2:
> > > - move patch later in series, right before driver
> > > - deliberately ignoring this Sashiko feedback:
> > >   https://lore.kernel.org/linux-phy/20260529125017.ifqunh52gdzhthdg@skbuf/
> > > ---
> > >  .../devicetree/bindings/phy/fsl,lynx-10g.yaml | 131 ++++++++++++++++++
> > >  1 file changed, 131 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > new file mode 100644
> > > index 000000000000..993f076bba4e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > @@ -0,0 +1,131 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml
> > > +
> > > +title: Freescale Lynx 10G SerDes PHY
> > > +
> > > +maintainers:
> > > +  - Vladimir Oltean <vladimir.oltean@nxp.com>
> > > +
> > > +description:
> > > +  The 10G Lynx is a multi-protocol SerDes block which handles networking, PCIe,
> > > +  SATA and other high-speed interfaces. It is present on most QorIQ and
> > > +  Layerscape SoCs. The register map is common, but the integration is
> > > +  SoC-specific, with the differences consisting in register endianness, the
> > > +  number of lanes, protocol converters available per lane and their location in
> > > +  the PCCR registers. Some SoCs have multiple SerDes blocks and those differ in
> > > +  their protocol capabilities per lane.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    description:
> > > +      There is intentionally no generic fsl,lynx-10g compatible string due to
> > > +      the hardware inability to report its capabilities, despite having a
> > > +      common register map.
> > > +    enum:
> > > +      - fsl,ls1028a-serdes
> > > +      - fsl,ls1046a-serdes1
> > > +      - fsl,ls1046a-serdes2
> > > +      - fsl,ls1088a-serdes1
> > > +      - fsl,ls1088a-serdes2
> > > +      - fsl,ls2088a-serdes1
> > > +      - fsl,ls2088a-serdes2
> > 
> > Silly question: What about LS1043A? AFAIK it has a single serdes block.
> > 
> > Best regards
> > Alexander
> 
> My understanding is that hardware validation for LS1043A was not
> budgeted for the main two features why the lynx-10g driver is necessary:
> RCW override for 1G <-> 10G dynamic protocol switching and KR link
> training. As such, this SoC isn't supported by the SerDes driver in the
> NXP BSP either. With the exception of 1G <-> 2.5G minor protocol
> switching, having a lynx-10g driver would not be very useful for the
> LS1043A as is, without a procedure from h/w validation to do RCW
> override.

Thanks for the explanation. If there is no need for serdes driver then good.
I was just wondering why ls1043a was not in the list, although it's quite
similar to ls1046a.

Thanks and best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



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

^ permalink raw reply

* Re: [PATCH v3 3/5] phy: fsl-imx8mq-usb: add runtime PM support
From: sashiko-bot @ 2026-06-03  6:02 UTC (permalink / raw)
  To: Xu Yang; +Cc: Frank.Li, imx, olteanv, neil.armstrong, vkoul, linux-phy
In-Reply-To: <20260603-imx8mp-usb-phy-improvement-v3-3-7afb8f89abc6@nxp.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unchecked return value of `pm_runtime_get_sync()` via `guard(pm_runtime_active)` in `tca_blk_typec_switch_set()` leads to unsafe register access.
- [High] Double disable of clocks (enable count underflow) on driver unbind due to mixing `devm_clk_get_enabled()` with manual clock management in PM runtime suspend.
--

commit 3ca36d3b5ee797e587a615f9df482c7159a29d88
Author: Xu Yang <xu.yang_2@nxp.com>

phy: fsl-imx8mq-usb: add runtime PM support

Add runtime PM to ensure the PHY is properly powered and clocked during
register access, preventing potential system hangs.

> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 591ddf3460618..b0092c34416e3 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
[ ... ]
> @@ -136,17 +137,13 @@ static int tca_blk_typec_switch_set(struct typec_switch_dev *sw,
>  {
>  	struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
>  	struct tca_blk *tca = imx_phy->tca;
> -	int ret;
>  
>  	if (tca->orientation == orientation)
>  		return 0;
>  
> -	ret = clk_prepare_enable(imx_phy->clk);
> -	if (ret)
> -		return ret;
> +	guard(pm_runtime_active)(&imx_phy->phy->dev);

[Severity: High]
Does this wrapper ignore the return value of pm_runtime_get_sync()?

If the runtime resume fails (for example, if the underlying clock
enablement fails), the error appears to be swallowed here. Execution would
then proceed to tca_blk_orientation_set(), which performs MMIO accesses
on the unclocked TCA hardware registers.

On i.MX/ARM platforms, accessing unclocked register space triggers a
synchronous external abort (SError), resulting in a system hang.

Should this explicitly use pm_runtime_resume_and_get() or similar, and
verify the return value before proceeding?

>  
>  	tca_blk_orientation_set(tca, orientation);
> -	clk_disable_unprepare(imx_phy->clk);
>  
>  	return 0;
>  }
[ ... ]
> @@ -686,6 +671,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct imx8mq_usb_phy *imx_phy;
>  	const struct phy_ops *phy_ops;
> +	int ret;
>  
>  	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
>  	if (!imx_phy)
> @@ -693,13 +679,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, imx_phy);
>  
> -	imx_phy->clk = devm_clk_get(dev, "phy");
> +	imx_phy->clk = devm_clk_get_enabled(dev, "phy");

[Severity: High]
Could mixing devm_clk_get_enabled() with manual clk_disable_unprepare()
in the runtime suspend callback cause a double-disable?

During driver unbind, the driver core may transition the device to the
suspended state, which invokes the runtime suspend callback and drops
the clock enable count to 0.

Immediately following this, devres teardown runs and unconditionally
calls clk_disable_unprepare() again for these devm_clk_get_enabled()
actions.

Would this drop the clock count to -1, triggering an enable count
underflow WARN in the clock core?

>  	if (IS_ERR(imx_phy->clk)) {
>  		dev_err(dev, "failed to get imx8mq usb phy clock\n");
>  		return PTR_ERR(imx_phy->clk);
>  	}
>  
> -	imx_phy->alt_clk = devm_clk_get_optional(dev, "alt");
> +	imx_phy->alt_clk = devm_clk_get_optional_enabled(dev, "alt");
>  	if (IS_ERR(imx_phy->alt_clk))
>  		return dev_err_probe(dev, PTR_ERR(imx_phy->alt_clk),
>  				    "Failed to get alt clk\n");
[ ... ]
> @@ -735,11 +725,43 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  }
>  
> +static int imx8mq_usb_phy_runtime_suspend(struct device *dev)
> +{
> +	struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(imx_phy->alt_clk);
> +	clk_disable_unprepare(imx_phy->clk);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-imx8mp-usb-phy-improvement-v3-0-7afb8f89abc6@nxp.com?part=3

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

^ permalink raw reply

* Re: [PATCH 2/2] phy: qcom: usb-hs: honour qcom,vendor-init-seq raw ULPI writes
From: sashiko-bot @ 2026-06-03  6:01 UTC (permalink / raw)
  To: Herman van Hazendonk
  Cc: devicetree, robh, vkoul, neil.armstrong, linux-phy, olteanv,
	conor+dt
In-Reply-To: <20260603054809.565723-3-github.com@herrie.org>

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

Pre-existing issues:
- [High] Using `seq->addr == 0` as a loop terminator prevents writing to offset 0 in `qcom,init-seq`.
--

commit 23768611bb7332cb583754c2260443d91ea51fb4
Author: Herman van Hazendonk <github.com@herrie.org>

phy: qcom: usb-hs: honour qcom,vendor-init-seq raw ULPI writes

This patch adds support for the optional qcom,vendor-init-seq DT property
to write directly to raw ULPI register addresses. It also hardens the parse
logic shared between qcom,init-seq and qcom,vendor-init-seq by factoring it
out into qcom_usb_hs_phy_parse_init_seq().

> diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> index 98a18987f1bec..89fbe8f8d0015 100644
> --- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
[ ... ]
> @@ -199,6 +219,59 @@ static const struct phy_ops qcom_usb_hs_phy_ops = {
[ ... ]
> +static int qcom_usb_hs_phy_parse_init_seq(struct ulpi *ulpi,
> +					  const char *propname,
> +					  struct ulpi_seq **out)
> +{
[ ... ]
> +	if (size) {
> +		int ret = of_property_read_u8_array(ulpi->dev.of_node,
> +						    propname, (u8 *)seq, size);
> +		if (ret)
> +			return ret;
> +	}
> +	/* NUL-terminate so the power_on loop's seq->addr-as-sentinel works. */
> +	seq[size / 2].addr = 0;

[Severity: High]
This is a pre-existing issue, but does using seq->addr == 0 as a sentinel
prevent writing to offset 0 in qcom,init-seq?

In qcom_usb_hs_phy_power_on(), the initialization loop terminates when
seq->addr is 0:

	for (seq = uphy->init_seq; seq->addr; seq++) {
		ret = ulpi_write(ulpi, ULPI_EXT_VENDOR_SPECIFIC + seq->addr,
				 seq->val);

Since seq->addr represents an offset added to ULPI_EXT_VENDOR_SPECIFIC
for qcom,init-seq, an offset of 0 is valid and maps to the first vendor
specific register. If a device tree specifies a write to offset 0, won't
the loop prematurely terminate and silently truncate the remainder of the
initialization sequence?

> +	seq[size / 2].val = 0;
> +
> +	*out = seq;
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603054809.565723-1-github.com@herrie.org?part=2

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

^ permalink raw reply


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