* [PATCH v4 phy-next 09/16] phy: lynx-28g: make lynx_28g_pll_read_configuration() callable per PLL
From: Vladimir Oltean @ 2026-06-08 22:17 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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->v4: 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 v4 phy-next 06/16] phy: lynx-28g: generalize protocol converter accessors
From: Vladimir Oltean @ 2026-06-08 22:17 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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->v4: 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 v4 phy-next 07/16] phy: lynx-28g: provide default lynx_lane_supports_mode() implementation
From: Vladimir Oltean @ 2026-06-08 22:17 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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->v4: 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 v4 phy-next 05/16] phy: lynx-28g: common lynx_pll_get()
From: Vladimir Oltean @ 2026-06-08 22:16 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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->v4: 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 v4 phy-next 04/16] phy: lynx-28g: move data structures to core
From: Vladimir Oltean @ 2026-06-08 22:16 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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->v4: 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 v4 phy-next 03/16] phy: lynx-28g: move lane mode helpers to new core module
From: Vladimir Oltean @ 2026-06-08 22:16 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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>
---
v3->v4: none
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 v4 phy-next 02/16] phy: lynx-28g: reject probing on devices with unsupported OF nodes
From: Vladimir Oltean @ 2026-06-08 22:16 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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->v4: 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 v4 phy-next 01/16] phy: lynx-28g: avoid returning NULL in of_xlate() function
From: Vladimir Oltean @ 2026-06-08 22:16 UTC (permalink / raw)
To: linux-phy
Cc: Ioana Ciornei, Vinod Koul, Neil Armstrong, Tanjeff Moos,
linux-kernel
In-Reply-To: <20260608221710.1572971-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
v3->v4: none
---
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 v4 phy-next 00/16] New Generic PHY driver for Lynx 10G SerDes
From: Vladimir Oltean @ 2026-06-08 22:16 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 v3:
- Minor dt-schema changes to avoid relying on fixups to make syntax
valid (14/16), and add Rob Herring's review tag:
https://github.com/devicetree-org/dt-schema/blob/v2026.04/dtschema/fixups.py#L19
v3 at:
https://lore.kernel.org/linux-phy/20260603131914.503053-1-vladimir.oltean@nxp.com/
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 | 137 ++
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, 2196 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 v2 2/2] dt-bindings: phy: qcom,qmp-usb: Add ipq5210 USB3 PHY
From: Krzysztof Kozlowski @ 2026-06-08 20:06 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, quic_wcheng,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260608103344.2740174-3-varadarajan.narayanan@oss.qualcomm.com>
On Mon, Jun 08, 2026 at 04:03:44PM +0530, Varadarajan Narayanan wrote:
> Add dt-bindings for the USB3 QMP PHY found on the Qualcomm IPQ5210 SoC. The
> IPQ5210 PHY is compatible with the IPQ9574 PHY, so add it as a fallback-
> compatible entry using a oneOf construct rather than a plain enum entry.
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
> .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 46 +++++++++++--------
> 1 file changed, 26 insertions(+), 20 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: phy: qcom,qusb2: Document IPQ5210 compatible
From: Krzysztof Kozlowski @ 2026-06-08 20:04 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, quic_wcheng,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260608103344.2740174-2-varadarajan.narayanan@oss.qualcomm.com>
On Mon, Jun 08, 2026 at 04:03:43PM +0530, Varadarajan Narayanan wrote:
> Document the QUSB2 PHY compatible for the IPQ5210 SoC. The IPQ5210 PHY is
> compatible with the IPQ6018 QUSB2 PHY, so allow it to use
> qcom,ipq6018-qusb2-phy as the fallback compatible.
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/3] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
From: Dmitry Baryshkov @ 2026-06-08 18:37 UTC (permalink / raw)
To: palash.kambar
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, mani, alim.akhtar,
bvanassche, andersson, abel.vesa, luca.weiss, linux-arm-msm,
linux-phy, devicetree, linux-kernel, linux-scsi, nitin.rawat
In-Reply-To: <20260526090956.2340262-4-palash.kambar@oss.qualcomm.com>
On Tue, May 26, 2026 at 02:39:56PM +0530, palash.kambar@oss.qualcomm.com wrote:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Add the init sequence tables and config for the UFS QMP phy found in
> the Hawi SoC.
>
> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
> ---
> .../phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h | 24 +++
> .../phy-qcom-qmp-qserdes-txrx-ufs-v8.h | 37 +++++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 139 ++++++++++++++++++
> 3 files changed, 200 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/3] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
From: Palash Kambar @ 2026-06-08 15:15 UTC (permalink / raw)
To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, mani, alim.akhtar,
bvanassche, andersson, dmitry.baryshkov, abel.vesa, luca.weiss
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-scsi,
nitin.rawat
In-Reply-To: <20260526090956.2340262-4-palash.kambar@oss.qualcomm.com>
On 5/26/2026 2:39 PM, palash.kambar@oss.qualcomm.com wrote:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Add the init sequence tables and config for the UFS QMP phy found in
> the Hawi SoC.
>
> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
> ---
> .../phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h | 24 +++
> .../phy-qcom-qmp-qserdes-txrx-ufs-v8.h | 37 +++++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 139 ++++++++++++++++++
> 3 files changed, 200 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> new file mode 100644
> index 000000000000..e80d3dd6a190
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_PCS_UFS_V7_H_
> +#define QCOM_PHY_QMP_PCS_UFS_V7_H_
> +
> +/* Only for QMP V7 PHY - UFS PCS registers */
> +#define QPHY_V7_PCS_UFS_PHY_START 0x000
> +#define QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL 0x004
> +#define QPHY_V7_PCS_UFS_SW_RESET 0x008
> +#define QPHY_V7_PCS_UFS_PCS_CTRL1 0x01C
> +#define QPHY_V7_PCS_UFS_PLL_CNTL 0x028
> +#define QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL 0x02C
> +#define QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY 0x060
> +#define QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY 0x094
> +#define QPHY_V7_PCS_UFS_LINECFG_DISABLE 0x140
> +#define QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2 0x150
> +#define QPHY_V7_PCS_UFS_READY_STATUS 0x16c
> +#define QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1 0x1b8
> +#define QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1 0x1c0
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> new file mode 100644
> index 000000000000..5f923c3e64ec
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +#define QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX (0x34)
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX (0x38)
> +#define QSERDES_UFS_V8_TX_LANE_MODE_1 (0x80)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2 (0x1BC)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4 (0x1C4)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4 (0x1DC)
> +#define QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1 (0x2C8)
> +#define QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS (0x1E4)
> +#define QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3 (0x2D0)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4 (0x120)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4 (0xD4)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4 (0xEC)
> +#define QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL (0x288)
> +#define QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4 (0x2B0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4 (0x324)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7 (0x3B4)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9 (0x3BC)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7 (0x3E0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9 (0x3E8)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7 (0x40C)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9 (0x414)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7 (0x438)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9 (0x440)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_SATURATION (0xF4)
> +#define QSERDES_UFS_V8_RX_TERM_BW_CTRL0 (0x1AC)
> +#define QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL (0x498)
> +#define QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM (0x4d0)
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> index 771bc7c2ab50..2fac3a7eb820 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -29,9 +29,11 @@
> #include "phy-qcom-qmp-pcs-ufs-v4.h"
> #include "phy-qcom-qmp-pcs-ufs-v5.h"
> #include "phy-qcom-qmp-pcs-ufs-v6.h"
> +#include "phy-qcom-qmp-pcs-ufs-v7.h"
>
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v6.h"
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v7.h"
> +#include "phy-qcom-qmp-qserdes-txrx-ufs-v8.h"
>
> /* QPHY_PCS_READY_STATUS bit */
> #define PCS_READY BIT(0)
> @@ -84,6 +86,13 @@ static const unsigned int ufsphy_v6_regs_layout[QPHY_LAYOUT_SIZE] = {
> [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V6_PCS_UFS_POWER_DOWN_CONTROL,
> };
>
> +static const unsigned int ufsphy_v7_regs_layout[QPHY_LAYOUT_SIZE] = {
> + [QPHY_START_CTRL] = QPHY_V7_PCS_UFS_PHY_START,
> + [QPHY_PCS_READY_STATUS] = QPHY_V7_PCS_UFS_READY_STATUS,
> + [QPHY_SW_RESET] = QPHY_V7_PCS_UFS_SW_RESET,
> + [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL,
> +};
> +
> static const struct qmp_phy_init_tbl milos_ufsphy_serdes[] = {
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0xd9),
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x16),
> @@ -1306,6 +1315,11 @@ static const struct regulator_bulk_data sm8750_ufsphy_vreg_l[] = {
> { .supply = "vdda-pll", .init_load_uA = 18300 },
> };
>
> +static const struct regulator_bulk_data hawi_ufsphy_vreg_l[] = {
> + { .supply = "vdda-phy", .init_load_uA = 324000 },
> + { .supply = "vdda-pll", .init_load_uA = 27000 },
> +};
> +
> static const struct qmp_ufs_offsets qmp_ufs_offsets = {
> .serdes = 0,
> .pcs = 0xc00,
> @@ -1324,6 +1338,15 @@ static const struct qmp_ufs_offsets qmp_ufs_offsets_v6 = {
> .rx2 = 0x1a00,
> };
>
> +static const struct qmp_ufs_offsets qmp_ufs_offsets_v7 = {
> + .serdes = 0,
> + .pcs = 0x0400,
> + .tx = 0x2000,
> + .rx = 0x2000,
> + .tx2 = 0x3000,
> + .rx2 = 0x3000,
> +};
> +
> static const struct qmp_phy_cfg milos_ufsphy_cfg = {
> .lanes = 2,
>
> @@ -1844,6 +1867,119 @@ static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
>
> };
>
> +static const struct qmp_phy_init_tbl hawi_ufsphy_serdes[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_SYSCLK_EN_SEL, 0xd9),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_CONFIG_1, 0x16),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_SEL_1, 0x11),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_EN, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_CFG, 0x60),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO_MODE1, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IETRIM, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IPTRIM, 0x20),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_MAP, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_CTRL, 0x40),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE0, 0x41),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE0, 0x7f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE0, 0x92),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE0, 0x1e),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE1, 0x4c),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE1, 0x99),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE1, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE1, 0xbe),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE1, 0x23),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_tx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_LANE_MODE_1, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX, 0x17),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_rx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3, 0x0e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4, 0x1c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL, 0x8e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4, 0xb8),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_SATURATION, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_TERM_BW_CTRL0, 0xfa),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL, 0x30),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM, 0x77),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1, 0x43),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PCS_CTRL1, 0x42),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL, 0x0f),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2, 0x68),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1, 0x02),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_g5_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PLL_CNTL, 0x3b),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY, 0x06),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY, 0x06),
> +};
> +
> +static const struct qmp_phy_cfg hawi_ufsphy_cfg = {
> + .lanes = 2,
> +
> + .offsets = &qmp_ufs_offsets_v7,
> + .max_supported_gear = UFS_HS_G5,
> +
> + .tbls = {
> + .serdes = hawi_ufsphy_serdes,
> + .serdes_num = ARRAY_SIZE(hawi_ufsphy_serdes),
> + .tx = hawi_ufsphy_tx,
> + .tx_num = ARRAY_SIZE(hawi_ufsphy_tx),
> + .rx = hawi_ufsphy_rx,
> + .rx_num = ARRAY_SIZE(hawi_ufsphy_rx),
> + .pcs = hawi_ufsphy_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_pcs),
> + },
> +
> + .tbls_hs_overlay[0] = {
> + .pcs = hawi_ufsphy_g5_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_g5_pcs),
> + .max_gear = UFS_HS_G5,
> + },
> +
> + .vreg_list = hawi_ufsphy_vreg_l,
> + .num_vregs = ARRAY_SIZE(hawi_ufsphy_vreg_l),
> + .regs = ufsphy_v7_regs_layout,
> +};
> +
> static void qmp_ufs_serdes_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls *tbls)
> {
> void __iomem *serdes = qmp->serdes;
> @@ -2258,6 +2394,9 @@ static int qmp_ufs_probe(struct platform_device *pdev)
>
> static const struct of_device_id qmp_ufs_of_match_table[] = {
> {
> + .compatible = "qcom,hawi-qmp-ufs-phy",
> + .data = &hawi_ufsphy_cfg,
> + }, {
> .compatible = "qcom,milos-qmp-ufs-phy",
> .data = &milos_ufsphy_cfg,
> }, {
Hi Dmitry,
I have addressed your previous review comments, please let me know if there are any other comments from your side.
Thanks.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 phy-next 14/16] dt-bindings: phy: lynx-10g: initial document
From: Vladimir Oltean @ 2026-06-08 12:03 UTC (permalink / raw)
To: Rob Herring
Cc: Conor Dooley, sashiko-reviews, vkoul, neil.armstrong, conor+dt,
linux-phy, devicetree, olteanv
In-Reply-To: <20260605202458.GA6304-robh@kernel.org>
On Fri, Jun 05, 2026 at 03:24:58PM -0500, Rob Herring wrote:
> On Fri, Jun 05, 2026 at 11:37:37AM +0300, Vladimir Oltean wrote:
> > On Wed, Jun 03, 2026 at 04:50:39PM +0100, Conor Dooley wrote:
> > > On Wed, Jun 03, 2026 at 04:38:25PM +0300, Vladimir Oltean wrote:
> > > > On Wed, Jun 03, 2026 at 01:33:50PM +0000, sashiko-bot@kernel.org wrote:
> > > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > > > - [Medium] The `if` condition for enforcing the `big-endian` property incorrectly matches the `compatible` string array directly against an `enum`, which will always evaluate to false and cause the requirement to silently fail.
> > > > > --
> > > > >
> > > > > commit 8bd3982a89722b20a3686e42d4ab3b9f0711ae11
> > > > > Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > >
> > > > > dt-bindings: phy: lynx-10g: initial document
> > > > >
> > > > > This commit adds the schema for the 10G Lynx SerDes used on most QorIQ and
> > > > > Layerscape SoCs. It defines the valid compatible strings, PHY cell requirements,
> > > > > and enforces SoC-specific constraints such as lane counts and endianness.
> > > > >
> > > > > > 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 0000000000000..8cf6027a75b91
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > > > > + - if:
> > > > > > + properties:
> > > > > > + compatible:
> > > > > > + enum:
> > > > > > + - fsl,ls1046a-serdes1
> > > > > > + - fsl,ls1046a-serdes2
> > > > >
> > > > > [Severity: Medium]
> > > > > Does this conditional correctly enforce the big-endian property?
> > > > >
> > > > > Since the compatible property is a string array in device trees, evaluating it
> > > > > directly against an enum always returns false. Should this use "contains:"
> > > > > with the enum, similar to the preceding if block, so dt_binding_check
> > > > > correctly enforces the requirement?
> > > >
> > > > The tests I did suggest that it does enforce the requirement.
> > >
> > > Yeah, sashiko just doesn't understand how this works without "contains".
> > > "contains" is only actually required for validation purposes if there's
> > > fallback compatibles and you're trying to get a partial match.
>
> Not exactly. The missing part is the schema is fixed up so the above is
> converted to:
>
> compatible:
> minItems: 1
> maxItems: 1
> items:
> - enum:
> - fsl,ls1046a-serdes1
> - fsl,ls1046a-serdes2
>
> If we didn't do that, the above is not what we want here, but 'contains'
> is as that's the loosest condition and works regardless of how many
> entries compatible has. That's what you should do here.
>
> This fixup used to be a lot more widespread when we used the DT YAML
> format and everything was decoded to an an array or matrix. The fixups
> saved a lot of boilerplate. That's less true now, so maybe getting rid
> of more fixups like this case would be better.
Thanks for the explanation, it makes sense now.
I ran "dt-mk-schema -j Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml"
and I can see the following, which seems to check out:
"http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml": {
"$filename": "/opt/linux/net-phy-upstreaming/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml",
"$id": "http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml#",
"$schema": "http://devicetree.org/meta-schemas/core.yaml#",
"additionalProperties": false,
"allOf": [
{
{
"if": {
"properties": {
"compatible": {
"items": [
{
"enum": [
"fsl,ls1046a-serdes1",
"fsl,ls1046a-serdes2"
]
}
],
"maxItems": 1,
"minItems": 1,
"type": "array"
}
}
},
"then": {
"required": [
"big-endian"
]
}
}
}
]
}
I can send a v4 replacing the constraint with:
allOf:
- if:
properties:
compatible:
contains:
enum:
- fsl,ls1028a-serdes
- fsl,ls1046a-serdes1
- fsl,ls1046a-serdes2
- fsl,ls1088a-serdes1
- fsl,ls1088a-serdes2
then:
patternProperties:
"^phy@[0-7]$":
properties:
reg:
minimum: 0
maximum: 3
- if:
properties:
compatible:
contains:
enum:
- fsl,ls1046a-serdes1
- fsl,ls1046a-serdes2
then:
properties:
big-endian: true
required:
- big-endian
since you've said that we shouldn't rely on this fixup anymore. I see
you gave me a Reviewed-by and I deliberately am not adding a fallback
compatible, but I can do so anyway.
> > Yeah, I have no clear understanding of the json-schema syntax either, I
> > just copied from another place where it was clear that the intention was
> > to have multiple matches on nodes having a single compatible string each.
> >
> > But maybe it would be good from DT bindings maintainers to teach LLMs
> > where they get things wrong in this repo?
> > https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/dt-bindings.md
>
> Yes, there's a few things it consistently gets wrong. I've looked at
> this and I can go write more instructions (and fix some things that seem
> wrong), but how do I know if it really works? First, I'd be testing with
> a different LLM as that's what I have access to. Second, how do I know
> if no warning is just the indeterminate nature of LLMs?
If you have a lot of time on your hands, the question "was this concern
considered and dismissed, or not considered?" should be able to be
answered.
In the sashiko web UI for the patch set, there is a "View Raw Log" link
for each patch:
https://sashiko.dev/#/patchset/20260603131914.503053-1-vladimir.oltean%40nxp.com
This gives you its reasoning process and its internal findings (which it
tries to dismiss again in a later stage, before reporting them):
https://sashiko.dev/#/log/67856
```json
{
"findings": [
{
"problem": "The `if` condition for enforcing the `big-endian` property incorrectly matches the `compatible` string array directly against an `enum`, which will always evaluate to false and cause the requirement to silently fail.",
"severity": "Medium",
"severity_explanation": "In Device Tree schema validation, the `compatible` property is processed as a JSON string array. Inside an `if` block, standard JSON schema rules apply. Using `properties: compatible: enum:` evaluates whether the array itself exactly equals one of the scalar strings in the enum, which is structurally impossible. As a result, the condition will always evaluate to false, and the `big-endian` requirement for `fsl,ls1046a-serdes1` and `fsl,ls1046a-serdes2` will never be enforced by `dt_binding_check`. The check must be updated to use `properties: compatible: contains: enum:` to correctly evaluate the elements within the array, similar to the preceding `if` block.",
"preexisting": false,
"locations": [
{
"file": "Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml",
"function_or_symbol": null,
"line": 91,
"code_snippet": " - if:\n properties:\n compatible:\n enum:\n - fsl,ls1046a-serdes1\n - fsl,ls1046a-serdes2\n then:\n required:\n - big-endian",
"why_this_location_matters": "This conditional block incorrectly omits `contains:`, rendering the entire schema validation rule for the `big-endian` property ineffective."
}
]
}
]
}
```
> What's really needed is for sashiko to incorporate feedback like any
> other developer. Otherwise, it's going to be like some certain
> reviewers we've banned.
I don't think that process is in place right now - the learning process
from mailing list discussions is human driven and not autonomous.
Nor would it be immediately obvious how to automate it, if you consider
the risk of pollution from certain reviewers, which you've also pointed
out. But as a reviewer and maintainer, I think you are in a good
position to write your experience down in the dt-schema skill and see
how it goes from there.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-06-08 11:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-phy, vkoul, neil.armstrong, olteanv
In-Reply-To: <20260608105406.C9E051F00893@smtp.kernel.org>
On 6/8/26 12:54, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The driver blindly casts `dev->parent` to a `struct spmi_device *` without validating the parent device type, leading to a wild pointer dereference on malformed device trees.
> --
>
> commit b6b479ab20e3f2d7a7247657ca4bb0a5a70fc75a
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
>
> This commit migrates the SDAM driver to use devm_spmi_subdevice_alloc_and_add().
> It registers a new SPMI sub-device for SDAM and initializes a regmap with its
> specific base address retrieved from the devicetree.
>
>> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
>> index 4f1cca6eab71e..4974105dd9632 100644
>> --- a/drivers/nvmem/qcom-spmi-sdam.c
>> +++ b/drivers/nvmem/qcom-spmi-sdam.c
> [ ... ]
>> @@ -110,8 +110,17 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>>
>> static int sdam_probe(struct platform_device *pdev)
>> {
> [ ... ]
>> struct sdam_chip *sdam;
>> struct nvmem_device *nvmem;
>> + struct spmi_device *sparent;
>> + struct spmi_subdevice *sub_sdev;
>> + struct device *dev = &pdev->dev;
>> unsigned int val;
>> int rc;
>>
>> @@ -119,19 +128,23 @@ static int sdam_probe(struct platform_device *pdev)
>> if (!sdam)
>> return -ENOMEM;
>>
>> - sdam->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> - if (!sdam->regmap) {
>> - dev_err(&pdev->dev, "Failed to get regmap handle\n");
>> - return -ENXIO;
>> - }
>> + sparent = to_spmi_device(dev->parent);
>> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
>
> [Severity: High]
> Does this code assume that the parent device is always a valid SPMI device?
>
Yes - and that's ok, because this device cannot exist outside of SPMI and will
never be registered on its own without a SPMI parent.
Overcautious coding bloats drivers for no reason.
> If the device tree is malformed and this node is placed under a simple-bus
> instead of an SPMI PMIC, the parent device will not be an spmi_device.
> Because to_spmi_device() uses container_of(), this will result in a bogus
> pointer.
>
> When that bogus pointer is passed into devm_spmi_subdevice_alloc_and_add(),
> does it cause a wild pointer dereference when accessing sparent->ctrl or
> sparent->usid?
>
> Prior to this patch, dev_get_regmap(pdev->dev.parent, NULL) would gracefully
> return NULL for malformed hierarchies, but this change appears to introduce
> a potential kernel panic.
>
>> + if (IS_ERR(sub_sdev))
>> + return PTR_ERR(sub_sdev);
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 08/10] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-06-08 11:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260608110945.608BD1F00893@smtp.kernel.org>
On 6/8/26 13:09, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unsafe downcast of `dev->parent` to `struct spmi_device` causing out-of-bounds read and kernel panic
> --
>
> commit eb0fe105cd38bff6269804925b90232e1c422522
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
>
> This commit updates the qcom-coincell driver to use a dedicated SPMI
> sub-device instead of fetching the regmap from its parent. It creates the
> sub-device using the parent device and initializes a specific regmap to
> handle the base address offsets automatically.
>
>> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
>> index 3c57f74291476..b0904f441300c 100644
>> --- a/drivers/misc/qcom-coincell.c
>> +++ b/drivers/misc/qcom-coincell.c
>> @@ -71,37 +71,55 @@ static int qcom_coincell_probe(struct platform_device *pdev)
> [ ... ]
>> - rc = of_property_read_u32(node, "reg", &chgr.base_addr);
>> + rc = device_property_read_u32(dev, "reg", &qcom_coincell_regmap_config.reg_base);
>> if (rc)
>> return rc;
>>
>> + sparent = to_spmi_device(dev->parent);
>> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
>
> [Severity: High]
> Could this lead to a panic if the parent device is not actually an SPMI
> device?
>
> If this driver is bound to a device whose parent is a regular platform
> device (for example, via a malformed device tree or a manual sysfs bind),
> dev->parent will not point to a valid spmi_device.
>
This device cannot exist outside of SPMI and will never be registered on its
own without a SPMI parent.
Overcautious coding bloats drivers for no reason, so this is fine.
> Because to_spmi_device() is a container_of cast, it will blindly cast the
> parent device. When passed to devm_spmi_subdevice_alloc_and_add(), the
> underlying spmi_subdevice_alloc_and_add() function reads sparent->ctrl:
>
> drivers/spmi/spmi.c:spmi_subdevice_alloc_and_add() {
> ...
> sdev->ctrl = sparent->ctrl;
> ...
> }
>
> Since the parent is actually a platform_device, this ends up reading the
> memory offset corresponding to platform_dma_mask instead of a valid ctrl
> pointer. Later regmap initialization will dereference this invalid pointer,
> causing an out-of-bounds read.
>
> The previous dev_get_regmap() approach avoided this by safely searching the
> parent's devres list without relying on a structural cast. Is there a way to
> verify the parent device type before casting?
>
>> + if (IS_ERR(sub_sdev))
>> + return PTR_ERR(sub_sdev);
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 02/10] spmi: Print error status with %pe format
From: AngeloGioacchino Del Regno @ 2026-06-08 11:29 UTC (permalink / raw)
To: sashiko-reviews; +Cc: neil.armstrong, linux-phy, vkoul, olteanv
In-Reply-To: <20260608103702.411271F00893@smtp.kernel.org>
On 6/8/26 12:37, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The OF device node assigned to the newly allocated SPMI device does not have its reference count incremented, creating a potential use-after-free condition when the iterator macro automatically drops the reference.
> --
>
> commit eb2ded032315a7ba1f8c878f6e0a10db80cf0346
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> spmi: Print error status with %pe format
>
> This commit updates error messages in the SPMI driver to use the %pe format
> specifier for printing error statuses instead of printing integer error codes.
>
>> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
>> index e93fc2c957590..95db19a0a44ec 100644
>> --- a/drivers/spmi/spmi.c
>> +++ b/drivers/spmi/spmi.c
> [ ... ]
>> @@ -522,7 +522,7 @@ static void of_spmi_register_devices(struct spmi_controller *ctrl)
>> err = spmi_device_add(sdev);
>> if (err) {
>> dev_err(&sdev->dev,
>> - "failure adding device. status %d\n", err);
>> + "failure adding device. status %pe\n", ERR_PTR(err));
>
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential use-after-free
> condition with the OF node reference counting.
>
> In drivers/spmi/spmi.c:of_spmi_register_devices(), the
> for_each_available_child_of_node() loop iterates through child nodes,
> automatically calling of_node_put() on the previous node when advancing.
>
> Inside the loop, the node is assigned to the SPMI device:
>
> device_set_node(&sdev->dev, of_fwnode_handle(node));
>
> Does this assignment need to invoke of_node_get() to increment the node's
> reference count? Without it, when the loop advances, the node's reference
> is dropped, leaving sdev->dev.of_node as a dangling pointer.
>
> If the device tree node is dynamically managed (e.g., via DT overlays),
> unloading the overlay will free the node, causing a use-after-free whenever
> the active SPMI device later accesses its of_node.
>
> Additionally, drivers/spmi/spmi.c:spmi_dev_release() lacks an of_node_put()
> to balance the required reference:
>
> static void spmi_dev_release(struct device *dev)
> {
> struct spmi_device *sdev = to_spmi_device(dev);
>
> kfree(sdev);
> }
>
> Should spmi_dev_release() be updated to safely release the reference once
> it is correctly obtained?
>
>> spmi_device_put(sdev);
>> }
>> }
>
Although very unlikely to happen at this moment in time, this is a valid concern.
It's out of scope for this series - so I will send a fix separately.
This should not block this series.
Cheers,
Angelo
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 5/5] phy: fsl-imx8mq-usb: keep PHY power domain runtime always-on for i.MX8MP
From: Xu Yang @ 2026-06-08 11:27 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <aiLrhzEdQE6zAauv@lizhi-Precision-Tower-5810>
On Fri, Jun 05, 2026 at 11:30:15AM -0400, Frank Li wrote:
> On Fri, Jun 05, 2026 at 07:13:06PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> >
> > On i.MX8MP, the USB PHY has a dedicated power domain that was previously
> > never powered off at runtime. With the introduction of runtime PM support,
> > the power domain will be powered off if the device is runtime suspended,
> > which breaks USB wakeup functionality.
>
> Does it need depend on if device enable wakeup?
No. The wakeup is for system suspend. We handle the runtime case here.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 4/5] phy: fsl-imx8mq-usb: add control register regmap
From: Xu Yang @ 2026-06-08 11:26 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <aiLrMs7AMECq7U66@lizhi-Precision-Tower-5810>
On Fri, Jun 05, 2026 at 11:28:50AM -0400, Frank Li wrote:
> On Fri, Jun 05, 2026 at 07:13:05PM +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
> > 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.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> > ---
> > Changes in v4:
> > - improve commit message as Haibo's suggestion
> > 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 27aa696f5dd4..e24f46d7924b 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. */
> >
> > #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;
> > @@ -667,6 +671,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,
> > +};
> > +
> > static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > {
> > struct phy_provider *phy_provider;
> > @@ -696,6 +708,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");
> > @@ -731,6 +750,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);
> > +
>
> I think this common problem, is possible to change
> regmap_read_debugfs(), let it call runtime_pm_get(), there are already
> have runtime_pm in regmap field.
Yes, it should be a common issue when access debugfs on a suspended device.
Not sure why regmap common driver doesn't add it. We can do it in future if
more such cases appear.
>
> So you debug fs always to get update value, instead cached value?
The default cache type is REGCACHE_NONE in imx_cr_regmap_config. So no
cache at all. The main purpose is to block register access here. Once
cache_only is disabled, it will read HW register value.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 04/10] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
From: AngeloGioacchino Del Regno @ 2026-06-08 11:27 UTC (permalink / raw)
To: Stephen Boyd, Jonathan Cameron
Cc: linux-phy, vkoul, olteanv, neil.armstrong, sashiko-reviews
In-Reply-To: <20260608104323.3B15D1F00893@smtp.kernel.org>
On 6/8/26 12:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] dev_set_name() uses the '%d' format specifier for a variable of type 'unsigned int', which will cause a -Wformat compiler warning.
> --
>
> commit 35ac69140b9b2d8a3b0e2aa01fe5f5ac3ffe4d02
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
>
> This commit introduces functions for managing sub-devices of an SPMI
> device, including spmi_subdevice_alloc_and_add() and its devm
> variant. These allow complex PMICs with multiple sub-devices to be
> properly modeled and managed via the SPMI bus.
>
>> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
>> index 91a40ea8031ad..7945cacf6d655 100644
>> --- a/drivers/spmi/spmi.c
>> +++ b/drivers/spmi/spmi.c
>
> [ ... ]
>
>> @@ -428,6 +454,58 @@ struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl)
>> }
>> EXPORT_SYMBOL_GPL(spmi_device_alloc);
>>
>> +/**
>> + * spmi_subdevice_alloc_and_add(): Allocate and add a new SPMI sub-device
>> + * @sparent: SPMI parent device with previously registered SPMI controller
>> + *
>> + * Returns:
>> + * Pointer to newly allocated SPMI sub-device for success or error pointer.
>> + */
>> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
>> +{
>> + struct spmi_subdevice *sub_sdev;
>> + struct spmi_device *sdev;
>> + int ret;
>> +
>> + sub_sdev = kzalloc(sizeof(*sub_sdev), GFP_KERNEL);
>> + if (!sub_sdev)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + sdev = &sub_sdev->sdev;
>> + sdev->ctrl = sparent->ctrl;
>> + sdev->usid = sparent->usid;
>> +
>> + ret = ida_alloc(&spmi_subdevice_ida, GFP_KERNEL);
>> + if (ret < 0) {
>> + kfree(sub_sdev);
>> + return ERR_PTR(ret);
>> + }
>> + sub_sdev->devid = ret;
>> +
>> + device_initialize(&sdev->dev);
>> + sdev->dev.parent = &sparent->dev;
>> + sdev->dev.bus = &spmi_bus_type;
>> + sdev->dev.type = &spmi_subdev_type;
>> +
>> + ret = dev_set_name(&sdev->dev, "%u-%02x.%d.auto",
>> + sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
>
> [Severity: Low]
> This isn't a bug, but should this use %u instead of %d for the format
> specifier? The devid field in struct spmi_subdevice is defined as an
> unsigned int, which can cause a -Wformat compiler warning here.
>
I wonder if this can be fixed while applying please?
Thanks,
Angelo
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 3/5] phy: fsl-imx8mq-usb: add runtime PM support
From: Xu Yang @ 2026-06-08 11:14 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <aiLn81lg32BPDZVp@lizhi-Precision-Tower-5810>
On Fri, Jun 05, 2026 at 11:14:59AM -0400, Frank Li wrote:
> On Fri, Jun 05, 2026 at 07:13:04PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> >
> > Add runtime PM to ensure the PHY is properly powered and clocked during
> > register access, preventing potential system hangs.
> >
> > It guards register access in the following scenarios:
> > - PHY operations: init() and power_on/off() callbacks are guarded by
> > phy core
> > - Type-C orientation switching when PHY/Controller are suspended which
> > needs explicitly care
> > - Future PHY control port register regmap debugfs access
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v4:
> > - replace guard() with PM_RUNTIME_ACQUIRE()
> > Changes in v3:
> > - new patch
> > ---
> > drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 62 +++++++++++++++++++++---------
> > 1 file changed, 43 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index 591ddf346061..27aa696f5dd4 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > @@ -9,6 +9,7 @@
> > #include <linux/of.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/regulator/consumer.h>
> > #include <linux/usb/typec_mux.h>
> >
> > @@ -136,17 +137,15 @@ 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;
> > + PM_RUNTIME_ACQUIRE(&imx_phy->phy->dev, pm);
> > + if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> > + return -ENXIO;
> >
> > tca_blk_orientation_set(tca, orientation);
> > - clk_disable_unprepare(imx_phy->clk);
> >
> > return 0;
> > }
> > @@ -620,16 +619,6 @@ static int imx8mq_phy_power_on(struct phy *phy)
> > if (ret)
> > return ret;
> >
> > - ret = clk_prepare_enable(imx_phy->clk);
> > - if (ret)
> > - return ret;
> > -
> > - ret = clk_prepare_enable(imx_phy->alt_clk);
> > - if (ret) {
> > - clk_disable_unprepare(imx_phy->clk);
> > - return ret;
> > - }
> > -
> > /* Disable rx term override */
> > value = readl(imx_phy->base + PHY_CTRL6);
> > value &= ~PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> > @@ -648,8 +637,6 @@ static int imx8mq_phy_power_off(struct phy *phy)
> > value |= PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> > writel(value, imx_phy->base + PHY_CTRL6);
> >
> > - clk_disable_unprepare(imx_phy->alt_clk);
> > - clk_disable_unprepare(imx_phy->clk);
> > regulator_disable(imx_phy->vbus);
> >
> > return 0;
> > @@ -686,6 +673,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 +681,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");
> > 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");
>
> when driver remove, devm will disable clock, which may cause refcound
> wrong if device already suspend by runtime pm
Right, so it's improper to use both devm for clk and runtime PM. I will
remove devm_pm_runtime_set_active_enabled() and use non-devm callbacks
to avoid such issue.
>
> > if (IS_ERR(imx_phy->alt_clk))
> > return dev_err_probe(dev, PTR_ERR(imx_phy->alt_clk),
> > "Failed to get alt clk\n");
> > @@ -708,6 +696,10 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > if (IS_ERR(imx_phy->base))
> > return PTR_ERR(imx_phy->base);
> >
> > + ret = devm_pm_runtime_set_active_enabled(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> > +
>
> You have not set auto suspend, so runtime_suspend will never entry.
At the end of __driver_probe_device(), it will call pm_request_idle() for
the device. So runtime_suspend() will run. And the test result align with
this logic.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 08/10] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
From: sashiko-bot @ 2026-06-08 11:09 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260608100949.36309-9-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsafe downcast of `dev->parent` to `struct spmi_device` causing out-of-bounds read and kernel panic
--
commit eb0fe105cd38bff6269804925b90232e1c422522
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
This commit updates the qcom-coincell driver to use a dedicated SPMI
sub-device instead of fetching the regmap from its parent. It creates the
sub-device using the parent device and initializes a specific regmap to
handle the base address offsets automatically.
> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
> index 3c57f74291476..b0904f441300c 100644
> --- a/drivers/misc/qcom-coincell.c
> +++ b/drivers/misc/qcom-coincell.c
> @@ -71,37 +71,55 @@ static int qcom_coincell_probe(struct platform_device *pdev)
[ ... ]
> - rc = of_property_read_u32(node, "reg", &chgr.base_addr);
> + rc = device_property_read_u32(dev, "reg", &qcom_coincell_regmap_config.reg_base);
> if (rc)
> return rc;
>
> + sparent = to_spmi_device(dev->parent);
> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
[Severity: High]
Could this lead to a panic if the parent device is not actually an SPMI
device?
If this driver is bound to a device whose parent is a regular platform
device (for example, via a malformed device tree or a manual sysfs bind),
dev->parent will not point to a valid spmi_device.
Because to_spmi_device() is a container_of cast, it will blindly cast the
parent device. When passed to devm_spmi_subdevice_alloc_and_add(), the
underlying spmi_subdevice_alloc_and_add() function reads sparent->ctrl:
drivers/spmi/spmi.c:spmi_subdevice_alloc_and_add() {
...
sdev->ctrl = sparent->ctrl;
...
}
Since the parent is actually a platform_device, this ends up reading the
memory offset corresponding to platform_dma_mask instead of a valid ctrl
pointer. Later regmap initialization will dereference this invalid pointer,
causing an out-of-bounds read.
The previous dev_get_regmap() approach avoided this by safely searching the
parent's devres list without relying on a structural cast. Is there a way to
verify the parent device type before casting?
> + if (IS_ERR(sub_sdev))
> + return PTR_ERR(sub_sdev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=8
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
From: sashiko-bot @ 2026-06-08 10:54 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: linux-phy, vkoul, neil.armstrong, olteanv
In-Reply-To: <20260608100949.36309-6-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver blindly casts `dev->parent` to a `struct spmi_device *` without validating the parent device type, leading to a wild pointer dereference on malformed device trees.
--
commit b6b479ab20e3f2d7a7247657ca4bb0a5a70fc75a
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
This commit migrates the SDAM driver to use devm_spmi_subdevice_alloc_and_add().
It registers a new SPMI sub-device for SDAM and initializes a regmap with its
specific base address retrieved from the devicetree.
> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
> index 4f1cca6eab71e..4974105dd9632 100644
> --- a/drivers/nvmem/qcom-spmi-sdam.c
> +++ b/drivers/nvmem/qcom-spmi-sdam.c
[ ... ]
> @@ -110,8 +110,17 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>
> static int sdam_probe(struct platform_device *pdev)
> {
[ ... ]
> struct sdam_chip *sdam;
> struct nvmem_device *nvmem;
> + struct spmi_device *sparent;
> + struct spmi_subdevice *sub_sdev;
> + struct device *dev = &pdev->dev;
> unsigned int val;
> int rc;
>
> @@ -119,19 +128,23 @@ static int sdam_probe(struct platform_device *pdev)
> if (!sdam)
> return -ENOMEM;
>
> - sdam->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> - if (!sdam->regmap) {
> - dev_err(&pdev->dev, "Failed to get regmap handle\n");
> - return -ENXIO;
> - }
> + sparent = to_spmi_device(dev->parent);
> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
[Severity: High]
Does this code assume that the parent device is always a valid SPMI device?
If the device tree is malformed and this node is placed under a simple-bus
instead of an SPMI PMIC, the parent device will not be an spmi_device.
Because to_spmi_device() uses container_of(), this will result in a bogus
pointer.
When that bogus pointer is passed into devm_spmi_subdevice_alloc_and_add(),
does it cause a wild pointer dereference when accessing sparent->ctrl or
sparent->usid?
Prior to this patch, dev_get_regmap(pdev->dev.parent, NULL) would gracefully
return NULL for malformed hierarchies, but this change appears to introduce
a potential kernel panic.
> + if (IS_ERR(sub_sdev))
> + return PTR_ERR(sub_sdev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding
From: Krzysztof Kozlowski @ 2026-06-08 10:45 UTC (permalink / raw)
To: Joey Lu
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jacky Huang, Shan-Chun Hung, linux-phy, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260604101220.1092822-2-a0987203069@gmail.com>
On Thu, Jun 04, 2026 at 06:12:19PM +0800, Joey Lu wrote:
A nit, subject: drop second/last, redundant "binding". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> +properties:
> + compatible:
> + const: nuvoton,ma35d1-usb2-phy-otg
> +
Where is reg? MMIO is expressed with reg, not via custom phandle.
> + clocks:
> + maxItems: 1
> +
> + nuvoton,sys:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + - items:
> + - description: phandle to the system management syscon.
> + - description: PHY instance index.
> + enum:
> + - 0 # USB0, OTG port (shared with DWC2 gadget controller)
> + - 1 # USB1, host-only port
> + description:
> + A phandle to the syscon node covering the SYS register block, with
> + one argument selecting the PHY instance. Index 0 selects the OTG
> + port PHY (USB0) and index 1 selects the host-only PHY (USB1).
> +
> + "#phy-cells":
> + const: 0
> +
> + nuvoton,rcalcode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + description:
> + Resistor calibration trim code written to the RCALCODE field in
> + USBPMISCR. The 4-bit value adjusts the PHY's internal termination
> + resistance. When absent the hardware reset default is used.
> +
> + nuvoton,oc-active-high:
> + type: boolean
> + description:
> + When present, the over-current detect input from the VBUS power
> + switch is treated as active-high. The default (property absent) is
> + active-low. This setting is shared by both USB host ports.
> +
> +required:
> + - compatible
> + - clocks
> + - nuvoton,sys
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> +
> + usb_hphy0: usb-host-phy {
usb-phy
And drop unused label.
> + compatible = "nuvoton,ma35d1-usb2-phy-otg";
> + clocks = <&clk HUSBH0_GATE>;
> + nuvoton,sys = <&sys 0>;
> + #phy-cells = <0>;
> + };
> --
> 2.43.0
>
--
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] arm64: dts: qcom: Add Shikra CQ2390M SoM platform
From: Konrad Dybcio @ 2026-06-08 10:44 UTC (permalink / raw)
To: Komal Bajaj, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
Wesley Cheng, Ulf Hansson
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
monish.chunara, Rakesh Kota
In-Reply-To: <3758be2d-4553-40b3-8ae6-bc3e502d0531@oss.qualcomm.com>
On 5/27/26 3:32 PM, Komal Bajaj wrote:
> On 5/25/2026 6:46 PM, Konrad Dybcio wrote:
>> On 5/22/26 8:02 PM, Komal Bajaj wrote:
>>> Add device tree include for the CQ2390M variant of the Shikra
>>> System-on-Module, a compact compute module integrating the Shikra SoC
>>> and PMIC for IoT applications, designed to mount on carrier boards.
>>>
>>> - shikra-cqm-som.dtsi: Retail SoM with modem (PM4125 and PM8005 PMIC)
>>>
>>> The DTSI includes the common shikra.dtsi, adds PM4125 and PM8005 PMIC
>>> regulator definitions specific to this variant.
>>>
>>> Co-developed-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
>>> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
>>> Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>>> ---
[...]
>>> +&pm8005_regulators {
>>> + status = "disabled";
>>> +};
>> ?
>
> Already there's an ongoing discussion over the thread [1].
> PM8005 regulators are controlled by rpmpd so it's not expected to be enabled here.
>
> [1]: https://lore.kernel.org/all/20260525093453.edmqryggp5kcqxxy@hu-kotarake-hyd.qualcomm.com/
Right. Downstream, MSM8998 (the other user) binds it to the GFX CPR,
which probably has the benefit lower latency than if it were to go
through RPM (although I was sorta hoping this would be an instance
of cargo cult that we could just rip out..).
If you add /omit-if-no-ref/ above the definition of the pm8005_regulators
node in pm8005.dtsi, does it get correctly skipped (i.e. if you do
dtc -I dtb shikra.dtb -O dts | grep qcom,pm8005-regulators, is is there)?
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox