[PATCH net-next v2 0/3] Follow-up to RGMII mode clarification: am65-cpsw fix + checkpatch

[PATCH net-next v2 0/3] Follow-up to RGMII mode clarification: am65-cpsw fix + checkpatch

[Matthias Schiffer]

Following previous discussion [1] and the documentation update by
Andrew [2]:

Fix up the mode to account for the fixed TX delay on the AM65 CPSW
Ethernet controllers, similar to the way the icssg-prueth does it. For
backwards compatibility, the "impossible" modes that claim to have a
delay on the PCB are still accepted, but trigger a warning message.

As Andrew suggested, I have also added a checkpatch check that requires
a comment for any RGMII mode that is not "rgmii-id".

No Device Trees are updated to avoid the warning for now, to give other
projects syncing the Linux Device Trees some time to fix their drivers
as well. I intend to submit an equivalent change for U-Boot's
am65-cpsw-nuss driver as soon as the changes are accepted for Linux.

Changelog v2:

- Previous patch 1/4 has been dropped, has it has been replaced by [2]
- Patches 1/3, 2/3: collected review and ack tags
- Patch 3/3:
  - Fixed multiple typos noted during review
  - Extended to check .dtso in addition to .dts and .dtsi
  - Changed CHK() to WARN(), so the warning triggers without --strict

[1] https://lore.kernel.org/lkml/d25b1447-c28b-4998-b238-92672434dc28@lunn.ch/
[2] https://lore.kernel.org/all/20250430-v6-15-rc3-net-rgmii-delays-v2-1-099ae651d5e5@lunn.ch/
    commit c360eb0c3ccb ("dt-bindings: net: ethernet-controller: Add informative text about RGMII delays")
Patch series v1: https://lore.kernel.org/all/cover.1744710099.git.matthias.schiffer@ew.tq-group.com/


Matthias Schiffer (3):
  dt-bindings: net: ti: k3-am654-cpsw-nuss: update phy-mode in example
  net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay
  checkpatch: check for comment explaining rgmii(|-rxid|-txid) PHY modes

 Documentation/dev-tools/checkpatch.rst        |  9 +++++++
 .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   |  2 +-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      | 27 +++++++++++++++++--
 scripts/checkpatch.pl                         | 12 +++++++++
 4 files changed, 47 insertions(+), 3 deletions(-)

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

[PATCH net-next v2 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: update phy-mode in example

[Matthias Schiffer]

k3-am65-cpsw-nuss controllers have a fixed internal TX delay, so RXID
mode is not actually possible and will result in a warning from the
driver going forward.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml          | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
index 7b3d948f187df..a959c1d7e643a 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -284,7 +284,7 @@ examples:
                     ti,syscon-efuse = <&mcu_conf 0x200>;
                     phys = <&phy_gmii_sel 1>;
 
-                    phy-mode = "rgmii-rxid";
+                    phy-mode = "rgmii-id";
                     phy-handle = <&phy0>;
                 };
             };
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

