* [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-02 22:23 ` Maxime Chevallier
2026-09-03 8:32 ` Maxime Chevallier
2026-09-01 15:01 ` [PATCH net-next v3 02/10] dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property Coia Prant
` (8 subsequent siblings)
9 siblings, 2 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
The current XPCS creation logic in stmmac_pcs_setup() is problematic
for several reasons.
First, if a device tree specifies a "pcs-handle" but no select_pcs()
callback is provided by the platform driver, the created XPCS is never
used. The phylink framework requires select_pcs() to actually return
the PCS to the core, so the pcs-handle property becomes effectively
useless without the matching callback. This is confusing for developers
who expect that specifying a pcs-handle in their device tree should be
sufficient to enable the PCS.
Second, and more critically, when stmmac_pcs_setup() fails to create
an XPCS (either because no pcs-handle is present and no pcs_mask is
configured), it falls through to the else branch and leaves
priv->hw->xpcs as NULL. This will silently override any XPCS that a
platform driver may have already set up during its own initialization,
for example in a pcs_init() callback or during probe. The platform
driver has no way to prevent this override because the common code
runs unconditionally after the platform-specific initialization.
After commit 93f84152e4ae ("net: stmmac: clean up
stmmac_mac_select_pcs()"), the common code no longer falls back to
priv->hw->phylink_pcs if select_pcs() is not set. This change
reinforces that each platform must manage its own PCS life cycle
explicitly, but the XPCS creation code in stmmac_pcs_setup() was not
updated to match this new expectation, leaving a gap where platform
drivers have no clean way to take control of XPCS creation.
Address all of these issues by introducing pcs_init() and pcs_exit()
callbacks in plat_stmmacenet_data. These callbacks give platform
drivers full control over when and how the XPCS is created, configured,
and destroyed. The common stmmac_pcs_setup() and stmmac_pcs_clean()
functions are simplified to just call these callbacks, removing the
confusing and error-prone XPCS creation logic from the common code.
Platforms that do not need an XPCS simply leave the callbacks as NULL
and no change in behavior occurs. Platforms that do need an XPCS can
now create it with the exact configuration they require, including
wrapping it with custom phylink_pcs_ops when necessary.
Existing platform drivers (intel, rzn1, socfpga) are updated to use
the new callbacks by moving their XPCS creation and cleanup logic into
pcs_init() and pcs_exit(). In their pcs_exit() implementations, the
pointer to the destroyed PCS is explicitly set to NULL to avoid
dangling pointer references.
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 44 +++++++++++++++++--
.../stmicro/stmmac/dwmac-renesas-gbeth.c | 7 ++-
.../net/ethernet/stmicro/stmmac/dwmac-rzn1.c | 7 ++-
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 7 ++-
.../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 37 +++-------------
5 files changed, 61 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index f5f9fa67ecd77..fd5f01c8941c1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -603,13 +603,47 @@ static void common_default_data(struct plat_stmmacenet_data *plat)
plat->mdio_bus_data->needs_reset = true;
}
+static int intel_mgbe_pcs_init(struct stmmac_priv *priv)
+{
+ struct fwnode_handle *devnode, *pcsnode;
+ struct dw_xpcs *xpcs = NULL;
+ int addr;
+
+ devnode = dev_fwnode(priv->device);
+
+ if (fwnode_property_present(devnode, "pcs-handle")) {
+ pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
+ xpcs = xpcs_create_fwnode(pcsnode);
+ fwnode_handle_put(pcsnode);
+ } else {
+ addr = ffs(priv->plat->mdio_bus_data->pcs_mask) - 1;
+ xpcs = xpcs_create_mdiodev(priv->mii, addr);
+ }
+
+ if (IS_ERR(xpcs))
+ return PTR_ERR(xpcs);
+
+ xpcs_config_eee_mult_fact(xpcs, priv->plat->mult_fact_100ns);
+
+ priv->hw->xpcs = xpcs;
+ return 0;
+}
+
+static void intel_mgbe_pcs_exit(struct stmmac_priv *priv)
+{
+ if (!priv->hw->xpcs)
+ return;
+
+ xpcs_destroy(priv->hw->xpcs);
+ priv->hw->xpcs = NULL;
+}
+
static struct phylink_pcs *intel_mgbe_select_pcs(struct stmmac_priv *priv,
phy_interface_t interface)
{
- /* plat->mdio_bus_data->has_xpcs has been set true, so there
- * should always be an XPCS. The original code would always
- * return this if present.
- */
+ if (!priv->hw->xpcs)
+ return NULL;
+
return xpcs_to_phylink_pcs(priv->hw->xpcs);
}
@@ -733,6 +767,8 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
plat->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
plat->mdio_bus_data->pcs_mask = BIT_U32(INTEL_MGBE_XPCS_ADDR);
plat->default_an_inband = true;
+ plat->pcs_init = intel_mgbe_pcs_init;
+ plat->pcs_exit = intel_mgbe_pcs_exit;
plat->select_pcs = intel_mgbe_select_pcs;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
index 19f34e18bfef2..9af32c26f9c14 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
@@ -81,8 +81,11 @@ static int renesas_gmac_pcs_init(struct stmmac_priv *priv)
static void renesas_gmac_pcs_exit(struct stmmac_priv *priv)
{
- if (priv->hw->phylink_pcs)
- miic_destroy(priv->hw->phylink_pcs);
+ if (!priv->hw->phylink_pcs)
+ return;
+
+ miic_destroy(priv->hw->phylink_pcs);
+ priv->hw->phylink_pcs = NULL;
}
static struct phylink_pcs *renesas_gmac_select_pcs(struct stmmac_priv *priv,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c
index 13634965bc19a..01df4776edb3f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c
@@ -35,8 +35,11 @@ static int rzn1_dwmac_pcs_init(struct stmmac_priv *priv)
static void rzn1_dwmac_pcs_exit(struct stmmac_priv *priv)
{
- if (priv->hw->phylink_pcs)
- miic_destroy(priv->hw->phylink_pcs);
+ if (!priv->hw->phylink_pcs)
+ return;
+
+ miic_destroy(priv->hw->phylink_pcs);
+ priv->hw->phylink_pcs = NULL;
}
static struct phylink_pcs *rzn1_dwmac_select_pcs(struct stmmac_priv *priv,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 1d7f0a57d2889..6d4bc1fe8f751 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -539,8 +539,11 @@ static int socfpga_dwmac_pcs_init(struct stmmac_priv *priv)
static void socfpga_dwmac_pcs_exit(struct stmmac_priv *priv)
{
- if (priv->hw->phylink_pcs)
- lynx_pcs_destroy(priv->hw->phylink_pcs);
+ if (!priv->hw->phylink_pcs)
+ return;
+
+ lynx_pcs_destroy(priv->hw->phylink_pcs);
+ priv->hw->phylink_pcs = NULL;
}
static struct phylink_pcs *socfpga_dwmac_select_pcs(struct stmmac_priv *priv,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index afe98ff5bdcb0..d2f77f0c223a7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -426,36 +426,15 @@ int stmmac_mdio_reset(struct mii_bus *bus)
int stmmac_pcs_setup(struct net_device *ndev)
{
struct stmmac_priv *priv = netdev_priv(ndev);
- struct fwnode_handle *devnode, *pcsnode;
- struct dw_xpcs *xpcs = NULL;
- int addr, ret;
-
- devnode = dev_fwnode(priv->device);
-
- if (priv->plat->pcs_init) {
- ret = priv->plat->pcs_init(priv);
- } else if (fwnode_property_present(devnode, "pcs-handle")) {
- pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
- xpcs = xpcs_create_fwnode(pcsnode);
- fwnode_handle_put(pcsnode);
- ret = PTR_ERR_OR_ZERO(xpcs);
- } else if (priv->plat->mdio_bus_data &&
- priv->plat->mdio_bus_data->pcs_mask) {
- addr = ffs(priv->plat->mdio_bus_data->pcs_mask) - 1;
- xpcs = xpcs_create_mdiodev(priv->mii, addr);
- ret = PTR_ERR_OR_ZERO(xpcs);
- } else {
+ int ret;
+
+ if (!priv->plat->pcs_init)
return 0;
- }
+ ret = priv->plat->pcs_init(priv);
if (ret)
return dev_err_probe(priv->device, ret, "No xPCS found\n");
- if (xpcs)
- xpcs_config_eee_mult_fact(xpcs, priv->plat->mult_fact_100ns);
-
- priv->hw->xpcs = xpcs;
-
return 0;
}
@@ -463,14 +442,10 @@ void stmmac_pcs_clean(struct net_device *ndev)
{
struct stmmac_priv *priv = netdev_priv(ndev);
- if (priv->plat->pcs_exit)
- priv->plat->pcs_exit(priv);
-
- if (!priv->hw->xpcs)
+ if (!priv->plat->pcs_exit)
return;
- xpcs_destroy(priv->hw->xpcs);
- priv->hw->xpcs = NULL;
+ priv->plat->pcs_exit(priv);
}
struct stmmac_clk_rate {
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers
2026-09-01 15:01 ` [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers Coia Prant
@ 2026-09-02 22:23 ` Maxime Chevallier
2026-09-03 8:32 ` Maxime Chevallier
1 sibling, 0 replies; 24+ messages in thread
From: Maxime Chevallier @ 2026-09-02 22:23 UTC (permalink / raw)
To: Coia Prant, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Vinod Koul, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
Hi,
On 9/1/26 17:01, Coia Prant wrote:
> The current XPCS creation logic in stmmac_pcs_setup() is problematic
> for several reasons.
[...]
>
> Existing platform drivers (intel, rzn1, socfpga) are updated to use
> the new callbacks by moving their XPCS creation and cleanup logic into
> pcs_init() and pcs_exit(). In their pcs_exit() implementations, the
> pointer to the destroyed PCS is explicitly set to NULL to avoid
> dangling pointer references.
Hopefully this covers all the glues out there, fingers crossed for
the qualcomm one. On Socfpga at least, no breakage reported :)
Romain may be able to run some tests on rzn1 at some point, but I
suspect this is fine as well.
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers
2026-09-01 15:01 ` [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers Coia Prant
2026-09-02 22:23 ` Maxime Chevallier
@ 2026-09-03 8:32 ` Maxime Chevallier
2026-09-03 8:51 ` Coia Prant
1 sibling, 1 reply; 24+ messages in thread
From: Maxime Chevallier @ 2026-09-03 8:32 UTC (permalink / raw)
To: Coia Prant, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Vinod Koul, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
Hi again,
On 9/1/26 17:01, Coia Prant wrote:
> The current XPCS creation logic in stmmac_pcs_setup() is problematic
> for several reasons.
>
> First, if a device tree specifies a "pcs-handle" but no select_pcs()
> callback is provided by the platform driver, the created XPCS is never
> used. The phylink framework requires select_pcs() to actually return
> the PCS to the core, so the pcs-handle property becomes effectively
> useless without the matching callback. This is confusing for developers
> who expect that specifying a pcs-handle in their device tree should be
> sufficient to enable the PCS.
>
> Second, and more critically, when stmmac_pcs_setup() fails to create
> an XPCS (either because no pcs-handle is present and no pcs_mask is
> configured), it falls through to the else branch and leaves
> priv->hw->xpcs as NULL. This will silently override any XPCS that a
> platform driver may have already set up during its own initialization,
> for example in a pcs_init() callback or during probe. The platform
> driver has no way to prevent this override because the common code
> runs unconditionally after the platform-specific initialization.
>
> After commit 93f84152e4ae ("net: stmmac: clean up
> stmmac_mac_select_pcs()"), the common code no longer falls back to
> priv->hw->phylink_pcs if select_pcs() is not set. This change
> reinforces that each platform must manage its own PCS life cycle
> explicitly, but the XPCS creation code in stmmac_pcs_setup() was not
> updated to match this new expectation, leaving a gap where platform
> drivers have no clean way to take control of XPCS creation.
>
> Address all of these issues by introducing pcs_init() and pcs_exit()
> callbacks in plat_stmmacenet_data. These callbacks give platform
> drivers full control over when and how the XPCS is created, configured,
> and destroyed. The common stmmac_pcs_setup() and stmmac_pcs_clean()
> functions are simplified to just call these callbacks, removing the
> confusing and error-prone XPCS creation logic from the common code.
>
> Platforms that do not need an XPCS simply leave the callbacks as NULL
> and no change in behavior occurs. Platforms that do need an XPCS can
> now create it with the exact configuration they require, including
> wrapping it with custom phylink_pcs_ops when necessary.
>
> Existing platform drivers (intel, rzn1, socfpga) are updated to use
> the new callbacks by moving their XPCS creation and cleanup logic into
> pcs_init() and pcs_exit(). In their pcs_exit() implementations, the
> pointer to the destroyed PCS is explicitly set to NULL to avoid
> dangling pointer references.
>
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> ---
> .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 44 +++++++++++++++++--
> .../stmicro/stmmac/dwmac-renesas-gbeth.c | 7 ++-
> .../net/ethernet/stmicro/stmmac/dwmac-rzn1.c | 7 ++-
> .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 7 ++-
> .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 37 +++-------------
> 5 files changed, 61 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> index f5f9fa67ecd77..fd5f01c8941c1 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> @@ -603,13 +603,47 @@ static void common_default_data(struct plat_stmmacenet_data *plat)
> plat->mdio_bus_data->needs_reset = true;
> }
>
> +static int intel_mgbe_pcs_init(struct stmmac_priv *priv)
> +{
> + struct fwnode_handle *devnode, *pcsnode;
> + struct dw_xpcs *xpcs = NULL;
> + int addr;
> +
> + devnode = dev_fwnode(priv->device);
> +
> + if (fwnode_property_present(devnode, "pcs-handle")) {
> + pcsnode = fwnode_find_reference(devnode, "pcs-handle", 0);
> + xpcs = xpcs_create_fwnode(pcsnode);
> + fwnode_handle_put(pcsnode);
> + } else {
> + addr = ffs(priv->plat->mdio_bus_data->pcs_mask) - 1;
> + xpcs = xpcs_create_mdiodev(priv->mii, addr);
> + }
Sorry I had a second look at that after a good night's sleep, and actually here
we may regress. The original logic checked for "pcs-handle" presence, then
for the pcs mask, but if no PCS is found we didn't error out, we returned 0.
This is making it mandatory to have a PCS. Please handle gracefully the "there's no
PCS" case :(
Maxime
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers
2026-09-03 8:32 ` Maxime Chevallier
@ 2026-09-03 8:51 ` Coia Prant
2026-09-03 8:58 ` Maxime Chevallier
0 siblings, 1 reply; 24+ messages in thread
From: Coia Prant @ 2026-09-03 8:51 UTC (permalink / raw)
To: Maxime Chevallier
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Coquelin, Alexandre Torgue,
Lad Prabhakar, Romain Gantois, Heiner Kallweit, Neil Armstrong,
Russell King, Shawn Lin, David Heidelberg, netdev, linux-rockchip,
devicetree, linux-arm-kernel, linux-kernel, linux-phy,
linux-stm32, linux-renesas-soc
Maxime Chevallier <maxime.chevallier@bootlin.com> 于2026年9月3日周四 16:32写道:
>
> Hi again,
>
> Sorry I had a second look at that after a good night's sleep, and actually here
> we may regress. The original logic checked for "pcs-handle" presence, then
> for the pcs mask, but if no PCS is found we didn't error out, we returned 0.
>
> This is making it mandatory to have a PCS. Please handle gracefully the "there's no
> PCS" case :(
>
>
> Maxime
Hi,
This change covers all callbacks that use `select_pcs`. Drivers
without `select_pcs` callbacks will not have their pcs used by
Phylink.
Currently, only the Intel mgbe driver uses xpcs, and Intel has also
set a mask for pcs-handle not set.
This means that on Intel platforms, the parts that implement
`select_pcs` always require xpcs to exist.
I think there shouldn't be any problems here, but I agree with you,
regression issues can indeed easily occur here.
Best,
Coia
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers
2026-09-03 8:51 ` Coia Prant
@ 2026-09-03 8:58 ` Maxime Chevallier
0 siblings, 0 replies; 24+ messages in thread
From: Maxime Chevallier @ 2026-09-03 8:58 UTC (permalink / raw)
To: Coia Prant
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Coquelin, Alexandre Torgue,
Lad Prabhakar, Romain Gantois, Heiner Kallweit, Neil Armstrong,
Russell King, Shawn Lin, David Heidelberg, netdev, linux-rockchip,
devicetree, linux-arm-kernel, linux-kernel, linux-phy,
linux-stm32, linux-renesas-soc
On 9/3/26 10:51, Coia Prant wrote:
> Maxime Chevallier <maxime.chevallier@bootlin.com> 于2026年9月3日周四 16:32写道:
>>
>> Hi again,
>>
>> Sorry I had a second look at that after a good night's sleep, and actually here
>> we may regress. The original logic checked for "pcs-handle" presence, then
>> for the pcs mask, but if no PCS is found we didn't error out, we returned 0.
>>
>> This is making it mandatory to have a PCS. Please handle gracefully the "there's no
>> PCS" case :(
>>
>>
>> Maxime
>
> Hi,
>
> This change covers all callbacks that use `select_pcs`. Drivers
> without `select_pcs` callbacks will not have their pcs used by
> Phylink.
>
> Currently, only the Intel mgbe driver uses xpcs, and Intel has also
> set a mask for pcs-handle not set.
>
> This means that on Intel platforms, the parts that implement
> `select_pcs` always require xpcs to exist.
>
> I think there shouldn't be any problems here, but I agree with you,
> regression issues can indeed easily occur here.
Alight, let's keep it this way. The assumption becomes "if you have
a pcs_init() callback, it means you expect a PCS to be there, ENODEV
is an actual error".
My reviewed-by still stands then.
Maxime
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next v3 02/10] dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 03/10] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568 Coia Prant
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
On RK3568, the SGMII interface can be routed to either GMAC0 or
GMAC1 via the pipe_sgmii_mac_sel bit in the pipe GRF registers.
Add the optional "rockchip,sgmii-mac-sel" property to allow the
device tree to select which GMAC controller is used for SGMII.
The property takes a value of 0 (GMAC0) or 1 (GMAC1). The hardware
reset value is 1 (GMAC1), but this can be overridden by setting the
property to 0 for boards where SGMII is connected to GMAC0.
This is necessary for boards such as the Ariaboard Photonicat, where
the SGMII interface is connected to GMAC0 and needs to be explicitly
configured.
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
.../bindings/phy/phy-rockchip-naneng-combphy.yaml | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
index 379b08bd9e97a..8e898bce9af73 100644
--- a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
@@ -80,6 +80,15 @@ properties:
description:
Some additional pipe settings are accessed through GRF regs.
+ rockchip,sgmii-mac-sel:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1]
+ default: 1
+ description:
+ Select gmac0 or gmac1 to be used as SGMII controller.
+ The hardware reset value is GMAC1 (1). Set this to 0 to route
+ SGMII to GMAC0.
+
"#phy-cells":
const: 1
@@ -105,6 +114,10 @@ allOf:
maxItems: 1
reset-names:
maxItems: 1
+ rockchip,sgmii-mac-sel: true
+ else:
+ properties:
+ rockchip,sgmii-mac-sel: false
- if:
properties:
compatible:
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH net-next v3 03/10] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 01/10] net: stmmac: move XPCS lifetime management to platform drivers Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 02/10] dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 04/10] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support Coia Prant
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
On RK3568, the SGMII interface can be routed to either GMAC0 or
GMAC1 via the GRF register pipe_sgmii_mac_sel.
Add support for this selection by introducing
the "rockchip,sgmii-mac-sel" DT property.
The hardware reset value is GMAC1 (1). If the property is set to 0,
the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
remains at GMAC1.
This is necessary for boards such as the Ariaboard Photonicat, which
uses the SGMII interface connected to GMAC0.
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
index 7843356a4dd47..919bb97a4b182 100644
--- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
@@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
struct combphy_reg pipe_xpcs_phy_ready;
struct combphy_reg pipe_pcie1l0_sel;
struct combphy_reg pipe_pcie1l1_sel;
+ struct combphy_reg pipe_sgmii_mac_sel;
struct combphy_reg u3otg0_port_en;
struct combphy_reg u3otg1_port_en;
};
@@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
bool enable_ssc;
bool ext_refclk;
struct clk *refclk;
+ u32 sgmii_mac_sel;
};
static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
@@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
+ priv->sgmii_mac_sel = 1;
+ device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);
+
priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy");
/* fallback to old behaviour */
if (PTR_ERR(priv->phy_rst) == -ENOENT)
@@ -873,6 +878,8 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
break;
case PHY_TYPE_SGMII:
+ rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
+ priv->sgmii_mac_sel > 0);
rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_xpcs_phy_ready, true);
rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_phymode_sel, true);
rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_sel_qsgmii, true);
@@ -984,6 +991,7 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
.con3_for_sata = { 0x000c, 15, 0, 0x00, 0x4407 },
/* pipe-grf */
.pipe_con0_for_sata = { 0x0000, 15, 0, 0x00, 0x2220 },
+ .pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
.pipe_xpcs_phy_ready = { 0x0040, 2, 2, 0x00, 0x01 },
.u3otg0_port_en = { 0x0104, 15, 0, 0x0181, 0x1100 },
.u3otg1_port_en = { 0x0144, 15, 0, 0x0181, 0x1100 },
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH net-next v3 04/10] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
` (2 preceding siblings ...)
2026-09-01 15:01 ` [PATCH net-next v3 03/10] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568 Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 05/10] arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes Coia Prant
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
Add device tree binding documentation for the Synopsys DesignWare
XPCS integrated on the Rockchip RK3568 SoC.
The XPCS is accessed over the APB3 bus and internally connected to
a Naneng Combo SerDes PHY. It supports 1000BASE-X, SGMII, and
QSGMII modes, with four MII ports.
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
.../bindings/net/pcs/rockchip-dwxpcs.yaml | 110 ++++++++++++++++++
1 file changed, 110 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip-dwxpcs.yaml
diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip-dwxpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip-dwxpcs.yaml
new file mode 100644
index 0000000000000..0852d0bcb66a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pcs/rockchip-dwxpcs.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pcs/rockchip-dwxpcs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip RK3568 Synopsys DesignWare Ethernet PCS
+
+maintainers:
+ - Coia Prant <coiaprant@gmail.com>
+
+description: |
+ Rockchip RK3568 SoC integrates a Synopsys DesignWare Ethernet Physical
+ Coding Sublayer (XPCS).
+ The PCS provides an interface between the Media Access Control (MAC)
+ and the Physical Medium Attachment (PMA) sublayer through a Media
+ Independent Interface (GMII).
+
+ The XPCS is accessed over the APB3 bus and internally connected to a
+ Naneng Combo SerDes PHY.
+ It supports 1000BASE-X, SGMII and QSGMII modes.
+
+ The block contains four MII ports that can be individually enabled and
+ routed to one of the Ethernet GMAC controllers via the pcs-handle
+ property in the MAC device tree node.
+
+properties:
+ compatible:
+ const: rockchip,rk3568-xpcs
+
+ reg:
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ clocks:
+ items:
+ - description: APB3 bus interface clock (clk_csr_i), required for register access
+ - description: EEE clock (clk_eee_i), required for Energy Efficient Ethernet operation
+
+ clock-names:
+ items:
+ - const: csr
+ - const: eee
+
+ phys:
+ maxItems: 1
+
+ phy-names:
+ const: serdes
+
+ power-domains:
+ maxItems: 1
+
+patternProperties:
+ "^pcs-mii@[0-3]$":
+ type: object
+ description:
+ One of the four MII ports of the XPCS. The port is linked to an
+ Ethernet MAC controller via the pcs-handle property in the MAC's
+ device tree node.
+
+ properties:
+ reg:
+ description: MII port number.
+ enum: [0, 1, 2, 3]
+
+ required:
+ - reg
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+ - clocks
+ - clock-names
+ - phys
+ - phy-names
+ - power-domains
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/rk3568-cru.h>
+ #include <dt-bindings/power/rk3568-power.h>
+ #include <dt-bindings/phy/phy.h>
+
+ pcs@fda00000 {
+ compatible = "rockchip,rk3568-xpcs";
+ reg = <0xfda00000 0x200000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&cru PCLK_XPCS>, <&cru CLK_XPCS_EEE>;
+ clock-names = "csr", "eee";
+ phys = <&combphy2 PHY_TYPE_SGMII>;
+ phy-names = "serdes";
+ power-domains = <&power RK3568_PD_PIPE>;
+
+ pcs-mii@0 {
+ reg = <0>;
+ };
+ };
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH net-next v3 05/10] arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
` (3 preceding siblings ...)
2026-09-01 15:01 ` [PATCH net-next v3 04/10] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-02 15:04 ` sashiko-bot
2026-09-01 15:01 ` [PATCH net-next v3 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery Coia Prant
` (4 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
The RK3568 SoC integrates a Synopsys DesignWare XPCS that provides
the Physical Coding Sublayer for 1000BASE-X, SGMII, and QSGMII
interfaces via its four MII ports. Add the XPCS device node and
its pcs-mii sub-nodes to the SoC device tree.
The XPCS device is accessed via the APB3 bus at 0xfda00000 and
requires the CSR clock (PCLK_XPCS) for register access and the EEE
clock (CLK_XPCS_EEE) for Energy Efficient Ethernet operation. The
PD_PIPE power domain must be enabled before any register access.
Also add two fixed-clock nodes (xpcs_gmac0_clk and xpcs_gmac1_clk)
providing the 125 MHz reference clock for the GMACs when operating
with XPCS. These clocks are used as the assigned-clock-parents
for the respective GMAC nodes in board-level device trees.
The XPCS node and its pcs-mii sub-nodes are disabled by default and
must be enabled at the board level when 1000BASE-X/SGMII/QSGMII is
in use. The fixed-clock nodes are always present and do not have a
status property, as they are static clock sources.
The XPCS node also requires a reference to the appropriate Naneng
Combo PHY via the phys property at the board level.
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3568.dtsi | 45 ++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 3bc653f027f1f..989e164c0eb39 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -110,6 +110,51 @@ sata0: sata@fc000000 {
status = "disabled";
};
+ xpcs: pcs@fda00000 {
+ compatible = "rockchip,rk3568-xpcs";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x0 0xfda00000 0x0 0x200000>;
+ clocks = <&cru PCLK_XPCS>, <&cru CLK_XPCS_EEE>;
+ clock-names = "csr", "eee";
+ power-domains = <&power RK3568_PD_PIPE>;
+ status = "disabled";
+
+ xpcs_mii0: pcs-mii@0 {
+ reg = <0>;
+ status = "disabled";
+ };
+
+ xpcs_mii1: pcs-mii@1 {
+ reg = <1>;
+ status = "disabled";
+ };
+
+ xpcs_mii2: pcs-mii@2 {
+ reg = <2>;
+ status = "disabled";
+ };
+
+ xpcs_mii3: pcs-mii@3 {
+ reg = <3>;
+ status = "disabled";
+ };
+ };
+
+ xpcs_gmac0_clk: xpcs-gmac0-clock {
+ compatible = "fixed-clock";
+ clock-frequency = <125000000>;
+ clock-output-names = "clk_gmac0_xpcs_mii";
+ #clock-cells = <0>;
+ };
+
+ xpcs_gmac1_clk: xpcs-gmac1-clock {
+ compatible = "fixed-clock";
+ clock-frequency = <125000000>;
+ clock-output-names = "clk_gmac1_xpcs_mii";
+ #clock-cells = <0>;
+ };
+
pipe_phy_grf0: syscon@fdc70000 {
compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
reg = <0x0 0xfdc70000 0x0 0x1000>;
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 05/10] arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes
2026-09-01 15:01 ` [PATCH net-next v3 05/10] arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes Coia Prant
@ 2026-09-02 15:04 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-02 15:04 UTC (permalink / raw)
To: Coia Prant
Cc: neil.armstrong, conor+dt, devicetree, robh, linux-phy, vkoul,
olteanv, wsa+renesas
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] DT Schema Validation Failure: Disabled 'xpcs' node lacks required properties
- [Low] DT Schema Validation Failure: 'status' property in 'pcs-mii' subnodes
- [Low] Missing Hardware Reset Property in XPCS node
- [Low] Incorrect Clock Modeling: Fake fixed-clock nodes for hardware clocks
--
commit eff2669bff2081bb72b25ba5712b66df2f70b9bf
Author: Coia Prant <coiaprant@gmail.com>
arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes
This commit adds the XPCS device node and its associated pcs-mii sub-nodes
to the RK3568 device tree. It also introduces two fixed-clock nodes to serve
as reference clocks for the GMACs.
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 3bc653f027f1f..989e164c0eb39 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
[ ... ]
> + xpcs: pcs@fda00000 {
> + compatible = "rockchip,rk3568-xpcs";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x0 0xfda00000 0x0 0x200000>;
> + clocks = <&cru PCLK_XPCS>, <&cru CLK_XPCS_EEE>;
> + clock-names = "csr", "eee";
> + power-domains = <&power RK3568_PD_PIPE>;
> + status = "disabled";
[Severity: Low]
Will this cause make dtbs_check to emit validation warnings for boards that
include rk3568.dtsi without overriding the xpcs node? The
rockchip,rk3568-xpcs binding schema unconditionally requires phys and
phy-names at the root level, even when the node is disabled.
[Severity: Low]
Does the xpcs node accurately represent the hardware without a resets
property? The RK3568 hardware provides dedicated resets (like SRST_XPCS)
for this block. Omitting the resets property might leave the hardware
description incomplete, even if the current Linux driver does not actively
use it.
> +
> + xpcs_mii0: pcs-mii@0 {
> + reg = <0>;
> + status = "disabled";
[Severity: Low]
Is the status property permitted in these pcs-mii subnodes? The
rockchip-dwxpcs.yaml schema uses additionalProperties: false for the
^pcs-mii@[0-3]$ pattern and does not explicitly allow the status property.
This might trigger another validation warning during make dtbs_check.
> + };
[ ... ]
> +
> + xpcs_gmac0_clk: xpcs-gmac0-clock {
> + compatible = "fixed-clock";
> + clock-frequency = <125000000>;
[Severity: Low]
Does modeling these 125 MHz reference clocks as standalone fixed-clock nodes
mask the true hardware dependency? Since these clocks are actually generated
by the XPCS/Combo PHY block, using dummy board-level clocks could hide the
real topology from the device tree, even if a power management race is
currently avoided by a device link in the MAC driver.
> + clock-output-names = "clk_gmac0_xpcs_mii";
> + #clock-cells = <0>;
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260901150111.141037-1-coiaprant@gmail.com?part=5
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next v3 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
` (4 preceding siblings ...)
2026-09-01 15:01 ` [PATCH net-next v3 05/10] arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-02 15:04 ` sashiko-bot
2026-09-01 15:01 ` [PATCH net-next v3 07/10] net: pcs: xpcs: add Rockchip RK3568 platform glue driver Coia Prant
` (3 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc, Jiawen Wu
On some hardware using the DesignWare XPCS IP (e.g., RK3568 MAC side
SGMII), the PCS does not automatically restart auto-negotiation when the
link goes down and comes back up. Without an explicit ANRESTART, the link
stays down forever.
Add BMCR_ANRESTART in two places:
1. In xpcs_config_aneg_c37_sgmii(), when starting AN, set ANRESTART
alongside ANENABLE to initiate a fresh negotiation.
2. In xpcs_get_state_c37_sgmii(), when link is down and AN completion is
detected, clear the interrupt and trigger ANRESTART to restart the
negotiation process.
Tested-by: Jiawen Wu <jiawenwu@trustnetic.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
drivers/net/pcs/pcs-xpcs.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 0337e2bcc0125..50b9cdf94fdaa 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -816,9 +816,14 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
if (ret < 0)
return ret;
+ /* Clear CL37 AN complete status */
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
+ if (ret < 0)
+ return ret;
+
if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_BMCR,
- mdio_ctrl | BMCR_ANENABLE);
+ mdio_ctrl | BMCR_ANENABLE | BMCR_ANRESTART);
return ret;
}
@@ -1093,10 +1098,15 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
return 0;
}
- /* Clear AN complete status or interrupt */
- if (state->an_complete)
+ if (state->an_complete) {
+ /* Clear AN complete status or interrupt */
xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
+ /* Initiate the next round of AN */
+ xpcs_modify(xpcs, MDIO_MMD_VEND2, MII_BMCR, BMCR_ANRESTART,
+ BMCR_ANRESTART);
+ }
+
return 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery
2026-09-01 15:01 ` [PATCH net-next v3 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery Coia Prant
@ 2026-09-02 15:04 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-02 15:04 UTC (permalink / raw)
To: Coia Prant
Cc: olteanv, neil.armstrong, vkoul, devicetree, conor+dt, linux-phy,
robh, wsa+renesas
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] State getter `xpcs_get_state_c37_sgmii` mutates hardware state (restarting autonegotiation) and applies a hardware-specific workaround universally to all platforms.
--
commit 71d0b44c6ef52210f70310761abecf6962cd887a
Author: Coia Prant <coiaprant@gmail.com>
net: pcs: xpcs: add ANRESTART support for SGMII link recovery
This commit introduces support for restarting autonegotiation on the DesignWare
XPCS IP to handle link recovery on platforms like the RK3568 MAC side SGMII.
> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index 0337e2bcc0125..50b9cdf94fdaa 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c
[ ... ]
> @@ -1093,10 +1098,15 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
> return 0;
> }
>
> - /* Clear AN complete status or interrupt */
> - if (state->an_complete)
> + if (state->an_complete) {
> + /* Clear AN complete status or interrupt */
> xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
>
> + /* Initiate the next round of AN */
> + xpcs_modify(xpcs, MDIO_MMD_VEND2, MII_BMCR, BMCR_ANRESTART,
> + BMCR_ANRESTART);
> + }
> +
[Severity: High]
Does this mutation inside xpcs_get_state_c37_sgmii violate the phylink API
semantics, which typically require state getters to be side-effect free?
Because the phylink_resolve workqueue polls the state periodically when the
link is down, won't this cause the getter to constantly restart the
autonegotiation state machine and generate continuous MDIO traffic?
For instance, when the SGMII PHY drops the copper link, it sends an SGMII
config word with link=0. The MAC completes autonegotiation based on this word,
meaning state->an_complete is true but state->link is false, which triggers
this exact path.
Additionally, does this apply an RK3568-specific workaround unconditionally to
all Generic DW XPCS controllers? Standard MAC-side SGMII automatically handles
link state updates via the SGMII config word without needing manual AN
restarts. Could this override that conformant behavior on other platforms?
> return 0;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260901150111.141037-1-coiaprant@gmail.com?part=6
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next v3 07/10] net: pcs: xpcs: add Rockchip RK3568 platform glue driver
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
` (5 preceding siblings ...)
2026-09-01 15:01 ` [PATCH net-next v3 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-02 15:04 ` sashiko-bot
2026-09-01 15:01 ` [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568 Coia Prant
` (2 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
The RK3568 SoC integrates a Synopsys DesignWare XPCS that is accessed
via APB3 memory-mapped registers.
This driver provides the glue logic to make the XPCS accessible to
the generic pcs-xpcs core.
The XPCS block contains four MII ports (0..3), each of which can be
routed to GMAC0 or GMAC1 via the pcs-handle property in the MAC node.
The hardware maps these ports to different MMDs:
- port 0: MMD 7 (ROCKCHIP_MMD_MII)
- port 1: MMD 2 (ROCKCHIP_MMD_MII1)
- port 2: MMD 3 (ROCKCHIP_MMD_MII2)
- port 3: MMD 4 (ROCKCHIP_MMD_MII3)
This driver creates a virtual MDIO bus that translates MDIO operations
to APB3 register accesses, with proper address remapping for each port.
The generic xpcs driver then creates a phylink_pcs instance on top of
this bus, allowing the MAC to use the PCS via the standard phylink API.
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf (Page 2078)
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
drivers/net/pcs/Kconfig | 23 ++
drivers/net/pcs/Makefile | 7 +-
drivers/net/pcs/pcs-xpcs-rk.c | 561 ++++++++++++++++++++++++++++++++
include/linux/pcs/pcs-xpcs-rk.h | 11 +
4 files changed, 598 insertions(+), 4 deletions(-)
create mode 100644 drivers/net/pcs/pcs-xpcs-rk.c
create mode 100644 include/linux/pcs/pcs-xpcs-rk.h
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index e417fd66f660a..88bc40791e69f 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -12,6 +12,29 @@ config PCS_XPCS
This module provides a driver and helper functions for Synopsys
DesignWare XPCS controllers.
+if PCS_XPCS
+
+config PCS_XPCS_PLATFORM
+ tristate "Generic XPCS controller support"
+ default PCS_XPCS
+ help
+ Generic DWXPCS driver for platforms that don't require any
+ platform specific code to function or is using platform
+ data for setup.
+
+ If you have a controller with this interface, say Y or M here.
+
+config PCS_XPCS_ROCKCHIP
+ tristate "Rockchip XPCS controller support"
+ default ARCH_ROCKCHIP
+ depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
+ help
+ Support for XPCS controller on Rockchip RK356x SoC.
+
+ If you have a Rockchip SoC with this interface, say Y or M here.
+
+endif # PCS_XPCS
+
config PCS_LYNX
tristate
help
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index 4f7920618b900..c809b7f942a51 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -1,10 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for Linux PCS drivers
-pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-plat.o \
- pcs-xpcs-nxp.o pcs-xpcs-wx.o
-
-obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o
+obj-$(CONFIG_PCS_XPCS) += pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-wx.o
+obj-$(CONFIG_PCS_XPCS_PLATFORM) += pcs-xpcs-plat.o
+obj-$(CONFIG_PCS_XPCS_ROCKCHIP) += pcs-xpcs-rk.o
obj-$(CONFIG_PCS_LYNX) += pcs-lynx.o
obj-$(CONFIG_PCS_MTK_LYNXI) += pcs-mtk-lynxi.o
obj-$(CONFIG_PCS_RZN1_MIIC) += pcs-rzn1-miic.o
diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
new file mode 100644
index 0000000000000..4ef20b093dada
--- /dev/null
+++ b/drivers/net/pcs/pcs-xpcs-rk.c
@@ -0,0 +1,561 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockchip XPCS platform device driver
+ *
+ * Based on the Synopsys DesignWare XPCS platform driver.
+ * Copyright (C) 2024 Serge Semin
+ *
+ * Adapted for Rockchip SoCs, with reference to the Rockchip OEM driver.
+ * Copyright (C) 2026 Coia Prant
+ */
+
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pcs/pcs-xpcs-rk.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/sizes.h>
+
+#include "pcs-xpcs.h"
+
+struct dw_xpcs_rk {
+ struct platform_device *pdev;
+ struct mii_bus *bus;
+ void __iomem *reg_base;
+ struct phy *serdes_phy;
+ struct clk *csr_clk;
+ struct clk *eee_clk;
+};
+
+static ptrdiff_t xpcs_rk_addr_format(int dev, int reg)
+{
+ return FIELD_PREP(0x70000, dev) | FIELD_PREP(0xffff, reg);
+}
+
+static int xpcs_rk_read_reg(struct dw_xpcs_rk *pxpcs, int dev, int reg)
+{
+ ptrdiff_t csr;
+ int ret;
+
+ csr = xpcs_rk_addr_format(dev, reg);
+
+ ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = readl(pxpcs->reg_base + (csr << 2)) & 0xffff;
+
+ pm_runtime_put(&pxpcs->pdev->dev);
+ return ret;
+}
+
+static int xpcs_rk_write_reg(struct dw_xpcs_rk *pxpcs, int dev, int reg, u16 val)
+{
+ ptrdiff_t csr;
+ int ret;
+
+ csr = xpcs_rk_addr_format(dev, reg);
+
+ ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+ if (ret)
+ return ret;
+
+ writel(val, pxpcs->reg_base + (csr << 2));
+
+ pm_runtime_put(&pxpcs->pdev->dev);
+ return 0;
+}
+
+#define ROCKCHIP_MMD_MII1 2
+#define ROCKCHIP_MMD_MII2 3
+#define ROCKCHIP_MMD_MII3 4
+#define ROCKCHIP_MMD_PMAPMD 6
+#define ROCKCHIP_MMD_MII 7
+
+static bool xpcs_rk_mdio_addr_validate(int addr)
+{
+ return !(addr < 0 || addr > 3);
+}
+
+static int xpcs_rk_mdio_read_remapping(int addr, int dev, int reg)
+{
+ switch (dev) {
+ case MDIO_MMD_PMAPMD:
+ return ROCKCHIP_MMD_PMAPMD;
+ case MDIO_MMD_VEND2:
+ break;
+ default:
+ return -ENXIO;
+ }
+
+ /* read remapping to MII is performed by HW */
+ switch (addr) {
+ case 0:
+ return ROCKCHIP_MMD_MII;
+ case 1:
+ return ROCKCHIP_MMD_MII1;
+ case 2:
+ return ROCKCHIP_MMD_MII2;
+ case 3:
+ return ROCKCHIP_MMD_MII3;
+ default:
+ return -ENODEV;
+ }
+}
+
+static int xpcs_rk_mdio_write_remapping(int addr, int dev, int reg)
+{
+ switch (dev) {
+ case MDIO_MMD_PMAPMD:
+ return ROCKCHIP_MMD_PMAPMD;
+ case MDIO_MMD_VEND2:
+ break;
+ default:
+ return -ENXIO;
+ }
+
+ /* Writable only on MII */
+ switch (reg) {
+ case DW_VR_MII_AN_CTRL:
+ case DW_VR_MII_AN_INTR_STS:
+ case DW_VR_MII_EEE_MCTRL0:
+ case DW_VR_MII_EEE_MCTRL1:
+ case DW_VR_MII_DIG_CTRL2:
+ return ROCKCHIP_MMD_MII;
+ default:
+ break;
+ }
+
+ switch (addr) {
+ case 0:
+ return ROCKCHIP_MMD_MII;
+ case 1:
+ return ROCKCHIP_MMD_MII1;
+ case 2:
+ return ROCKCHIP_MMD_MII2;
+ case 3:
+ return ROCKCHIP_MMD_MII3;
+ default:
+ return -ENODEV;
+ }
+}
+
+static int xpcs_rk_read_c22(struct mii_bus *bus, int addr, int reg)
+{
+ struct dw_xpcs_rk *pxpcs = bus->priv;
+ int dev;
+
+ if (!xpcs_rk_mdio_addr_validate(addr))
+ return -ENODEV;
+
+ dev = xpcs_rk_mdio_read_remapping(addr, MDIO_MMD_VEND2, reg);
+ if (dev < 0)
+ return 0xffff;
+
+ return xpcs_rk_read_reg(pxpcs, dev, reg);
+}
+
+static int xpcs_rk_write_c22(struct mii_bus *bus, int addr, int reg, u16 val)
+{
+ struct dw_xpcs_rk *pxpcs = bus->priv;
+ int dev;
+
+ if (!xpcs_rk_mdio_addr_validate(addr))
+ return -ENODEV;
+
+ dev = xpcs_rk_mdio_write_remapping(addr, MDIO_MMD_VEND2, reg);
+ if (dev < 0)
+ return 0;
+
+ return xpcs_rk_write_reg(pxpcs, dev, reg, val);
+}
+
+static int xpcs_rk_read_c45(struct mii_bus *bus, int addr, int dev, int reg)
+{
+ struct dw_xpcs_rk *pxpcs = bus->priv;
+
+ if (!xpcs_rk_mdio_addr_validate(addr))
+ return -ENODEV;
+
+ dev = xpcs_rk_mdio_read_remapping(addr, dev, reg);
+ if (dev < 0)
+ return 0xffff;
+
+ return xpcs_rk_read_reg(pxpcs, dev, reg);
+}
+
+static int xpcs_rk_write_c45(struct mii_bus *bus, int addr, int dev, int reg, u16 val)
+{
+ struct dw_xpcs_rk *pxpcs = bus->priv;
+
+ if (!xpcs_rk_mdio_addr_validate(addr))
+ return -ENODEV;
+
+ dev = xpcs_rk_mdio_write_remapping(addr, dev, reg);
+ if (dev < 0)
+ return 0;
+
+ return xpcs_rk_write_reg(pxpcs, dev, reg, val);
+}
+
+static struct dw_xpcs_rk *xpcs_rk_create_data(struct platform_device *pdev)
+{
+ struct dw_xpcs_rk *pxpcs;
+
+ pxpcs = devm_kzalloc(&pdev->dev, sizeof(*pxpcs), GFP_KERNEL);
+ if (!pxpcs)
+ return ERR_PTR(-ENOMEM);
+
+ pxpcs->pdev = pdev;
+
+ dev_set_drvdata(&pdev->dev, pxpcs);
+
+ return pxpcs;
+}
+
+static int xpcs_rk_serdes_phy_init(struct dw_xpcs_rk *pxpcs)
+{
+ struct device *dev = &pxpcs->pdev->dev;
+
+ pxpcs->serdes_phy = devm_phy_get(dev, "serdes");
+ if (IS_ERR(pxpcs->serdes_phy))
+ return dev_err_probe(dev, PTR_ERR(pxpcs->serdes_phy),
+ "Failed to get SerDes PHY\n");
+
+ return 0;
+}
+
+static void xpcs_rk_serdes_phy_poweroff(void *data)
+{
+ struct dw_xpcs_rk *pxpcs = data;
+ struct device *dev = &pxpcs->pdev->dev;
+
+ phy_power_off(pxpcs->serdes_phy);
+ phy_exit(pxpcs->serdes_phy);
+
+ dev_pm_genpd_rpm_always_on(dev, false);
+}
+
+static int xpcs_rk_serdes_phy_poweron(struct dw_xpcs_rk *pxpcs)
+{
+ struct device *dev = &pxpcs->pdev->dev;
+ int ret;
+
+ /*
+ * The power domain is required and must be enabled, which allows us to
+ * dynamically turn the CSR clock on/off using PM while keeping the PCS
+ * powered on.
+ */
+ ret = dev_pm_genpd_rpm_always_on(dev, true);
+ if (ret) {
+ dev_err(dev, "Failed to power on power-domains\n");
+ return ret;
+ }
+
+ ret = phy_init(pxpcs->serdes_phy);
+ if (ret) {
+ dev_err(dev, "Failed to init SerDes PHY\n");
+ goto pm_domain;
+ }
+
+ ret = phy_power_on(pxpcs->serdes_phy);
+ if (ret) {
+ dev_err(dev, "Failed to power on SerDes PHY\n");
+ goto serdes_phy;
+ }
+
+ ret = devm_add_action_or_reset(dev, xpcs_rk_serdes_phy_poweroff, pxpcs);
+ if (ret) {
+ dev_err(dev, "Failed to register devm for SerDes PHY: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+
+serdes_phy:
+ phy_exit(pxpcs->serdes_phy);
+pm_domain:
+ dev_pm_genpd_rpm_always_on(dev, false);
+ return ret;
+}
+
+static int xpcs_rk_init_res(struct dw_xpcs_rk *pxpcs)
+{
+ struct platform_device *pdev = pxpcs->pdev;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(dev, "No reg-space found\n");
+ return -EINVAL;
+ }
+
+ if (resource_size(res) < SZ_2M) {
+ dev_err(dev, "Invalid reg-space size\n");
+ return -EINVAL;
+ }
+
+ pxpcs->reg_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(pxpcs->reg_base)) {
+ dev_err(dev, "Failed to map reg-space\n");
+ return PTR_ERR(pxpcs->reg_base);
+ }
+
+ return 0;
+}
+
+static void xpcs_rk_exit_clk(void *data)
+{
+ struct dw_xpcs_rk *pxpcs = data;
+
+ clk_disable_unprepare(pxpcs->eee_clk);
+}
+
+static int xpcs_rk_init_clk(struct dw_xpcs_rk *pxpcs)
+{
+ struct device *dev = &pxpcs->pdev->dev;
+ int ret;
+
+ pxpcs->csr_clk = devm_clk_get(dev, "csr");
+ if (IS_ERR(pxpcs->csr_clk))
+ return dev_err_probe(dev, PTR_ERR(pxpcs->csr_clk),
+ "Failed to get CSR clock\n");
+
+ pm_runtime_set_suspended(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret) {
+ dev_err(dev, "Failed to enable runtime-PM\n");
+ return ret;
+ }
+
+ pxpcs->eee_clk = devm_clk_get(dev, "eee");
+ if (IS_ERR(pxpcs->eee_clk))
+ return dev_err_probe(dev, PTR_ERR(pxpcs->eee_clk),
+ "Failed to get EEE clock\n");
+
+ ret = clk_prepare_enable(pxpcs->eee_clk);
+ if (ret) {
+ dev_err(dev, "Failed to enable EEE clock\n");
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, xpcs_rk_exit_clk, pxpcs);
+ if (ret) {
+ dev_err(dev, "Failed to register devm for EEE clock: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int xpcs_rk_init_bus(struct dw_xpcs_rk *pxpcs)
+{
+ struct device *dev = &pxpcs->pdev->dev;
+ static atomic_t id = ATOMIC_INIT(-1);
+ struct mii_bus *bus;
+ int ret;
+
+ bus = devm_mdiobus_alloc_size(dev, 0);
+ if (!bus)
+ return -ENOMEM;
+
+ bus->name = "Rockchip DW XPCS MCI/APB3";
+ bus->read = xpcs_rk_read_c22;
+ bus->write = xpcs_rk_write_c22;
+ bus->read_c45 = xpcs_rk_read_c45;
+ bus->write_c45 = xpcs_rk_write_c45;
+ bus->phy_mask = ~0;
+ bus->parent = dev;
+ bus->priv = pxpcs;
+
+ snprintf(bus->id, MII_BUS_ID_SIZE,
+ "rockchip_dwxpcs-%x", atomic_inc_return(&id));
+
+ /*
+ * MDIO-bus here serves as just a back-end engine abstracting out
+ * the MDIO and MCI/APB3 IO interfaces utilized for the Rockchip DWXPCS CSRs
+ * access.
+ */
+ ret = devm_mdiobus_register(dev, bus);
+ if (ret) {
+ dev_err(dev, "Failed to create MDIO bus\n");
+ return ret;
+ }
+
+ pxpcs->bus = bus;
+ return 0;
+}
+
+static int xpcs_rk_probe(struct platform_device *pdev)
+{
+ struct dw_xpcs_rk *pxpcs;
+ int ret;
+
+ pxpcs = xpcs_rk_create_data(pdev);
+ if (IS_ERR(pxpcs))
+ return PTR_ERR(pxpcs);
+
+ /*
+ * The XPCS may be attached to a power domain (e.g. PD_PIPE). The domain
+ * must be powered on before any register access, otherwise the SoC will
+ * trigger a synchronous external abort (SError).
+ *
+ * Accessing the XPCS registers also requires a TX clock from the SerDes,
+ * which is needed for the soft reset.
+ */
+ ret = xpcs_rk_serdes_phy_init(pxpcs);
+ if (ret)
+ return ret;
+
+ ret = xpcs_rk_serdes_phy_poweron(pxpcs);
+ if (ret)
+ return ret;
+
+ ret = xpcs_rk_init_res(pxpcs);
+ if (ret)
+ return ret;
+
+ ret = xpcs_rk_init_clk(pxpcs);
+ if (ret)
+ return ret;
+
+ ret = xpcs_rk_init_bus(pxpcs);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void xpcs_rk_remove(struct platform_device *pdev)
+{
+ /*
+ * Force the device into suspend state to gate the CSR clock. This
+ * prevents clock leakage after devm resources are released.
+ *
+ * This is safe because:
+ * 1. The device link (DL_FLAG_AUTOREMOVE_CONSUMER) prevents unbind
+ * while any consumer (MAC) is still attached.
+ * 2. No MDIO access will occur after this point, as the MAC driver
+ * has already been unbound (or we are being removed because no
+ * consumer exists).
+ */
+ pm_runtime_force_suspend(&pdev->dev);
+}
+
+static const struct of_device_id xpcs_rk_of_ids[] = {
+ { .compatible = "rockchip,rk3568-xpcs" },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, xpcs_rk_of_ids);
+
+struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np)
+{
+ struct platform_device *pdev;
+ struct device_node *pcs_np;
+ struct dw_xpcs_rk *pxpcs;
+ struct dw_xpcs *xpcs;
+ u32 port;
+
+ if (!of_device_is_available(np))
+ return ERR_PTR(-ENODEV);
+
+ if (of_property_read_u32(np, "reg", &port))
+ return ERR_PTR(-EINVAL);
+
+ if (!xpcs_rk_mdio_addr_validate((int)port))
+ return ERR_PTR(-EINVAL);
+
+ /* The XPCS pdev is attached to the parent node */
+ pcs_np = of_get_parent(np);
+ if (!pcs_np)
+ return ERR_PTR(-ENODEV);
+
+ if (!of_device_is_available(pcs_np)) {
+ of_node_put(pcs_np);
+ return ERR_PTR(-ENODEV);
+ }
+
+ if (!of_match_node(xpcs_rk_of_ids, pcs_np)) {
+ of_node_put(pcs_np);
+ return ERR_PTR(-EINVAL);
+ }
+
+ pdev = of_find_device_by_node(pcs_np);
+ of_node_put(pcs_np);
+ if (!pdev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ device_lock(&pdev->dev);
+ pxpcs = platform_get_drvdata(pdev);
+ if (!pxpcs || !pxpcs->bus) {
+ device_unlock(&pdev->dev);
+ put_device(&pdev->dev);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ xpcs = xpcs_create_mdiodev(pxpcs->bus, (int)port);
+ device_unlock(&pdev->dev);
+ if (IS_ERR(xpcs)) {
+ put_device(&pdev->dev);
+ return xpcs;
+ }
+
+ if (!device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER)) {
+ xpcs_destroy(xpcs);
+ put_device(&pdev->dev);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ put_device(&pdev->dev);
+ return xpcs;
+}
+EXPORT_SYMBOL_GPL(xpcs_rk_create);
+
+static int xpcs_rk_pm_runtime_suspend(struct device *dev)
+{
+ struct dw_xpcs_rk *pxpcs = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(pxpcs->csr_clk);
+
+ return 0;
+}
+
+static int xpcs_rk_pm_runtime_resume(struct device *dev)
+{
+ struct dw_xpcs_rk *pxpcs = dev_get_drvdata(dev);
+
+ return clk_prepare_enable(pxpcs->csr_clk);
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(xpcs_rk_pm_ops,
+ xpcs_rk_pm_runtime_suspend,
+ xpcs_rk_pm_runtime_resume,
+ NULL);
+
+static struct platform_driver xpcs_rk_driver = {
+ .probe = xpcs_rk_probe,
+ .remove = xpcs_rk_remove,
+ .driver = {
+ .name = "rk_xpcs-dwxpcs",
+ .pm = pm_ptr(&xpcs_rk_pm_ops),
+ .of_match_table = xpcs_rk_of_ids,
+ },
+};
+module_platform_driver(xpcs_rk_driver);
+
+MODULE_DESCRIPTION("Rockchip XPCS platform device driver");
+MODULE_AUTHOR("Coia Prant <coiaprant@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pcs/pcs-xpcs-rk.h b/include/linux/pcs/pcs-xpcs-rk.h
new file mode 100644
index 0000000000000..28723d5bd75cc
--- /dev/null
+++ b/include/linux/pcs/pcs-xpcs-rk.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_PCS_XPCS_ROCKCHIP_H
+#define __LINUX_PCS_XPCS_ROCKCHIP_H
+
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/pcs/pcs-xpcs.h>
+
+struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np);
+
+#endif /* __LINUX_PCS_XPCS_ROCKCHIP_H */
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 07/10] net: pcs: xpcs: add Rockchip RK3568 platform glue driver
2026-09-01 15:01 ` [PATCH net-next v3 07/10] net: pcs: xpcs: add Rockchip RK3568 platform glue driver Coia Prant
@ 2026-09-02 15:04 ` sashiko-bot
2026-09-03 10:15 ` Coia Prant
0 siblings, 1 reply; 24+ messages in thread
From: sashiko-bot @ 2026-09-02 15:04 UTC (permalink / raw)
To: Coia Prant
Cc: neil.armstrong, linux-phy, olteanv, vkoul, devicetree, robh,
conor+dt, wsa+renesas
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Hardware clock gated while MDIO bus is still active, leading to SError panic
- [High] TOCTOU race in xpcs_rk_create() allows use-after-free of MDIO bus
- [High] Missing PM dependency causes SError crash when CONFIG_PM is disabled
- [High] Unbalanced pm_runtime_disable() depth permanently breaks device on rebind
- [Low] Build failure due to split composite module when CONFIG_PCS_XPCS=m
--
commit 813e502fc8133a43782c6cdc92f677a818bf8605
Author: Coia Prant <coiaprant@gmail.com>
net: pcs: xpcs: add Rockchip RK3568 platform glue driver
The RK3568 SoC integrates a Synopsys DesignWare XPCS accessed via APB3
memory-mapped registers. This driver provides the glue logic to make the
XPCS accessible to the generic pcs-xpcs core.
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
> --- a/drivers/net/pcs/Kconfig
> +++ b/drivers/net/pcs/Kconfig
> @@ -12,6 +12,29 @@ config PCS_XPCS
[ ... ]
> +config PCS_XPCS_ROCKCHIP
> + tristate "Rockchip XPCS controller support"
> + default ARCH_ROCKCHIP
> + depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
[Severity: High]
Is this missing a PM dependency?
If the kernel is built with CONFIG_PM disabled, pm_runtime_resume_and_get()
returns 0 without enabling the hardware clock. This could lead to a
synchronous external abort (SError) when the unclocked APB registers are
subsequently accessed in xpcs_rk_read_reg().
> diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> --- a/drivers/net/pcs/Makefile
> +++ b/drivers/net/pcs/Makefile
> @@ -1,10 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> # Makefile for Linux PCS drivers
>
> -pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-plat.o \
> - pcs-xpcs-nxp.o pcs-xpcs-wx.o
> -
> -obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o
> +obj-$(CONFIG_PCS_XPCS) += pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-wx.o
[Severity: Low]
Does this syntax create separate modules instead of a composite module?
Because pcs-xpcs.c references functions like nxp_sja1105_sgmii_pma_config
defined in pcs-xpcs-nxp.c without EXPORT_SYMBOL being present, creating
separate modules will break symbol resolution and cause modpost linkage
failures when CONFIG_PCS_XPCS=m.
> diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-xpcs-rk.c
[ ... ]
> +static int xpcs_rk_init_clk(struct dw_xpcs_rk *pxpcs)
> +{
[ ... ]
> + pm_runtime_set_suspended(dev);
> + ret = devm_pm_runtime_enable(dev);
[Severity: High]
This devm action schedules pm_runtime_disable() to run during devres
cleanup on device removal.
[ ... ]
> +static void xpcs_rk_remove(struct platform_device *pdev)
> +{
[ ... ]
> + pm_runtime_force_suspend(&pdev->dev);
> +}
[Severity: High]
Will calling pm_runtime_force_suspend() here cause an unbalanced disable
depth?
Since pm_runtime_force_suspend() internally calls pm_runtime_disable(), the
disable depth is incremented twice during removal (once here, and once
during the devres cleanup). When the driver is rebound, it will only be
decremented once, leaving the depth at 1 and permanently breaking runtime
PM hardware access.
[Severity: High]
Can a concurrent sysfs MDIO access race with xpcs_rk_remove() and crash the
system?
Because the MDIO bus is registered with devm_mdiobus_register(), it remains
active and accessible until devres cleanup runs after xpcs_rk_remove()
returns. Calling pm_runtime_force_suspend() gates the hardware clock while
the MDIO bus is still exposed to userspace. A concurrent sysfs read could
increment the usage counter, call into xpcs_rk_read_reg(), and execute
readl() on unclocked hardware, triggering an SError panic.
[ ... ]
> +struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np)
> +{
[ ... ]
> + xpcs = xpcs_create_mdiodev(pxpcs->bus, (int)port);
> + device_unlock(&pdev->dev);
> + if (IS_ERR(xpcs)) {
> + put_device(&pdev->dev);
> + return xpcs;
> + }
> +
> + if (!device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER)) {
[Severity: High]
Is there a race window between device_unlock() and device_link_add()?
If a concurrent unbind of the supplier proceeds after the lock is dropped
but before device_link_add() creates the dependency, the devm-managed
pxpcs->bus could be torn down and freed. This would allow the MAC driver
to continue probing with a freed MDIO bus, leading to a use-after-free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260901150111.141037-1-coiaprant@gmail.com?part=7
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 07/10] net: pcs: xpcs: add Rockchip RK3568 platform glue driver
2026-09-02 15:04 ` sashiko-bot
@ 2026-09-03 10:15 ` Coia Prant
0 siblings, 0 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-03 10:15 UTC (permalink / raw)
To: sashiko-reviews
Cc: neil.armstrong, linux-phy, olteanv, vkoul, devicetree, robh,
conor+dt, wsa+renesas
<sashiko-bot@kernel.org> 于2026年9月2日周三 23:04写道:
> > diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
> > --- a/drivers/net/pcs/Kconfig
> > +++ b/drivers/net/pcs/Kconfig
> > @@ -12,6 +12,29 @@ config PCS_XPCS
> [ ... ]
> > +config PCS_XPCS_ROCKCHIP
> > + tristate "Rockchip XPCS controller support"
> > + default ARCH_ROCKCHIP
> > + depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
>
> [Severity: High]
> Is this missing a PM dependency?
>
> If the kernel is built with CONFIG_PM disabled, pm_runtime_resume_and_get()
> returns 0 without enabling the hardware clock. This could lead to a
> synchronous external abort (SError) when the unclocked APB registers are
> subsequently accessed in xpcs_rk_read_reg().
ARCH_ROCKCHIP already selects CONFIG_PM.
> > diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> > --- a/drivers/net/pcs/Makefile
> > +++ b/drivers/net/pcs/Makefile
> > @@ -1,10 +1,9 @@
> > # SPDX-License-Identifier: GPL-2.0
> > # Makefile for Linux PCS drivers
> >
> > -pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-plat.o \
> > - pcs-xpcs-nxp.o pcs-xpcs-wx.o
> > -
> > -obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o
> > +obj-$(CONFIG_PCS_XPCS) += pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-wx.o
>
> [Severity: Low]
> Does this syntax create separate modules instead of a composite module?
>
> Because pcs-xpcs.c references functions like nxp_sja1105_sgmii_pma_config
> defined in pcs-xpcs-nxp.c without EXPORT_SYMBOL being present, creating
> separate modules will break symbol resolution and cause modpost linkage
> failures when CONFIG_PCS_XPCS=m.
Sure, I will revert it in next version.
> > diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
> > --- /dev/null
> > +++ b/drivers/net/pcs/pcs-xpcs-rk.c
> [ ... ]
> > +static int xpcs_rk_init_clk(struct dw_xpcs_rk *pxpcs)
> > +{
> [ ... ]
> > + pm_runtime_set_suspended(dev);
> > + ret = devm_pm_runtime_enable(dev);
>
> [Severity: High]
> This devm action schedules pm_runtime_disable() to run during devres
> cleanup on device removal.
Yes, I will fix it in next version.
> [ ... ]
> > +static void xpcs_rk_remove(struct platform_device *pdev)
> > +{
> [ ... ]
> > + pm_runtime_force_suspend(&pdev->dev);
> > +}
>
> [Severity: High]
> Will calling pm_runtime_force_suspend() here cause an unbalanced disable
> depth?
>
> Since pm_runtime_force_suspend() internally calls pm_runtime_disable(), the
> disable depth is incremented twice during removal (once here, and once
> during the devres cleanup). When the driver is rebound, it will only be
> decremented once, leaving the depth at 1 and permanently breaking runtime
> PM hardware access.
Yes, I will fix it in next version.
> [Severity: High]
> Can a concurrent sysfs MDIO access race with xpcs_rk_remove() and crash the
> system?
>
> Because the MDIO bus is registered with devm_mdiobus_register(), it remains
> active and accessible until devres cleanup runs after xpcs_rk_remove()
> returns. Calling pm_runtime_force_suspend() gates the hardware clock while
> the MDIO bus is still exposed to userspace. A concurrent sysfs read could
> increment the usage counter, call into xpcs_rk_read_reg(), and execute
> readl() on unclocked hardware, triggering an SError panic.
Yes, I will fix it in next version.
> [ ... ]
> > +struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np)
> > +{
> [ ... ]
> > + xpcs = xpcs_create_mdiodev(pxpcs->bus, (int)port);
> > + device_unlock(&pdev->dev);
> > + if (IS_ERR(xpcs)) {
> > + put_device(&pdev->dev);
> > + return xpcs;
> > + }
> > +
> > + if (!device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER)) {
>
> [Severity: High]
> Is there a race window between device_unlock() and device_link_add()?
>
> If a concurrent unbind of the supplier proceeds after the lock is dropped
> but before device_link_add() creates the dependency, the devm-managed
> pxpcs->bus could be torn down and freed. This would allow the MAC driver
> to continue probing with a freed MDIO bus, leading to a use-after-free.
of_find_device_by_node() increments the reference count of pdev->dev,
so the supplier cannot be unbound between the lock release and
device_link_add(). The put_device() is only called after device_link_add()
completes.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
` (6 preceding siblings ...)
2026-09-01 15:01 ` [PATCH net-next v3 07/10] net: pcs: xpcs: add Rockchip RK3568 platform glue driver Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-02 15:04 ` sashiko-bot
` (2 more replies)
2026-09-01 15:01 ` [PATCH net-next v3 09/10] arm64: dts: rockchip: rk3568-photonicat: enable SGMII LAN port Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 10/10] MAINTAINERS: add entry for Rockchip XPCS driver Coia Prant
9 siblings, 3 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
The RK3568 SoC integrates a Synopsys DesignWare XPCS that can be
connected to GMAC0 or GMAC1 in SGMII mode. Add the necessary glue
logic to support this configuration.
The current dwmac-rk driver does not support SGMII mode. SGMII
requires a PCS to handle auto-negotiation and link state reporting,
but the existing driver only supports RGMII and RMII.
Add a set_to_sgmii() callback to configure the GMAC GRF register for
SGMII mode (bit 7 set, interface selection bits 4:6 cleared). Also
add a supports_sgmii flag to indicate SGMII capability.
Provide pcs_init/pcs_exit callbacks to create/destroy the XPCS via
xpcs_rk_create() from the Rockchip XPCS platform driver, and a
select_pcs callback to return the XPCS to phylink.
While at it, fix the clock enable ordering in rk_gmac_powerup():
gmac_clk_enable() is now called before any register access, including
the SGMII mode setup path. Previously SGMII mode would bypass the
clock enable via a goto, which could cause synchronous external abort
when accessing MAC registers with the clock domain disabled.
Also clean up the error handling in rk_gmac_powerup() by using a
unified clk_disable label, and add error handling for the default
(unhandled interface) case.
SGMII In-band vs Out-of-band
============================
On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
changed by the stmmac core's set_clk_tx_rate callback. In-band mode
works because the PCS handles rate adaptation internally. Out-of-band
mode does not work because the MAC would need to change the clock rate
to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
is fixed.
Enable default_an_inband for SGMII and disable the generic stmmac
set_clk_tx_rate callback. This forces phylink to use in-band mode,
where the PCS is responsible for speed/duplex negotiation. Without
this, the stmmac core would attempt to change the clock rate on speed
changes, causing TX to work but RX to fail.
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
.../net/ethernet/stmicro/stmmac/dwmac-rk.c | 121 +++++++++++++++---
2 files changed, 104 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index e3dd5adda5aca..5088acc06982e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -170,6 +170,7 @@ config DWMAC_ROCKCHIP
default ARCH_ROCKCHIP
depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
select MFD_SYSCON
+ select PCS_XPCS_ROCKCHIP
help
Support for Ethernet controller on Rockchip RK3288 SoC.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 8d7042e689261..e47ca1bec5b8b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -20,6 +20,7 @@
#include <linux/delay.h>
#include <linux/mfd/syscon.h>
#include <linux/regmap.h>
+#include <linux/pcs/pcs-xpcs-rk.h>
#include <linux/pm_runtime.h>
#include "stmmac_platform.h"
@@ -47,6 +48,7 @@ struct rk_gmac_ops {
void (*set_to_rgmii)(struct rk_priv_data *bsp_priv,
int tx_delay, int rx_delay);
void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
+ void (*set_to_sgmii)(struct rk_priv_data *bsp_priv);
int (*set_speed)(struct rk_priv_data *bsp_priv,
phy_interface_t interface, int speed);
void (*integrated_phy_powerup)(struct rk_priv_data *bsp_priv);
@@ -63,6 +65,7 @@ struct rk_gmac_ops {
bool clock_grf_reg_in_php;
bool supports_rgmii;
bool supports_rmii;
+ bool supports_sgmii;
bool php_grf_required;
bool regs_valid;
u32 regs[];
@@ -98,6 +101,7 @@ struct rk_priv_data {
bool integrated_phy;
bool supports_rgmii;
bool supports_rmii;
+ bool supports_sgmii;
struct clk_bulk_data *clks;
int num_clks;
@@ -809,6 +813,8 @@ static const struct rk_gmac_ops rk3528_ops = {
#define RK3568_GRF_GMAC1_CON1 0x038c
/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
+#define RK3568_GMAC_MODE_RMII_RGMII GRF_CLR_BIT(7)
+#define RK3568_GMAC_MODE_SGMII_QSGMII GRF_BIT(7)
#define RK3568_GMAC_FLOW_CTRL GRF_BIT(3)
#define RK3568_GMAC_FLOW_CTRL_CLR GRF_CLR_BIT(3)
#define RK3568_GMAC_RXCLK_DLY_ENABLE GRF_BIT(1)
@@ -851,18 +857,32 @@ static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
regmap_write(bsp_priv->grf, con1,
+ RK3568_GMAC_MODE_RMII_RGMII |
RK3568_GMAC_RXCLK_DLY_ENABLE |
RK3568_GMAC_TXCLK_DLY_ENABLE);
}
+static void rk3568_set_to_sgmii(struct rk_priv_data *bsp_priv)
+{
+ u32 con1;
+
+ con1 = (bsp_priv->id == 1) ? RK3568_GRF_GMAC1_CON1 :
+ RK3568_GRF_GMAC0_CON1;
+
+ regmap_write(bsp_priv->grf, con1, RK3568_GMAC_MODE_SGMII_QSGMII);
+}
+
static const struct rk_gmac_ops rk3568_ops = {
.init = rk3568_init,
.set_to_rgmii = rk3568_set_to_rgmii,
+ .set_to_sgmii = rk3568_set_to_sgmii,
+
.set_speed = rk_set_clk_mac_speed,
.gmac_phy_intf_sel_mask = GENMASK_U16(6, 4),
.supports_rmii = true,
+ .supports_sgmii = true,
.regs_valid = true,
.regs = {
@@ -1208,6 +1228,43 @@ static void rk_phy_powerdown(struct rk_priv_data *bsp_priv)
dev_err(bsp_priv->dev, "fail to disable phy-supply\n");
}
+static int rk_pcs_init(struct stmmac_priv *priv)
+{
+ struct device_node *np = priv->device->of_node;
+ struct device_node *pcs_node;
+ struct dw_xpcs *xpcs;
+
+ pcs_node = of_parse_phandle(np, "pcs-handle", 0);
+ if (!pcs_node)
+ return -ENODEV;
+
+ xpcs = xpcs_rk_create(priv->device, pcs_node);
+ of_node_put(pcs_node);
+ if (IS_ERR(xpcs))
+ return PTR_ERR(xpcs);
+
+ priv->hw->xpcs = xpcs;
+ return 0;
+}
+
+static void rk_pcs_exit(struct stmmac_priv *priv)
+{
+ if (!priv->hw->xpcs)
+ return;
+
+ xpcs_destroy(priv->hw->xpcs);
+ priv->hw->xpcs = NULL;
+}
+
+static struct phylink_pcs *rk_select_pcs(struct stmmac_priv *priv,
+ phy_interface_t interface)
+{
+ if (!priv->hw->xpcs)
+ return NULL;
+
+ return xpcs_to_phylink_pcs(priv->hw->xpcs);
+}
+
static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
struct plat_stmmacenet_data *plat,
const struct rk_gmac_ops *ops)
@@ -1330,6 +1387,7 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
bsp_priv->supports_rgmii = ops->supports_rgmii || !!ops->set_to_rgmii;
bsp_priv->supports_rmii = ops->supports_rmii || !!ops->set_to_rmii;
+ bsp_priv->supports_sgmii = ops->supports_sgmii || !!ops->set_to_sgmii;
if (ops->init) {
ret = ops->init(bsp_priv);
@@ -1361,6 +1419,10 @@ static int rk_gmac_check_ops(struct rk_priv_data *bsp_priv)
if (!bsp_priv->supports_rmii)
return -EINVAL;
break;
+ case PHY_INTERFACE_MODE_SGMII:
+ if (!bsp_priv->supports_sgmii)
+ return -EINVAL;
+ break;
default:
dev_err(bsp_priv->dev,
"unsupported interface %d", bsp_priv->phy_iface);
@@ -1379,16 +1441,19 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
if (ret)
return ret;
+ ret = gmac_clk_enable(bsp_priv, true);
+ if (ret)
+ return ret;
+
+ if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_SGMII)
+ goto set_mode;
+
ret = rk_get_phy_intf_sel(bsp_priv->phy_iface);
if (ret < 0)
- return ret;
+ goto clk_disable;
intf = ret;
- ret = gmac_clk_enable(bsp_priv, true);
- if (ret)
- return ret;
-
if (bsp_priv->gmac_phy_intf_sel_mask ||
bsp_priv->gmac_rmii_mode_mask) {
/* If defined, encode the phy_intf_sel value */
@@ -1399,10 +1464,8 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
bsp_priv->gmac_rmii_mode_mask);
ret = rk_write_gmac_grf_reg(bsp_priv, val);
- if (ret < 0) {
- gmac_clk_enable(bsp_priv, false);
- return ret;
- }
+ if (ret < 0)
+ goto clk_disable;
}
if (bsp_priv->clock.rmii_mode_mask) {
@@ -1410,13 +1473,12 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
bsp_priv->clock.rmii_mode_mask);
ret = rk_write_clock_grf_reg(bsp_priv, val);
- if (ret < 0) {
- gmac_clk_enable(bsp_priv, false);
- return ret;
- }
+ if (ret < 0)
+ goto clk_disable;
}
- /*rmii or rgmii*/
+set_mode:
+ /* rmii, rgmii, sgmii */
switch (bsp_priv->phy_iface) {
case PHY_INTERFACE_MODE_RGMII:
dev_info(dev, "init for RGMII\n");
@@ -1447,15 +1509,20 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
if (bsp_priv->ops->set_to_rmii)
bsp_priv->ops->set_to_rmii(bsp_priv);
break;
+ case PHY_INTERFACE_MODE_SGMII:
+ dev_info(dev, "init for SGMII\n");
+ if (bsp_priv->ops->set_to_sgmii)
+ bsp_priv->ops->set_to_sgmii(bsp_priv);
+ break;
default:
dev_err(dev, "NO interface defined!\n");
+ ret = -EINVAL;
+ goto clk_disable;
}
ret = rk_phy_powerup(bsp_priv);
- if (ret) {
- gmac_clk_enable(bsp_priv, false);
- return ret;
- }
+ if (ret)
+ goto clk_disable;
pm_runtime_get_sync(dev);
@@ -1463,6 +1530,10 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
bsp_priv->ops->integrated_phy_powerup(bsp_priv);
return 0;
+
+clk_disable:
+ gmac_clk_enable(bsp_priv, false);
+ return ret;
}
static void rk_gmac_powerdown(struct rk_priv_data *gmac)
@@ -1486,6 +1557,9 @@ static void rk_get_interfaces(struct stmmac_priv *priv, void *bsp_priv,
if (rk->supports_rmii)
__set_bit(PHY_INTERFACE_MODE_RMII, interfaces);
+
+ if (rk->supports_sgmii)
+ __set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
}
static int rk_set_clk_tx_rate(void *bsp_priv_, struct clk *clk_tx_i,
@@ -1602,6 +1676,17 @@ static int rk_gmac_probe(struct platform_device *pdev)
plat_dat->suspend = rk_gmac_suspend;
plat_dat->resume = rk_gmac_resume;
+ if (plat_dat->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+ /* SGMII clock always runs at 125 MHz */
+ plat_dat->set_clk_tx_rate = NULL;
+
+ /* SGMII requires a PCS */
+ plat_dat->default_an_inband = true;
+ plat_dat->pcs_init = rk_pcs_init;
+ plat_dat->pcs_exit = rk_pcs_exit;
+ plat_dat->select_pcs = rk_select_pcs;
+ }
+
plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
if (IS_ERR(plat_dat->bsp_priv))
return PTR_ERR(plat_dat->bsp_priv);
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568
2026-09-01 15:01 ` [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568 Coia Prant
@ 2026-09-02 15:04 ` sashiko-bot
2026-09-03 8:34 ` Maxime Chevallier
2026-09-03 8:59 ` Maxime Chevallier
2 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-02 15:04 UTC (permalink / raw)
To: Coia Prant
Cc: vkoul, wsa+renesas, robh, devicetree, neil.armstrong, olteanv,
conor+dt, linux-phy
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Hardware initialization failure: The interface selection bits 4:6 are not cleared for SGMII mode, contradicting the commit message and leaving the MAC in an invalid state.
Pre-existing issues:
- [High] Unhandled Error: pm_runtime_get_sync() return value is ignored, potentially causing hardware access without proper resume.
--
commit 5a7c3b38ba957e74b26623c8e3e94e4c77b594de
Author: Coia Prant <coiaprant@gmail.com>
net: stmmac: dwmac-rk: add SGMII support for RK3568
This commit adds support for SGMII mode on the RK3568 SoC by providing the
necessary glue logic and XPCS configuration. It introduces callbacks to set
up the GMAC GRF register and handle PCS initialization, while also fixing
clock enable ordering in the powerup sequence.
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
[ ... ]
> @@ -851,18 +857,32 @@ static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
>
> regmap_write(bsp_priv->grf, con1,
> + RK3568_GMAC_MODE_RMII_RGMII |
> RK3568_GMAC_RXCLK_DLY_ENABLE |
> RK3568_GMAC_TXCLK_DLY_ENABLE);
> }
>
> +static void rk3568_set_to_sgmii(struct rk_priv_data *bsp_priv)
> +{
> + u32 con1;
> +
> + con1 = (bsp_priv->id == 1) ? RK3568_GRF_GMAC1_CON1 :
> + RK3568_GRF_GMAC0_CON1;
> +
> + regmap_write(bsp_priv->grf, con1, RK3568_GMAC_MODE_SGMII_QSGMII);
> +}
[Severity: High]
The commit message mentions "bit 7 set, interface selection bits 4:6 cleared",
but this write only targets bit 7 using RK3568_GMAC_MODE_SGMII_QSGMII (which
is GRF_BIT(7)). Does this leave the interface selection bits 4:6 in their
previous state instead of clearing them? Could this cause the SGMII interface
to fail to initialize properly?
[ ... ]
> @@ -1379,16 +1441,19 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
> if (ret)
> return ret;
>
> + ret = gmac_clk_enable(bsp_priv, true);
> + if (ret)
> + return ret;
> +
> + if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_SGMII)
> + goto set_mode;
[Severity: High]
Because PHY_INTERFACE_MODE_SGMII takes this goto set_mode, it skips the
generic GRF interface selection mask configuration right below it. Since
rk3568_set_to_sgmii() also doesn't clear bits 4:6, is there anywhere that the
clearing mask is actually applied for SGMII?
> +
> ret = rk_get_phy_intf_sel(bsp_priv->phy_iface);
> if (ret < 0)
> - return ret;
> + goto clk_disable;
>
> intf = ret;
>
[ ... ]
> @@ -1447,15 +1509,20 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
> if (bsp_priv->ops->set_to_rmii)
> bsp_priv->ops->set_to_rmii(bsp_priv);
> break;
> + case PHY_INTERFACE_MODE_SGMII:
> + dev_info(dev, "init for SGMII\n");
> + if (bsp_priv->ops->set_to_sgmii)
> + bsp_priv->ops->set_to_sgmii(bsp_priv);
> + break;
> default:
> dev_err(dev, "NO interface defined!\n");
> + ret = -EINVAL;
> + goto clk_disable;
> }
>
> ret = rk_phy_powerup(bsp_priv);
> - if (ret) {
> - gmac_clk_enable(bsp_priv, false);
> - return ret;
> - }
> + if (ret)
> + goto clk_disable;
>
> pm_runtime_get_sync(dev);
>
> @@ -1463,6 +1530,10 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
> bsp_priv->ops->integrated_phy_powerup(bsp_priv);
>
> return 0;
[Severity: High]
This is a pre-existing issue, but the return value of pm_runtime_get_sync()
is ignored here. If the device power domain fails to resume, would the
subsequent integrated_phy_powerup() hardware access trigger a synchronous
external abort or cause silent data corruption?
> +
> +clk_disable:
> + gmac_clk_enable(bsp_priv, false);
> + return ret;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260901150111.141037-1-coiaprant@gmail.com?part=8
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568
2026-09-01 15:01 ` [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568 Coia Prant
2026-09-02 15:04 ` sashiko-bot
@ 2026-09-03 8:34 ` Maxime Chevallier
2026-09-03 8:38 ` Coia Prant
2026-09-03 8:59 ` Maxime Chevallier
2 siblings, 1 reply; 24+ messages in thread
From: Maxime Chevallier @ 2026-09-03 8:34 UTC (permalink / raw)
To: Coia Prant, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Vinod Koul, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
Hi
On 9/1/26 17:01, Coia Prant wrote:
> The RK3568 SoC integrates a Synopsys DesignWare XPCS that can be
> connected to GMAC0 or GMAC1 in SGMII mode. Add the necessary glue
> logic to support this configuration.
>
> The current dwmac-rk driver does not support SGMII mode. SGMII
> requires a PCS to handle auto-negotiation and link state reporting,
> but the existing driver only supports RGMII and RMII.
>
> Add a set_to_sgmii() callback to configure the GMAC GRF register for
> SGMII mode (bit 7 set, interface selection bits 4:6 cleared). Also
> add a supports_sgmii flag to indicate SGMII capability.
>
> Provide pcs_init/pcs_exit callbacks to create/destroy the XPCS via
> xpcs_rk_create() from the Rockchip XPCS platform driver, and a
> select_pcs callback to return the XPCS to phylink.
>
> While at it, fix the clock enable ordering in rk_gmac_powerup():
> gmac_clk_enable() is now called before any register access, including
> the SGMII mode setup path. Previously SGMII mode would bypass the
> clock enable via a goto, which could cause synchronous external abort
> when accessing MAC registers with the clock domain disabled.
>
> Also clean up the error handling in rk_gmac_powerup() by using a
> unified clk_disable label, and add error handling for the default
> (unhandled interface) case.
>
> SGMII In-band vs Out-of-band
> ============================
> On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
> changed by the stmmac core's set_clk_tx_rate callback. In-band mode
> works because the PCS handles rate adaptation internally. Out-of-band
> mode does not work because the MAC would need to change the clock rate
> to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
> is fixed.
>
> Enable default_an_inband for SGMII and disable the generic stmmac
> set_clk_tx_rate callback. This forces phylink to use in-band mode,
> where the PCS is responsible for speed/duplex negotiation. Without
> this, the stmmac core would attempt to change the clock rate on speed
> changes, causing TX to work but RX to fail.
>
> Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
[...]
> +static int rk_pcs_init(struct stmmac_priv *priv)
> +{
> + struct device_node *np = priv->device->of_node;
> + struct device_node *pcs_node;
> + struct dw_xpcs *xpcs;
> +
> + pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> + if (!pcs_node)
> + return -ENODEV;
Here aswell you make it mandatory to have a PCS, as the generic pcs logic
introduced in patch 1 doesn't handle -ENODEV, it treats it as any other
error.
So, either you return 0 when there's no PCS (so that we don't break platforms
that don't have one), or you handle -ENODEV gracefully in patch 1.
Maxime
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568
2026-09-03 8:34 ` Maxime Chevallier
@ 2026-09-03 8:38 ` Coia Prant
2026-09-03 8:44 ` Maxime Chevallier
0 siblings, 1 reply; 24+ messages in thread
From: Coia Prant @ 2026-09-03 8:38 UTC (permalink / raw)
To: Maxime Chevallier
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Coquelin, Alexandre Torgue,
Lad Prabhakar, Romain Gantois, Heiner Kallweit, Neil Armstrong,
Russell King, Shawn Lin, David Heidelberg, netdev, linux-rockchip,
devicetree, linux-arm-kernel, linux-kernel, linux-phy,
linux-stm32, linux-renesas-soc
Maxime Chevallier <maxime.chevallier@bootlin.com> 于2026年9月3日周四 16:34写道:
>
> Hi
>
> On 9/1/26 17:01, Coia Prant wrote:
> > The RK3568 SoC integrates a Synopsys DesignWare XPCS that can be
> > connected to GMAC0 or GMAC1 in SGMII mode. Add the necessary glue
> > logic to support this configuration.
> >
> > The current dwmac-rk driver does not support SGMII mode. SGMII
> > requires a PCS to handle auto-negotiation and link state reporting,
> > but the existing driver only supports RGMII and RMII.
> >
> > Add a set_to_sgmii() callback to configure the GMAC GRF register for
> > SGMII mode (bit 7 set, interface selection bits 4:6 cleared). Also
> > add a supports_sgmii flag to indicate SGMII capability.
> >
> > Provide pcs_init/pcs_exit callbacks to create/destroy the XPCS via
> > xpcs_rk_create() from the Rockchip XPCS platform driver, and a
> > select_pcs callback to return the XPCS to phylink.
> >
> > While at it, fix the clock enable ordering in rk_gmac_powerup():
> > gmac_clk_enable() is now called before any register access, including
> > the SGMII mode setup path. Previously SGMII mode would bypass the
> > clock enable via a goto, which could cause synchronous external abort
> > when accessing MAC registers with the clock domain disabled.
> >
> > Also clean up the error handling in rk_gmac_powerup() by using a
> > unified clk_disable label, and add error handling for the default
> > (unhandled interface) case.
> >
> > SGMII In-band vs Out-of-band
> > ============================
> > On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
> > changed by the stmmac core's set_clk_tx_rate callback. In-band mode
> > works because the PCS handles rate adaptation internally. Out-of-band
> > mode does not work because the MAC would need to change the clock rate
> > to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
> > is fixed.
> >
> > Enable default_an_inband for SGMII and disable the generic stmmac
> > set_clk_tx_rate callback. This forces phylink to use in-band mode,
> > where the PCS is responsible for speed/duplex negotiation. Without
> > this, the stmmac core would attempt to change the clock rate on speed
> > changes, causing TX to work but RX to fail.
> >
> > Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
> > Signed-off-by: Coia Prant <coiaprant@gmail.com>
>
> [...]
>
> > +static int rk_pcs_init(struct stmmac_priv *priv)
> > +{
> > + struct device_node *np = priv->device->of_node;
> > + struct device_node *pcs_node;
> > + struct dw_xpcs *xpcs;
> > +
> > + pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> > + if (!pcs_node)
> > + return -ENODEV;
>
> Here aswell you make it mandatory to have a PCS, as the generic pcs logic
> introduced in patch 1 doesn't handle -ENODEV, it treats it as any other
> error.
>
> So, either you return 0 when there's no PCS (so that we don't break platforms
> that don't have one), or you handle -ENODEV gracefully in patch 1.
>
> Maxime
>
Hi,
Currently, `rk_pcs_init` is called only for SGMII, which requires a PCS.
As it stands, only the RK3568 supports SGMII, so we won't break anything here.
Best,
Coia
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568
2026-09-03 8:38 ` Coia Prant
@ 2026-09-03 8:44 ` Maxime Chevallier
0 siblings, 0 replies; 24+ messages in thread
From: Maxime Chevallier @ 2026-09-03 8:44 UTC (permalink / raw)
To: Coia Prant
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Coquelin, Alexandre Torgue,
Lad Prabhakar, Romain Gantois, Heiner Kallweit, Neil Armstrong,
Russell King, Shawn Lin, David Heidelberg, netdev, linux-rockchip,
devicetree, linux-arm-kernel, linux-kernel, linux-phy,
linux-stm32, linux-renesas-soc
Hi,
>>> gmac_clk_enable() is now called before any register access, including
>>> the SGMII mode setup path. Previously SGMII mode would bypass the
>>> clock enable via a goto, which could cause synchronous external abort
>>> when accessing MAC registers with the clock domain disabled.
>>>
>>> Also clean up the error handling in rk_gmac_powerup() by using a
>>> unified clk_disable label, and add error handling for the default
>>> (unhandled interface) case.
>>>
>>> SGMII In-band vs Out-of-band
>>> ============================
>>> On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
>>> changed by the stmmac core's set_clk_tx_rate callback. In-band mode
>>> works because the PCS handles rate adaptation internally. Out-of-band
>>> mode does not work because the MAC would need to change the clock rate
>>> to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
>>> is fixed.
>>>
>>> Enable default_an_inband for SGMII and disable the generic stmmac
>>> set_clk_tx_rate callback. This forces phylink to use in-band mode,
>>> where the PCS is responsible for speed/duplex negotiation. Without
>>> this, the stmmac core would attempt to change the clock rate on speed
>>> changes, causing TX to work but RX to fail.
>>>
>>> Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
>>> Signed-off-by: Coia Prant <coiaprant@gmail.com>
>>
>> [...]
>>
>>> +static int rk_pcs_init(struct stmmac_priv *priv)
>>> +{
>>> + struct device_node *np = priv->device->of_node;
>>> + struct device_node *pcs_node;
>>> + struct dw_xpcs *xpcs;
>>> +
>>> + pcs_node = of_parse_phandle(np, "pcs-handle", 0);
>>> + if (!pcs_node)
>>> + return -ENODEV;
>>
>> Here aswell you make it mandatory to have a PCS, as the generic pcs logic
>> introduced in patch 1 doesn't handle -ENODEV, it treats it as any other
>> error.
>>
>> So, either you return 0 when there's no PCS (so that we don't break platforms
>> that don't have one), or you handle -ENODEV gracefully in patch 1.
>>
>> Maxime
>>
>
> Hi,
>
> Currently, `rk_pcs_init` is called only for SGMII, which requires a PCS.
>
> As it stands, only the RK3568 supports SGMII, so we won't break anything here.
Ah yes, it's protected by
if (plat_dat->phy_interface == PHY_INTERFACE_MODE_SGMII) {
...
}
My bad, this is fine then :)
the intel thing from patch 1 is still standing though from what I can see ? or have
I made the same error there ?
Maxime
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568
2026-09-01 15:01 ` [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568 Coia Prant
2026-09-02 15:04 ` sashiko-bot
2026-09-03 8:34 ` Maxime Chevallier
@ 2026-09-03 8:59 ` Maxime Chevallier
2 siblings, 0 replies; 24+ messages in thread
From: Maxime Chevallier @ 2026-09-03 8:59 UTC (permalink / raw)
To: Coia Prant, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Vinod Koul, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
Hi
On 9/1/26 17:01, Coia Prant wrote:
> The RK3568 SoC integrates a Synopsys DesignWare XPCS that can be
> connected to GMAC0 or GMAC1 in SGMII mode. Add the necessary glue
> logic to support this configuration.
>
> The current dwmac-rk driver does not support SGMII mode. SGMII
> requires a PCS to handle auto-negotiation and link state reporting,
> but the existing driver only supports RGMII and RMII.
>
> Add a set_to_sgmii() callback to configure the GMAC GRF register for
> SGMII mode (bit 7 set, interface selection bits 4:6 cleared). Also
> add a supports_sgmii flag to indicate SGMII capability.
>
> Provide pcs_init/pcs_exit callbacks to create/destroy the XPCS via
> xpcs_rk_create() from the Rockchip XPCS platform driver, and a
> select_pcs callback to return the XPCS to phylink.
>
> While at it, fix the clock enable ordering in rk_gmac_powerup():
> gmac_clk_enable() is now called before any register access, including
> the SGMII mode setup path. Previously SGMII mode would bypass the
> clock enable via a goto, which could cause synchronous external abort
> when accessing MAC registers with the clock domain disabled.
>
> Also clean up the error handling in rk_gmac_powerup() by using a
> unified clk_disable label, and add error handling for the default
> (unhandled interface) case.
>
> SGMII In-band vs Out-of-band
> ============================
> On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
> changed by the stmmac core's set_clk_tx_rate callback. In-band mode
> works because the PCS handles rate adaptation internally. Out-of-band
> mode does not work because the MAC would need to change the clock rate
> to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
> is fixed.
>
> Enable default_an_inband for SGMII and disable the generic stmmac
> set_clk_tx_rate callback. This forces phylink to use in-band mode,
> where the PCS is responsible for speed/duplex negotiation. Without
> this, the stmmac core would attempt to change the clock rate on speed
> changes, causing TX to work but RX to fail.
>
> Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next v3 09/10] arm64: dts: rockchip: rk3568-photonicat: enable SGMII LAN port
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
` (7 preceding siblings ...)
2026-09-01 15:01 ` [PATCH net-next v3 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568 Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
2026-09-01 15:01 ` [PATCH net-next v3 10/10] MAINTAINERS: add entry for Rockchip XPCS driver Coia Prant
9 siblings, 0 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
The Ariaboard Photonicat has a Motorcomm YT8521SC Gigabit Ethernet PHY
connected to GMAC0 via XPCS SGMII. Enable the necessary nodes to make
this port functional.
Enable combphy2 with rockchip,sgmii-mac-sel = <0> to route the SGMII
interface to GMAC0. Enable the xpcs node and its port 0 sub-node,
referencing combphy2 as the SerDes PHY.
Add the mdio0 node with the YT8521SC PHY at address 3, including its
reset GPIO and LED configuration. Also add LED configuration for the
existing RGMII PHY on mdio1 for consistency.
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
.../boot/dts/rockchip/rk3568-photonicat.dts | 74 ++++++++++++++++++-
1 file changed, 72 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
index 58c1052ba8ef3..25caa44198843 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dts
@@ -3,6 +3,7 @@
/dts-v1/;
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
#include <dt-bindings/pinctrl/rockchip.h>
#include <dt-bindings/soc/rockchip,vop2.h>
#include "rk3568.dtsi"
@@ -242,6 +243,7 @@ &combphy1 {
&combphy2 {
status = "okay";
+ rockchip,sgmii-mac-sel = <0>;
};
&cpu0 {
@@ -260,9 +262,18 @@ &cpu3 {
cpu-supply = <&vdd_cpu>;
};
-/* Motorcomm YT8521SC LAN port (require SGMII) */
+/* Motorcomm YT8521SC LAN port */
&gmac0 {
- status = "disabled";
+ assigned-clocks = <&cru SCLK_GMAC0_RX_TX>;
+ assigned-clock-parents = <&xpcs_gmac0_clk>;
+ pcs-handle = <&xpcs_mii0>;
+ managed = "in-band-status";
+ phy-handle = <&sgmii_phy>;
+ phy-mode = "sgmii";
+ phy-supply = <&vcc_3v3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&gmac0_miim>;
+ status = "okay";
};
/* Motorcomm YT8521SC WAN port */
@@ -341,6 +352,36 @@ &i2s0_8ch {
status = "okay";
};
+&mdio0 {
+ sgmii_phy: ethernet-phy@3 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0x3>;
+ max-speed = <1000>;
+ reset-assert-us = <20000>;
+ reset-deassert-us = <100000>;
+ reset-gpios = <&gpio3 RK_PC6 GPIO_ACTIVE_LOW>;
+
+ leds {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@1 {
+ reg = <1>;
+ color = <LED_COLOR_ID_AMBER>;
+ function = LED_FUNCTION_LAN;
+ default-state = "keep";
+ };
+
+ led@2 {
+ reg = <2>;
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_LAN;
+ default-state = "keep";
+ };
+ };
+ };
+};
+
&mdio1 {
rgmii_phy: ethernet-phy@3 {
compatible = "ethernet-phy-ieee802.3-c22";
@@ -350,6 +391,25 @@ rgmii_phy: ethernet-phy@3 {
reset-gpios = <&gpio4 RK_PC0 GPIO_ACTIVE_LOW>;
rx-internal-delay-ps = <1500>;
tx-internal-delay-ps = <1500>;
+
+ leds {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@1 {
+ reg = <1>;
+ color = <LED_COLOR_ID_AMBER>;
+ function = LED_FUNCTION_WAN;
+ default-state = "keep";
+ };
+
+ led@2 {
+ reg = <2>;
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_WAN;
+ default-state = "keep";
+ };
+ };
};
};
@@ -586,3 +646,13 @@ &xin32k {
pinctrl-names = "default";
pinctrl-0 = <&clk32k_out1>;
};
+
+&xpcs {
+ status = "okay";
+ phys = <&combphy2 PHY_TYPE_SGMII>;
+ phy-names = "serdes";
+};
+
+&xpcs_mii0 {
+ status = "okay";
+};
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH net-next v3 10/10] MAINTAINERS: add entry for Rockchip XPCS driver
2026-09-01 15:01 [PATCH net-next v3 00/10] net: pcs: add basic support for RK3568 XPCS Coia Prant
` (8 preceding siblings ...)
2026-09-01 15:01 ` [PATCH net-next v3 09/10] arm64: dts: rockchip: rk3568-photonicat: enable SGMII LAN port Coia Prant
@ 2026-09-01 15:01 ` Coia Prant
9 siblings, 0 replies; 24+ messages in thread
From: Coia Prant @ 2026-09-01 15:01 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner, Vinod Koul, Maxime Chevallier, Maxime Coquelin,
Alexandre Torgue, Lad Prabhakar, Romain Gantois, Heiner Kallweit,
Coia Prant
Cc: Neil Armstrong, Russell King, Shawn Lin, David Heidelberg, netdev,
linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
linux-phy, linux-stm32, linux-renesas-soc
Add a MAINTAINERS entry for the Rockchip RK3568 XPCS platform driver
and its device tree binding.
Include the relevant mailing lists (netdev and linux-rockchip) so that
future patches are properly distributed.
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3a19da74d00c9..c3b17752cc817 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23744,6 +23744,15 @@ S: Maintained
F: Documentation/devicetree/bindings/sound/rockchip,rk3576-sai.yaml
F: sound/soc/rockchip/rockchip_sai.*
+ROCKCHIP XPCS DRIVER
+M: Coia Prant <coiaprant@gmail.com>
+L: netdev@vger.kernel.org
+L: linux-rockchip@lists.infradead.org
+S: Maintained
+F: Documentation/devicetree/bindings/net/pcs/rockchip-dwxpcs.yaml
+F: drivers/net/pcs/pcs-xpcs-rk.c
+F: include/linux/pcs/pcs-xpcs-rk.h
+
ROCKER DRIVER
M: Jiri Pirko <jiri@resnulli.us>
L: netdev@vger.kernel.org
--
2.47.3
^ permalink raw reply related [flat|nested] 24+ messages in thread