Re: [PATCH net-next v2 1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: update phy-mode in example

[Siddharth Vadapalli]

On Tue, Jun 24, 2025 at 12:53:32PM +0200, Matthias Schiffer wrote:
> k3-am65-cpsw-nuss controllers have a fixed internal TX delay, so RXID
> mode is not actually possible and will result in a warning from the
> driver going forward.
> 
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>

Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>

Regards,
Siddharth.

[PATCH net-next v2 2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay

[Matthias Schiffer]

All am65-cpsw controllers have a fixed TX delay, so the PHY interface
mode must be fixed up to account for this.

Modes that claim to a delay on the PCB can't actually work. Warn people
to update their Device Trees if one of the unsupported modes is specified.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 27 ++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index f20d1ff192efe..519757e618ad0 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2602,6 +2602,7 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
 		return -ENOENT;
 
 	for_each_child_of_node(node, port_np) {
+		phy_interface_t phy_if;
 		struct am65_cpsw_port *port;
 		u32 port_id;
 
@@ -2667,14 +2668,36 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
 
 		/* get phy/link info */
 		port->slave.port_np = of_node_get(port_np);
-		ret = of_get_phy_mode(port_np, &port->slave.phy_if);
+		ret = of_get_phy_mode(port_np, &phy_if);
 		if (ret) {
 			dev_err(dev, "%pOF read phy-mode err %d\n",
 				port_np, ret);
 			goto of_node_put;
 		}
 
-		ret = phy_set_mode_ext(port->slave.ifphy, PHY_MODE_ETHERNET, port->slave.phy_if);
+		/* CPSW controllers supported by this driver have a fixed
+		 * internal TX delay in RGMII mode. Fix up PHY mode to account
+		 * for this and warn about Device Trees that claim to have a TX
+		 * delay on the PCB.
+		 */
+		switch (phy_if) {
+		case PHY_INTERFACE_MODE_RGMII_ID:
+			phy_if = PHY_INTERFACE_MODE_RGMII_RXID;
+			break;
+		case PHY_INTERFACE_MODE_RGMII_TXID:
+			phy_if = PHY_INTERFACE_MODE_RGMII;
+			break;
+		case PHY_INTERFACE_MODE_RGMII:
+		case PHY_INTERFACE_MODE_RGMII_RXID:
+			dev_warn(dev,
+				 "RGMII mode without internal TX delay unsupported; please fix your Device Tree\n");
+			break;
+		default:
+			break;
+		}
+
+		port->slave.phy_if = phy_if;
+		ret = phy_set_mode_ext(port->slave.ifphy, PHY_MODE_ETHERNET, phy_if);
 		if (ret)
 			goto of_node_put;
 
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

Re: [PATCH net-next v2 2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay

[Siddharth Vadapalli]

On Tue, Jun 24, 2025 at 12:53:33PM +0200, Matthias Schiffer wrote:

Hello Matthias,

> All am65-cpsw controllers have a fixed TX delay, so the PHY interface
> mode must be fixed up to account for this.
> 
> Modes that claim to a delay on the PCB can't actually work. Warn people
> to update their Device Trees if one of the unsupported modes is specified.
> 
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 27 ++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index f20d1ff192efe..519757e618ad0 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -2602,6 +2602,7 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
>  		return -ENOENT;
>  
>  	for_each_child_of_node(node, port_np) {
> +		phy_interface_t phy_if;
>  		struct am65_cpsw_port *port;
>  		u32 port_id;
>  
> @@ -2667,14 +2668,36 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
>  
>  		/* get phy/link info */
>  		port->slave.port_np = of_node_get(port_np);
> -		ret = of_get_phy_mode(port_np, &port->slave.phy_if);
> +		ret = of_get_phy_mode(port_np, &phy_if);
>  		if (ret) {
>  			dev_err(dev, "%pOF read phy-mode err %d\n",
>  				port_np, ret);
>  			goto of_node_put;
>  		}
>  
> -		ret = phy_set_mode_ext(port->slave.ifphy, PHY_MODE_ETHERNET, port->slave.phy_if);
> +		/* CPSW controllers supported by this driver have a fixed
> +		 * internal TX delay in RGMII mode. Fix up PHY mode to account
> +		 * for this and warn about Device Trees that claim to have a TX
> +		 * delay on the PCB.
> +		 */
> +		switch (phy_if) {
> +		case PHY_INTERFACE_MODE_RGMII_ID:
> +			phy_if = PHY_INTERFACE_MODE_RGMII_RXID;
> +			break;
> +		case PHY_INTERFACE_MODE_RGMII_TXID:
> +			phy_if = PHY_INTERFACE_MODE_RGMII;
> +			break;
> +		case PHY_INTERFACE_MODE_RGMII:
> +		case PHY_INTERFACE_MODE_RGMII_RXID:
> +			dev_warn(dev,
> +				 "RGMII mode without internal TX delay unsupported; please fix your Device Tree\n");

Existing users designed boards and enabled Ethernet functionality using
"rgmii-rxid" in the device-tree and implementing the PCB traces in a
way that they interpret "rgmii-rxid". So their (mis)interpretation of
it is being challenged by the series. While it is true that we are updating
the bindings and driver to move towards the correct definition, I believe that
the above message would cause confusion. Would it be alright to update it to
something similar to:

"Interpretation of RGMII delays has been corrected; no functional impact; please fix your Device Tree"

Regards,
Siddharth.

Re: [PATCH net-next v2 2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay

[Andrew Lunn]

On Thu, Jun 26, 2025 at 03:10:50PM +0530, Siddharth Vadapalli wrote:
> On Tue, Jun 24, 2025 at 12:53:33PM +0200, Matthias Schiffer wrote:
> 
> Hello Matthias,
> 
> > All am65-cpsw controllers have a fixed TX delay, so the PHY interface
> > mode must be fixed up to account for this.
> > 
> > Modes that claim to a delay on the PCB can't actually work. Warn people
> > to update their Device Trees if one of the unsupported modes is specified.
> > 
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 27 ++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > index f20d1ff192efe..519757e618ad0 100644
> > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > @@ -2602,6 +2602,7 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
> >  		return -ENOENT;
> >  
> >  	for_each_child_of_node(node, port_np) {
> > +		phy_interface_t phy_if;
> >  		struct am65_cpsw_port *port;
> >  		u32 port_id;
> >  
> > @@ -2667,14 +2668,36 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
> >  
> >  		/* get phy/link info */
> >  		port->slave.port_np = of_node_get(port_np);
> > -		ret = of_get_phy_mode(port_np, &port->slave.phy_if);
> > +		ret = of_get_phy_mode(port_np, &phy_if);
> >  		if (ret) {
> >  			dev_err(dev, "%pOF read phy-mode err %d\n",
> >  				port_np, ret);
> >  			goto of_node_put;
> >  		}
> >  
> > -		ret = phy_set_mode_ext(port->slave.ifphy, PHY_MODE_ETHERNET, port->slave.phy_if);
> > +		/* CPSW controllers supported by this driver have a fixed
> > +		 * internal TX delay in RGMII mode. Fix up PHY mode to account
> > +		 * for this and warn about Device Trees that claim to have a TX
> > +		 * delay on the PCB.
> > +		 */
> > +		switch (phy_if) {
> > +		case PHY_INTERFACE_MODE_RGMII_ID:
> > +			phy_if = PHY_INTERFACE_MODE_RGMII_RXID;
> > +			break;
> > +		case PHY_INTERFACE_MODE_RGMII_TXID:
> > +			phy_if = PHY_INTERFACE_MODE_RGMII;
> > +			break;
> > +		case PHY_INTERFACE_MODE_RGMII:
> > +		case PHY_INTERFACE_MODE_RGMII_RXID:
> > +			dev_warn(dev,
> > +				 "RGMII mode without internal TX delay unsupported; please fix your Device Tree\n");
> 
> Existing users designed boards and enabled Ethernet functionality using
> "rgmii-rxid" in the device-tree and implementing the PCB traces in a
> way that they interpret "rgmii-rxid". So their (mis)interpretation of
> it is being challenged by the series. While it is true that we are updating
> the bindings and driver to move towards the correct definition, I believe that
> the above message would cause confusion. Would it be alright to update it to
> something similar to:
> 
> "Interpretation of RGMII delays has been corrected; no functional impact; please fix your Device Tree"

It is dev_warn() not dev_err(), so it should be read as a warning. And
the device will continue to probe and work. So I think the message is
O.K. What we don't want is DT developers thinking they can just ignore
it. So i would keep it reasonably strongly worded.

	Andrew

Re: [PATCH net-next v2 2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay

[Siddharth Vadapalli]

On Thu, Jun 26, 2025 at 01:58:07PM +0200, Andrew Lunn wrote:
> On Thu, Jun 26, 2025 at 03:10:50PM +0530, Siddharth Vadapalli wrote:
> > On Tue, Jun 24, 2025 at 12:53:33PM +0200, Matthias Schiffer wrote:
> > 
> > Hello Matthias,
> > 
> > > All am65-cpsw controllers have a fixed TX delay, so the PHY interface
> > > mode must be fixed up to account for this.
> > > 
> > > Modes that claim to a delay on the PCB can't actually work. Warn people
> > > to update their Device Trees if one of the unsupported modes is specified.
> > > 
> > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > > ---
> > >  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 27 ++++++++++++++++++++++--
> > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > index f20d1ff192efe..519757e618ad0 100644
> > > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> > > @@ -2602,6 +2602,7 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
> > >  		return -ENOENT;
> > >  
> > >  	for_each_child_of_node(node, port_np) {
> > > +		phy_interface_t phy_if;
> > >  		struct am65_cpsw_port *port;
> > >  		u32 port_id;
> > >  
> > > @@ -2667,14 +2668,36 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
> > >  
> > >  		/* get phy/link info */
> > >  		port->slave.port_np = of_node_get(port_np);
> > > -		ret = of_get_phy_mode(port_np, &port->slave.phy_if);
> > > +		ret = of_get_phy_mode(port_np, &phy_if);
> > >  		if (ret) {
> > >  			dev_err(dev, "%pOF read phy-mode err %d\n",
> > >  				port_np, ret);
> > >  			goto of_node_put;
> > >  		}
> > >  
> > > -		ret = phy_set_mode_ext(port->slave.ifphy, PHY_MODE_ETHERNET, port->slave.phy_if);
> > > +		/* CPSW controllers supported by this driver have a fixed
> > > +		 * internal TX delay in RGMII mode. Fix up PHY mode to account
> > > +		 * for this and warn about Device Trees that claim to have a TX
> > > +		 * delay on the PCB.
> > > +		 */
> > > +		switch (phy_if) {
> > > +		case PHY_INTERFACE_MODE_RGMII_ID:
> > > +			phy_if = PHY_INTERFACE_MODE_RGMII_RXID;
> > > +			break;
> > > +		case PHY_INTERFACE_MODE_RGMII_TXID:
> > > +			phy_if = PHY_INTERFACE_MODE_RGMII;
> > > +			break;
> > > +		case PHY_INTERFACE_MODE_RGMII:
> > > +		case PHY_INTERFACE_MODE_RGMII_RXID:
> > > +			dev_warn(dev,
> > > +				 "RGMII mode without internal TX delay unsupported; please fix your Device Tree\n");
> > 
> > Existing users designed boards and enabled Ethernet functionality using
> > "rgmii-rxid" in the device-tree and implementing the PCB traces in a
> > way that they interpret "rgmii-rxid". So their (mis)interpretation of
> > it is being challenged by the series. While it is true that we are updating
> > the bindings and driver to move towards the correct definition, I believe that
> > the above message would cause confusion. Would it be alright to update it to
> > something similar to:
> > 
> > "Interpretation of RGMII delays has been corrected; no functional impact; please fix your Device Tree"
> 
> It is dev_warn() not dev_err(), so it should be read as a warning. And
> the device will continue to probe and work. So I think the message is
> O.K. What we don't want is DT developers thinking they can just ignore
> it. So i would keep it reasonably strongly worded.

Thank you for the clarification. I have no further concerns and the
patch looks good to me.

Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>

Regards,
Siddharth.

Re: [PATCH net-next v2 2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

Hi,

On Tue Jun 24, 2025 at 12:53 PM CEST, Matthias Schiffer wrote:
> All am65-cpsw controllers have a fixed TX delay, so the PHY interface
> mode must be fixed up to account for this.
>
> Modes that claim to a delay on the PCB can't actually work. Warn people
> to update their Device Trees if one of the unsupported modes is specified.
>
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

For whatever reason, this patch is breaking network on our board
(just transmission). We have rgmii-id in our devicetree which is now
modified to be just rgmii-rxid. The board has a TI AM67A (J722S) with a
Broadcom BCM54210E PHY. I'm not sure, if AM67A MAC doesn't add any
delay or if it's too small. I'll need to ask around if there are any
measurements but my colleague doing the measurements is on holiday
at the moment.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH net-next v2 2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay

[Andrew Lunn]

On Mon, Jul 14, 2025 at 12:01:22PM +0200, Michael Walle wrote:
> Hi,
> 
> On Tue Jun 24, 2025 at 12:53 PM CEST, Matthias Schiffer wrote:
> > All am65-cpsw controllers have a fixed TX delay, so the PHY interface
> > mode must be fixed up to account for this.
> >
> > Modes that claim to a delay on the PCB can't actually work. Warn people
> > to update their Device Trees if one of the unsupported modes is specified.
> >
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> For whatever reason, this patch is breaking network on our board
> (just transmission). We have rgmii-id in our devicetree which is now
> modified to be just rgmii-rxid. The board has a TI AM67A (J722S) with a
> Broadcom BCM54210E PHY. I'm not sure, if AM67A MAC doesn't add any
> delay or if it's too small. I'll need to ask around if there are any
> measurements but my colleague doing the measurements is on holiday
> at the moment.

I agree, we need to see if this is a AM65 vs AM67 issue. rgmii-id
would be correct if the MAC is not adding delays.

Do you have access to the datasheets for both? Can you do a side by
side comparison for the section which describes the fixed TX delay?

	Andrew

Re: [PATCH net-next v2 2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay

[Michael Walle]

Hi,

On Mon Jul 14, 2025 at 3:09 PM CEST, Andrew Lunn wrote:
> On Mon, Jul 14, 2025 at 12:01:22PM +0200, Michael Walle wrote:
> > On Tue Jun 24, 2025 at 12:53 PM CEST, Matthias Schiffer wrote:
> > > All am65-cpsw controllers have a fixed TX delay, so the PHY interface
> > > mode must be fixed up to account for this.
> > >
> > > Modes that claim to a delay on the PCB can't actually work. Warn people
> > > to update their Device Trees if one of the unsupported modes is specified.
> > >
> > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > For whatever reason, this patch is breaking network on our board
> > (just transmission). We have rgmii-id in our devicetree which is now
> > modified to be just rgmii-rxid. The board has a TI AM67A (J722S) with a
> > Broadcom BCM54210E PHY. I'm not sure, if AM67A MAC doesn't add any
> > delay or if it's too small. I'll need to ask around if there are any
> > measurements but my colleague doing the measurements is on holiday
> > at the moment.
>
> I agree, we need to see if this is a AM65 vs AM67 issue. rgmii-id
> would be correct if the MAC is not adding delays.
>
> Do you have access to the datasheets for both? Can you do a side by
> side comparison for the section which describes the fixed TX delay?

The datasheets and TRMs of the SoC are public of the SoC. According
to the AM67A TRM the delay should be 1.2ns if I'm reading it
correctly. The BCM PHY requires a setup time of -0.9ns (min). So, is
should work (?), but it doesn't. I'm also not aware of any routing
skew between the signals. But as I said, I'll have to check with my
colleague next week.

-michael

[PATCH net-next v2 3/3] checkpatch: check for comment explaining rgmii(|-rxid|-txid) PHY modes

[Matthias Schiffer]

Historically, the RGMII PHY modes specified in Device Trees have been
used inconsistently, often referring to the usage of delays on the PHY
side rather than describing the board; many drivers still implement this
incorrectly.

Require a comment in Devices Trees using these modes (usually mentioning
that the delay is realized on the PCB), so we can avoid adding more
incorrect uses (or will at least notice which drivers still need to be
fixed).

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 Documentation/dev-tools/checkpatch.rst |  9 +++++++++
 scripts/checkpatch.pl                  | 12 ++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index 76bd0ddb00416..d5c47e560324f 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -495,6 +495,15 @@ Comments
 
     See: https://lore.kernel.org/lkml/20131006222342.GT19510@leaf/
 
+  **UNCOMMENTED_RGMII_MODE**
+    Historically, the RGMII PHY modes specified in Device Trees have been
+    used inconsistently, often referring to the usage of delays on the PHY
+    side rather than describing the board.
+
+    PHY modes "rgmii", "rgmii-rxid" and "rgmii-txid" modes require the clock
+    signal to be delayed on the PCB; this unusual configuration should be
+    described in a comment. If they are not (meaning that the delay is realized
+    internally in the MAC or PHY), "rgmii-id" is the correct PHY mode.
 
 Commit message
 --------------
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 664f7b7a622c2..f597734d83cc0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3741,6 +3741,18 @@ sub process {
 			}
 		}
 
+# Check for RGMII phy-mode with delay on PCB
+		if ($realfile =~ /\.(dts|dtsi|dtso)$/ &&
+		    $line =~ /^\+\s*(phy-mode|phy-connection-type)\s*=\s*"/ &&
+		    !ctx_has_comment($first_line, $linenr)) {
+			my $prop = $1;
+			my $mode = get_quoted_string($line, $rawline);
+			if ($mode =~ /^"rgmii(?:|-rxid|-txid)"$/) {
+				WARN("UNCOMMENTED_RGMII_MODE",
+				     "$prop $mode without comment -- delays on the PCB should be described, otherwise use \"rgmii-id\"\n" . $herecurr);
+			}
+		}
+
 # check for using SPDX license tag at beginning of files
 		if ($realline == $checklicenseline) {
 			if ($rawline =~ /^[ \+]\s*\#\!\s*\//) {
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

Re: [PATCH net-next v2 3/3] checkpatch: check for comment explaining rgmii(|-rxid|-txid) PHY modes

[Andrew Lunn]

On Tue, Jun 24, 2025 at 12:53:34PM +0200, Matthias Schiffer wrote:
> Historically, the RGMII PHY modes specified in Device Trees have been
> used inconsistently, often referring to the usage of delays on the PHY
> side rather than describing the board; many drivers still implement this
> incorrectly.
> 
> Require a comment in Devices Trees using these modes (usually mentioning
> that the delay is realized on the PCB), so we can avoid adding more
> incorrect uses (or will at least notice which drivers still need to be
> fixed).
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>

One question, how should this be merged? The two DT patches might want
to go via the TI DT Maintainer. And this patch via the checkpatch
Maintainer? Or do you plan to merge it some other way?

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 3/3] checkpatch: check for comment explaining rgmii(|-rxid|-txid) PHY modes

[Matthias Schiffer]

On Thu, 2025-06-26 at 10:02 +0200, Andrew Lunn wrote:
> 
> On Tue, Jun 24, 2025 at 12:53:34PM +0200, Matthias Schiffer wrote:
> > Historically, the RGMII PHY modes specified in Device Trees have been
> > used inconsistently, often referring to the usage of delays on the PHY
> > side rather than describing the board; many drivers still implement this
> > incorrectly.
> > 
> > Require a comment in Devices Trees using these modes (usually mentioning
> > that the delay is realized on the PCB), so we can avoid adding more
> > incorrect uses (or will at least notice which drivers still need to be
> > fixed).
> > 
> > Suggested-by: Andrew Lunn <andrew@lunn.ch>
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> 
> One question, how should this be merged? The two DT patches might want
> to go via the TI DT Maintainer. And this patch via the checkpatch
> Maintainer? Or do you plan to merge it some other way?

The first two patches should go via net-next I think (the first is DT bindings
only, the second one modifies the AM65-CPSW driver), although I would prefer to
get a review/ack from a TI maintainer, too.

I don't know what tree checkpatch usually goes through, MAINTAINERS doesn't list
a specific repo. The whole series could be merged via net-next if that's fine
with the checkpatch maintainers.

Best,
Matthias



> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew


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

Re: [PATCH net-next v2 3/3] checkpatch: check for comment explaining rgmii(|-rxid|-txid) PHY modes

[Joe Perches]

On Thu, 2025-06-26 at 10:11 +0200, Matthias Schiffer wrote:
> I don't know what tree checkpatch usually goes through, MAINTAINERS doesn't list
> a specific repo. The whole series could be merged via net-next if that's fine
> with the checkpatch maintainers.

Merging via net-next is fine with me.

Andrew Morton is the typical upstream path for random checkpatch
changes.

Re: [PATCH net-next v2 0/3] Follow-up to RGMII mode clarification: am65-cpsw fix + checkpatch

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 24 Jun 2025 12:53:31 +0200 you wrote:
> Following previous discussion [1] and the documentation update by
> Andrew [2]:
> 
> Fix up the mode to account for the fixed TX delay on the AM65 CPSW
> Ethernet controllers, similar to the way the icssg-prueth does it. For
> backwards compatibility, the "impossible" modes that claim to have a
> delay on the PCB are still accepted, but trigger a warning message.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] dt-bindings: net: ti: k3-am654-cpsw-nuss: update phy-mode in example
    https://git.kernel.org/netdev/net-next/c/9b357ea52523
  - [net-next,v2,2/3] net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay
    https://git.kernel.org/netdev/net-next/c/ca13b249f291
  - [net-next,v2,3/3] checkpatch: check for comment explaining rgmii(|-rxid|-txid) PHY modes
    https://git.kernel.org/netdev/net-next/c/e02adac7c84b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html