[PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Michael Walle]

This reverts commit ca13b249f291f4920466638d1adbfb3f9c8db6e9.

This patch breaks the transmit path on an AM67A/J722S. This SoC has an
(undocumented) configurable delay (CTRL_MMR0_CFG0_ENET1_CTRL, bit 4).

The u-boot driver (net/ti/am65-cpsw-nuss.c) will configure the delay in
am65_cpsw_gmii_sel_k3(). If the u-boot device tree uses rgmii-id this
patch will break the transmit path because it will disable the PHY delay
on the transmit path, but the bootloader has already disabled the MAC
delay, hence there will be no delay at all.

Although the datasheet reads the delay is fixed, that is at least
wrong for the J722S/AM67A and apparently for at least the AM65x
(which was the original target of the u-boot driver).

Fixes: ca13b249f291 ("net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
This is targeted as net-next although the merge window is open, because
the original patch is just in net-next.
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 27 ++----------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index ecd6ecac87bb..231ca141331f 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2600,7 +2600,6 @@ 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;
 
@@ -2666,36 +2665,14 @@ 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, &phy_if);
+		ret = of_get_phy_mode(port_np, &port->slave.phy_if);
 		if (ret) {
 			dev_err(dev, "%pOF read phy-mode err %d\n",
 				port_np, ret);
 			goto of_node_put;
 		}
 
-		/* 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);
+		ret = phy_set_mode_ext(port->slave.ifphy, PHY_MODE_ETHERNET, port->slave.phy_if);
 		if (ret)
 			goto of_node_put;
 
-- 
2.39.5

Re: [PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Andrew Lunn]

On Mon, Jul 28, 2025 at 08:49:38AM +0200, Michael Walle wrote:
> This reverts commit ca13b249f291f4920466638d1adbfb3f9c8db6e9.
> 
> This patch breaks the transmit path on an AM67A/J722S. This SoC has an
> (undocumented) configurable delay (CTRL_MMR0_CFG0_ENET1_CTRL, bit 4).

Is this undocumented register only on the AM67A/J722S?

The patch being reverted says:

   All am65-cpsw controllers have a fixed TX delay

So we have some degree of contradiction here.

> The u-boot driver (net/ti/am65-cpsw-nuss.c) will configure the delay in
> am65_cpsw_gmii_sel_k3(). If the u-boot device tree uses rgmii-id this
> patch will break the transmit path because it will disable the PHY delay
> on the transmit path, but the bootloader has already disabled the MAC
> delay, hence there will be no delay at all.

We have maybe 8 weeks to fix this, before it makes it into a released
kernel. So rather than revert, i would prefer to extend the patch to
make it work with all variants of the SoC.

Is CTRL_MMR0_CFG0_ENET1_CTRL in the Ethernet address space? Would it
be possible for the MAC driver to read it, and know if the delay has
been disabled? The switch statement can then be made conditional?

If this register actually exists on all SoC variants, can we just
globally disable it, and remove the switch statement?

	 Andrew
	

Re: [PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Michael Walle]

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

On Mon Jul 28, 2025 at 4:41 PM CEST, Andrew Lunn wrote:
> On Mon, Jul 28, 2025 at 08:49:38AM +0200, Michael Walle wrote:
> > This reverts commit ca13b249f291f4920466638d1adbfb3f9c8db6e9.
> > 
> > This patch breaks the transmit path on an AM67A/J722S. This SoC has an
> > (undocumented) configurable delay (CTRL_MMR0_CFG0_ENET1_CTRL, bit 4).
>
> Is this undocumented register only on the AM67A/J722S?

I've looked at the AM65x TRM (search for MMR0 or RGMII_ID_MODE),
which reads that bit 4 is r/w but only '0' is documented as
'internal transmit delay', value '1' is called "reserved".

I couldn't find anything in the AM64x TRM. Didn't look further.

There has to be a reason why TI states that TX delay is always on
and don't document that bit. OTOH, they wrote code to serve that bit
in u-boot. Sigh. Someone from TI have to chime in here to shed some
light to this.

> The patch being reverted says:
>
>    All am65-cpsw controllers have a fixed TX delay
>
> So we have some degree of contradiction here.

I've digged through the old thread and Matthias just references the
datasheet saying it is fixed. Matthias, could you actually try to
set/read this bit? I'm not sure it is really read-only.

> > The u-boot driver (net/ti/am65-cpsw-nuss.c) will configure the delay in
> > am65_cpsw_gmii_sel_k3(). If the u-boot device tree uses rgmii-id this
> > patch will break the transmit path because it will disable the PHY delay
> > on the transmit path, but the bootloader has already disabled the MAC
> > delay, hence there will be no delay at all.
>
> We have maybe 8 weeks to fix this, before it makes it into a released
> kernel. So rather than revert, i would prefer to extend the patch to
> make it work with all variants of the SoC.
>
> Is CTRL_MMR0_CFG0_ENET1_CTRL in the Ethernet address space?

No, that register is part of the global configuration space (search
for phy_gmii_sel in the k3-am62p-j722s-common-main.dtsi), but is
modeled after a PHY (not a network PHY). And actually, I've just
found out that the PHY driver for that will serve the rgmii_id bit
if .features has PHY_GMII_SEL_RGMII_ID_MODE set. So there is already
a whitelist (although it's wrong at the moment, because the J722S
SoC is not listed as having it). As a side note, the j722s also
doesn't have it's own SoC specific compatible it is reusing the
am654-phy-gmii-sel compatible. That might or might not bite us now..

I digress..

> Would it be possible for the MAC driver to read it, and know if the delay has
> been disabled? The switch statement can then be made conditional?
>
> If this register actually exists on all SoC variants, can we just
> globally disable it, and remove the switch statement?

Given that all the handling is in the PHY subsystem I don't know.
You'd have to ask the PHY if it supports that, before patching the
phy-interface-mode - before attaching the network PHY I guess?

If we want to just disable (and I assume with disable you mean
disable the MAC delay) it: the PHY is optional, not sure every SoC
will have one. And also, the reset default is exactly the opposite
and TI says it's fixed to the opposite and there has to be a reason
for that.

Sounds like a real mess to me.

-michael

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

Re: [PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Matthias Schiffer]

On Tue, 2025-07-29 at 09:33 +0200, Michael Walle wrote:
> On Mon Jul 28, 2025 at 4:41 PM CEST, Andrew Lunn wrote:
> > On Mon, Jul 28, 2025 at 08:49:38AM +0200, Michael Walle wrote:
> > > This reverts commit ca13b249f291f4920466638d1adbfb3f9c8db6e9.
> > > 
> > > This patch breaks the transmit path on an AM67A/J722S. This SoC has an
> > > (undocumented) configurable delay (CTRL_MMR0_CFG0_ENET1_CTRL, bit 4).
> > 
> > Is this undocumented register only on the AM67A/J722S?
> 
> I've looked at the AM65x TRM (search for MMR0 or RGMII_ID_MODE),
> which reads that bit 4 is r/w but only '0' is documented as
> 'internal transmit delay', value '1' is called "reserved".
> 
> I couldn't find anything in the AM64x TRM. Didn't look further.
> 
> There has to be a reason why TI states that TX delay is always on
> and don't document that bit. OTOH, they wrote code to serve that bit
> in u-boot. Sigh. Someone from TI have to chime in here to shed some
> light to this.

Adding TI K3 maintainers, as am65-cpsw doesn't have a MAINTAINERS entry of its
own.

> 
> > The patch being reverted says:
> > 
> >    All am65-cpsw controllers have a fixed TX delay
> > 
> > So we have some degree of contradiction here.
> 
> I've digged through the old thread and Matthias just references the
> datasheet saying it is fixed. Matthias, could you actually try to
> set/read this bit? I'm not sure it is really read-only.

I just referred to the datasheets of various K3 SoCs, I did not try modifying
the reserved bits.

> 
> > > The u-boot driver (net/ti/am65-cpsw-nuss.c) will configure the delay in
> > > am65_cpsw_gmii_sel_k3(). If the u-boot device tree uses rgmii-id this
> > > patch will break the transmit path because it will disable the PHY delay
> > > on the transmit path, but the bootloader has already disabled the MAC
> > > delay, hence there will be no delay at all.

I have a patch that removes this piece of U-Boot code and had intended to submit
that soon to align the U-Boot driver with Linux again. I'll hold off until we
know how the solution in Linux is going to look.

> > 
> > We have maybe 8 weeks to fix this, before it makes it into a released
> > kernel. So rather than revert, i would prefer to extend the patch to
> > make it work with all variants of the SoC.
> > 
> > Is CTRL_MMR0_CFG0_ENET1_CTRL in the Ethernet address space?
> 
> No, that register is part of the global configuration space (search
> for phy_gmii_sel in the k3-am62p-j722s-common-main.dtsi), but is
> modeled after a PHY (not a network PHY). And actually, I've just
> found out that the PHY driver for that will serve the rgmii_id bit
> if .features has PHY_GMII_SEL_RGMII_ID_MODE set. So there is already
> a whitelist (although it's wrong at the moment, because the J722S
> SoC is not listed as having it). As a side note, the j722s also
> doesn't have it's own SoC specific compatible it is reusing the
> am654-phy-gmii-sel compatible. That might or might not bite us now..
> 
> I digress..
> 
> > Would it be possible for the MAC driver to read it, and know if the delay has
> > been disabled? The switch statement can then be made conditional?
> > 
> > If this register actually exists on all SoC variants, can we just
> > globally disable it, and remove the switch statement?

If we just remove the switch statement, thus actually supporting all the
different delay modes, we're back at the point where there is no way for the
driver to determine whether rgmii-rxid is supposed to be interpreted correctly
or not (currently all Device Trees using this driver require the old/incorrect
interpretation for Ethernet to work).

> 
> Given that all the handling is in the PHY subsystem I don't know.
> You'd have to ask the PHY if it supports that, before patching the
> phy-interface-mode - before attaching the network PHY I guess?

The previous generation of the CPSW IP handles this in
drivers/net/ethernet/ti/cpsw-phy-sel.c, which is just a custom platform device
referenced by the MAC node. The code currently (partially) implements the
old/incorrect interpretation for phy-mode, enabling the delay on the MAC side
for PHY_INTERFACE_MODE_RGMII.

> 
> If we want to just disable (and I assume with disable you mean
> disable the MAC delay) it: the PHY is optional, not sure every SoC
> will have one. And also, the reset default is exactly the opposite
> and TI says it's fixed to the opposite and there has to be a reason
> for that.

My preference would be to unconditionally enable the MAC-side delay on Linux to
align with the reset default and what the datasheet claims is the only supported
mode, but let's hear what the TI folks think about this.

Best,
Matthias


-- 
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] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Michael Walle]

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

Hi,

> > > The patch being reverted says:
> > > 
> > >    All am65-cpsw controllers have a fixed TX delay
> > > 
> > > So we have some degree of contradiction here.
> > 
> > I've digged through the old thread and Matthias just references the
> > datasheet saying it is fixed. Matthias, could you actually try to
> > set/read this bit? I'm not sure it is really read-only.
>
> I just referred to the datasheets of various K3 SoCs, I did not try modifying
> the reserved bits.

So can you try to modify them?

> > > > The u-boot driver (net/ti/am65-cpsw-nuss.c) will configure the delay in
> > > > am65_cpsw_gmii_sel_k3(). If the u-boot device tree uses rgmii-id this
> > > > patch will break the transmit path because it will disable the PHY delay
> > > > on the transmit path, but the bootloader has already disabled the MAC
> > > > delay, hence there will be no delay at all.
>
> I have a patch that removes this piece of U-Boot code and had intended to submit
> that soon to align the U-Boot driver with Linux again. I'll hold off until we
> know how the solution in Linux is going to look.

That doesn't fix older bootloaders though. So in linux we still have
to work around that.

Although I don't get it why you want to remove that feature.

> > > We have maybe 8 weeks to fix this, before it makes it into a released
> > > kernel. So rather than revert, i would prefer to extend the patch to
> > > make it work with all variants of the SoC.
> > > 
> > > Is CTRL_MMR0_CFG0_ENET1_CTRL in the Ethernet address space?
> > 
> > No, that register is part of the global configuration space (search
> > for phy_gmii_sel in the k3-am62p-j722s-common-main.dtsi), but is
> > modeled after a PHY (not a network PHY). And actually, I've just
> > found out that the PHY driver for that will serve the rgmii_id bit
> > if .features has PHY_GMII_SEL_RGMII_ID_MODE set. So there is already
> > a whitelist (although it's wrong at the moment, because the J722S
> > SoC is not listed as having it). As a side note, the j722s also
> > doesn't have it's own SoC specific compatible it is reusing the
> > am654-phy-gmii-sel compatible. That might or might not bite us now..
> > 
> > I digress..
> > 
> > > Would it be possible for the MAC driver to read it, and know if the delay has
> > > been disabled? The switch statement can then be made conditional?
> > > 
> > > If this register actually exists on all SoC variants, can we just
> > > globally disable it, and remove the switch statement?
>
> If we just remove the switch statement, thus actually supporting all the
> different delay modes, we're back at the point where there is no way for the
> driver to determine whether rgmii-rxid is supposed to be interpreted correctly
> or not (currently all Device Trees using this driver require the old/incorrect
> interpretation for Ethernet to work).

I can't follow you here. Are you assuming that the TX delay is
fixed? For me, that's still the culprit. Is that a fair assumption?
And only TI can tell us.

> > Given that all the handling is in the PHY subsystem I don't know.
> > You'd have to ask the PHY if it supports that, before patching the
> > phy-interface-mode - before attaching the network PHY I guess?
>
> The previous generation of the CPSW IP handles this in
> drivers/net/ethernet/ti/cpsw-phy-sel.c, which is just a custom platform device
> referenced by the MAC node. The code currently (partially) implements the
> old/incorrect interpretation for phy-mode, enabling the delay on the MAC side
> for PHY_INTERFACE_MODE_RGMII.
>
> > 
> > If we want to just disable (and I assume with disable you mean
> > disable the MAC delay) it: the PHY is optional, not sure every SoC
> > will have one. And also, the reset default is exactly the opposite
> > and TI says it's fixed to the opposite and there has to be a reason
> > for that.
>
> My preference would be to unconditionally enable the MAC-side delay on Linux to
> align with the reset default and what the datasheet claims is the only supported
> mode, but let's hear what the TI folks think about this.

Which goes against Andrew's "lets to all the delays in the PHY". We
have rgmii-id in our AM67A based board and we've actually measured
the signals with and without the MAC delay.

Last week, I've also opened an e2e forum case, maybe we get some
more insights. Funny enough, TI seem to have a different register
description where this bit is described.
https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1544031/am67a-internal-rgmii-delay/

-michael

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

Re: [PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Matthias Schiffer]

On Tue, 2025-07-29 at 10:47 +0200, Michael Walle wrote:
> Hi,
> 
> > > > The patch being reverted says:
> > > > 
> > > >    All am65-cpsw controllers have a fixed TX delay
> > > > 
> > > > So we have some degree of contradiction here.
> > > 
> > > I've digged through the old thread and Matthias just references the
> > > datasheet saying it is fixed. Matthias, could you actually try to
> > > set/read this bit? I'm not sure it is really read-only.
> > 
> > I just referred to the datasheets of various K3 SoCs, I did not try modifying
> > the reserved bits.
> 
> So can you try to modify them?

I can try some time this week.

> 
> > > > > The u-boot driver (net/ti/am65-cpsw-nuss.c) will configure the delay in
> > > > > am65_cpsw_gmii_sel_k3(). If the u-boot device tree uses rgmii-id this
> > > > > patch will break the transmit path because it will disable the PHY delay
> > > > > on the transmit path, but the bootloader has already disabled the MAC
> > > > > delay, hence there will be no delay at all.
> > 
> > I have a patch that removes this piece of U-Boot code and had intended to submit
> > that soon to align the U-Boot driver with Linux again. I'll hold off until we
> > know how the solution in Linux is going to look.
> 
> That doesn't fix older bootloaders though. So in linux we still have
> to work around that.

I don't consider this something to "work around" but a necessary fix - there is
simply no way for Linux to do the right thing without knowing whether the MAC
adds delays (either by reading or writing the flag, if that actually does
anything).

> 
> Although I don't get it why you want to remove that feature.

My patch was based on the assumption that these bits don't actually work, as the
datasheet states that there is always a delay... but if this assumption is
wrong, I need to reconsider my approach.


> 
> > > > We have maybe 8 weeks to fix this, before it makes it into a released
> > > > kernel. So rather than revert, i would prefer to extend the patch to
> > > > make it work with all variants of the SoC.
> > > > 
> > > > Is CTRL_MMR0_CFG0_ENET1_CTRL in the Ethernet address space?
> > > 
> > > No, that register is part of the global configuration space (search
> > > for phy_gmii_sel in the k3-am62p-j722s-common-main.dtsi), but is
> > > modeled after a PHY (not a network PHY). And actually, I've just
> > > found out that the PHY driver for that will serve the rgmii_id bit
> > > if .features has PHY_GMII_SEL_RGMII_ID_MODE set. So there is already
> > > a whitelist (although it's wrong at the moment, because the J722S
> > > SoC is not listed as having it). As a side note, the j722s also
> > > doesn't have it's own SoC specific compatible it is reusing the
> > > am654-phy-gmii-sel compatible. That might or might not bite us now..
> > > 
> > > I digress..
> > > 
> > > > Would it be possible for the MAC driver to read it, and know if the delay has
> > > > been disabled? The switch statement can then be made conditional?
> > > > 
> > > > If this register actually exists on all SoC variants, can we just
> > > > globally disable it, and remove the switch statement?
> > 
> > If we just remove the switch statement, thus actually supporting all the
> > different delay modes, we're back at the point where there is no way for the
> > driver to determine whether rgmii-rxid is supposed to be interpreted correctly
> > or not (currently all Device Trees using this driver require the old/incorrect
> > interpretation for Ethernet to work).
> 
> I can't follow you here. Are you assuming that the TX delay is
> fixed? For me, that's still the culprit. Is that a fair assumption?
> And only TI can tell us.

Right. In particular if it's the same for all K3 SoCs - I can only test AM62,
AM64 and AM67A/J772S here.


> 
> > > Given that all the handling is in the PHY subsystem I don't know.
> > > You'd have to ask the PHY if it supports that, before patching the
> > > phy-interface-mode - before attaching the network PHY I guess?
> > 
> > The previous generation of the CPSW IP handles this in
> > drivers/net/ethernet/ti/cpsw-phy-sel.c, which is just a custom platform device
> > referenced by the MAC node. The code currently (partially) implements the
> > old/incorrect interpretation for phy-mode, enabling the delay on the MAC side
> > for PHY_INTERFACE_MODE_RGMII.
> > 
> > > 
> > > If we want to just disable (and I assume with disable you mean
> > > disable the MAC delay) it: the PHY is optional, not sure every SoC
> > > will have one. And also, the reset default is exactly the opposite
> > > and TI says it's fixed to the opposite and there has to be a reason
> > > for that.
> > 
> > My preference would be to unconditionally enable the MAC-side delay on Linux to
> > align with the reset default and what the datasheet claims is the only supported
> > mode, but let's hear what the TI folks think about this.
> 
> Which goes against Andrew's "lets to all the delays in the PHY". We
> have rgmii-id in our AM67A based board and we've actually measured
> the signals with and without the MAC delay.
> 
> Last week, I've also opened an e2e forum case, maybe we get some
> more insights. Funny enough, TI seem to have a different register
> description where this bit is described.
> https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1544031/am67a-internal-rgmii-delay/
> 
> -michael

-- 
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] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Matthias Schiffer]

On Tue, 2025-07-29 at 11:09 +0200, Matthias Schiffer wrote:
> On Tue, 2025-07-29 at 10:47 +0200, Michael Walle wrote:
> > Hi,
> > 
> > > > > The patch being reverted says:
> > > > > 
> > > > >    All am65-cpsw controllers have a fixed TX delay
> > > > > 
> > > > > So we have some degree of contradiction here.
> > > > 
> > > > I've digged through the old thread and Matthias just references the
> > > > datasheet saying it is fixed. Matthias, could you actually try to
> > > > set/read this bit? I'm not sure it is really read-only.
> > > 
> > > I just referred to the datasheets of various K3 SoCs, I did not try modifying
> > > the reserved bits.
> > 
> > So can you try to modify them?
> 
> I can try some time this week.

I can confirm that the undocumented/reserved bit switches the MAC-side TX delay
on and off on the J722S/AM67A. I have not checked if there is anything wrong
with the undelayed mode that might explain why TI doesn't want to support it,
but traffic appears to flow through the interface without issue if I disable the
MAC-side and enable the PHY-side delay.

> 
> > 
> > > > > > The u-boot driver (net/ti/am65-cpsw-nuss.c) will configure the delay in
> > > > > > am65_cpsw_gmii_sel_k3(). If the u-boot device tree uses rgmii-id this
> > > > > > patch will break the transmit path because it will disable the PHY delay
> > > > > > on the transmit path, but the bootloader has already disabled the MAC
> > > > > > delay, hence there will be no delay at all.
> > > 
> > > I have a patch that removes this piece of U-Boot code and had intended to submit
> > > that soon to align the U-Boot driver with Linux again. I'll hold off until we
> > > know how the solution in Linux is going to look.
> > 
> > That doesn't fix older bootloaders though. So in linux we still have
> > to work around that.
> 
> I don't consider this something to "work around" but a necessary fix - there is
> simply no way for Linux to do the right thing without knowing whether the MAC
> adds delays (either by reading or writing the flag, if that actually does
> anything).
> 
> > 
> > Although I don't get it why you want to remove that feature.
> 
> My patch was based on the assumption that these bits don't actually work, as the
> datasheet states that there is always a delay... but if this assumption is
> wrong, I need to reconsider my approach.
> 
> 
> > 
> > > > > We have maybe 8 weeks to fix this, before it makes it into a released
> > > > > kernel. So rather than revert, i would prefer to extend the patch to
> > > > > make it work with all variants of the SoC.
> > > > > 
> > > > > Is CTRL_MMR0_CFG0_ENET1_CTRL in the Ethernet address space?
> > > > 
> > > > No, that register is part of the global configuration space (search
> > > > for phy_gmii_sel in the k3-am62p-j722s-common-main.dtsi), but is
> > > > modeled after a PHY (not a network PHY). And actually, I've just
> > > > found out that the PHY driver for that will serve the rgmii_id bit
> > > > if .features has PHY_GMII_SEL_RGMII_ID_MODE set. So there is already
> > > > a whitelist (although it's wrong at the moment, because the J722S
> > > > SoC is not listed as having it). As a side note, the j722s also
> > > > doesn't have it's own SoC specific compatible it is reusing the
> > > > am654-phy-gmii-sel compatible. That might or might not bite us now..
> > > > 
> > > > I digress..
> > > > 
> > > > > Would it be possible for the MAC driver to read it, and know if the delay has
> > > > > been disabled? The switch statement can then be made conditional?
> > > > > 
> > > > > If this register actually exists on all SoC variants, can we just
> > > > > globally disable it, and remove the switch statement?
> > > 
> > > If we just remove the switch statement, thus actually supporting all the
> > > different delay modes, we're back at the point where there is no way for the
> > > driver to determine whether rgmii-rxid is supposed to be interpreted correctly
> > > or not (currently all Device Trees using this driver require the old/incorrect
> > > interpretation for Ethernet to work).
> > 
> > I can't follow you here. Are you assuming that the TX delay is
> > fixed? For me, that's still the culprit. Is that a fair assumption?
> > And only TI can tell us.
> 
> Right. In particular if it's the same for all K3 SoCs - I can only test AM62,
> AM64 and AM67A/J772S here.
> 
> 
> > 
> > > > Given that all the handling is in the PHY subsystem I don't know.
> > > > You'd have to ask the PHY if it supports that, before patching the
> > > > phy-interface-mode - before attaching the network PHY I guess?
> > > 
> > > The previous generation of the CPSW IP handles this in
> > > drivers/net/ethernet/ti/cpsw-phy-sel.c, which is just a custom platform device
> > > referenced by the MAC node. The code currently (partially) implements the
> > > old/incorrect interpretation for phy-mode, enabling the delay on the MAC side
> > > for PHY_INTERFACE_MODE_RGMII.
> > > 
> > > > 
> > > > If we want to just disable (and I assume with disable you mean
> > > > disable the MAC delay) it: the PHY is optional, not sure every SoC
> > > > will have one. And also, the reset default is exactly the opposite
> > > > and TI says it's fixed to the opposite and there has to be a reason
> > > > for that.

I looked this again - every SoC DTSI with a ti,*-cpsw-nuss also has a ti,*-phy-
gmii-sel, which will be used to choose between RMII/RGMII/SGMII/... (depending
on the supported interfaces of each SoC). This is set in the same register as
the ID mode, so it should be trivial to enable or disable the MAC delay where
the flag is supported. We will however need to figure out

- if all SoCs behave the same (and why TI doesn't document the flag)
- how to preserve backwards compatibility with old Device Trees

Best,
Matthias

> > > 
> > > My preference would be to unconditionally enable the MAC-side delay on Linux to
> > > align with the reset default and what the datasheet claims is the only supported
> > > mode, but let's hear what the TI folks think about this.
> > 
> > Which goes against Andrew's "lets to all the delays in the PHY". We
> > have rgmii-id in our AM67A based board and we've actually measured
> > the signals with and without the MAC delay.
> > 
> > Last week, I've also opened an e2e forum case, maybe we get some
> > more insights. Funny enough, TI seem to have a different register
> > description where this bit is described.
> > https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1544031/am67a-internal-rgmii-delay/
> > 
> > -michael
> 

-- 
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] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Andrew Lunn]

> I can confirm that the undocumented/reserved bit switches the MAC-side TX delay
> on and off on the J722S/AM67A.

Thanks.

> I have not checked if there is anything wrong with the undelayed
> mode that might explain why TI doesn't want to support it, but
> traffic appears to flow through the interface without issue if I
> disable the MAC-side and enable the PHY-side delay.

I cannot say this is true for TI, but i've often had vendors say that
they want the MAC to do the delay so you can use a PHY which does not
implement delays. However, every single RGMII PHY driver in Linux
supports all four RGMII modes. So it is a bit of a pointless argument.

And MAC vendors want to make full use of the hardware they have, so
naturally want to do the delay in the MAC because they can.

TI is a bit unusual in this, in that they force the delay on. So that
adds a little bit of weight towards maybe there being a design issue
with it turned off.

	Andrew

Re: [PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Siddharth Vadapalli]

On Wed, Jul 30, 2025 at 04:27:52PM +0200, Andrew Lunn wrote:
> > I can confirm that the undocumented/reserved bit switches the MAC-side TX delay
> > on and off on the J722S/AM67A.
> 
> Thanks.
> 
> > I have not checked if there is anything wrong with the undelayed
> > mode that might explain why TI doesn't want to support it, but
> > traffic appears to flow through the interface without issue if I
> > disable the MAC-side and enable the PHY-side delay.
> 
> I cannot say this is true for TI, but i've often had vendors say that
> they want the MAC to do the delay so you can use a PHY which does not
> implement delays. However, every single RGMII PHY driver in Linux
> supports all four RGMII modes. So it is a bit of a pointless argument.
> 
> And MAC vendors want to make full use of the hardware they have, so
> naturally want to do the delay in the MAC because they can.
> 
> TI is a bit unusual in this, in that they force the delay on. So that
> adds a little bit of weight towards maybe there being a design issue
> with it turned off.

Based on internal discussions with the SoC and Documentation teams,
disabling TX delay in the MAC (CPSW) is not officially supported by
TI. The RGMII switching characteristics have been validated only with
the TX delay enabled - users are therefore expected not to disable it.
Disabling the TX delay may or may not result in an operational system.
This holds true for all SoCs with various CPSW instances that are
programmed by the am65-cpsw-nuss.c driver along with the phy-gmii-sel.c
driver.

In addition to the above, I would like to point out the source of
confusion. When the am65-cpsw-nuss.c driver was written(2020), the
documentation indicated that the internal delay could be disabled.
Later on, the documentation was updated to indicate that internal
delay cannot (should not) be disabled by marking the feature reserved.
This was done to be consistent with the hardware validation performed.
As a result, older documentation contains references to the possibility
of disabling the internal delay whereas newer documentation doesn't.

Regards,
Siddharth.

Re: [PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Michael Walle]

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

On Sat Aug 2, 2025 at 7:44 AM CEST, Siddharth Vadapalli wrote:
> On Wed, Jul 30, 2025 at 04:27:52PM +0200, Andrew Lunn wrote:
> > > I can confirm that the undocumented/reserved bit switches the MAC-side TX delay
> > > on and off on the J722S/AM67A.
> > 
> > Thanks.
> > 
> > > I have not checked if there is anything wrong with the undelayed
> > > mode that might explain why TI doesn't want to support it, but
> > > traffic appears to flow through the interface without issue if I
> > > disable the MAC-side and enable the PHY-side delay.
> > 
> > I cannot say this is true for TI, but i've often had vendors say that
> > they want the MAC to do the delay so you can use a PHY which does not
> > implement delays. However, every single RGMII PHY driver in Linux
> > supports all four RGMII modes. So it is a bit of a pointless argument.
> > 
> > And MAC vendors want to make full use of the hardware they have, so
> > naturally want to do the delay in the MAC because they can.
> > 
> > TI is a bit unusual in this, in that they force the delay on. So that
> > adds a little bit of weight towards maybe there being a design issue
> > with it turned off.
>
> Based on internal discussions with the SoC and Documentation teams,
> disabling TX delay in the MAC (CPSW) is not officially supported by
> TI. The RGMII switching characteristics have been validated only with
> the TX delay enabled - users are therefore expected not to disable it.

Of all the myriad settings of the SoC, this was the one which was
not validated? Anyway, TI should really get that communicated in a
proper way because in the e2e forum you'll get the exact opposite
answer, which is, it is a documentation issue. And also, the
original document available to TI engineers apparently has that setting
documented (judging by the screenshot in the e2e forum).

> Disabling the TX delay may or may not result in an operational system.
> This holds true for all SoCs with various CPSW instances that are
> programmed by the am65-cpsw-nuss.c driver along with the phy-gmii-sel.c
> driver.

In that case u-boot shall be fixed, soon. And to workaround older
u-boot versions, linux shall always enable that delay, like Andrew
proposed.

> In addition to the above, I would like to point out the source of
> confusion. When the am65-cpsw-nuss.c driver was written(2020), the
> documentation indicated that the internal delay could be disabled.
> Later on, the documentation was updated to indicate that internal
> delay cannot (should not) be disabled by marking the feature reserved.

> This was done to be consistent with the hardware validation performed.
> As a result, older documentation contains references to the possibility
> of disabling the internal delay whereas newer documentation doesn't.

See above, that seems to be still the case.

-michael

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

Re: [PATCH net-next] Revert "net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay"

[Matthias Schiffer]

On Mon, 2025-08-04 at 09:37 +0200, Michael Walle wrote:
> On Sat Aug 2, 2025 at 7:44 AM CEST, Siddharth Vadapalli wrote:
> > On Wed, Jul 30, 2025 at 04:27:52PM +0200, Andrew Lunn wrote:
> > > > I can confirm that the undocumented/reserved bit switches the MAC-side TX delay
> > > > on and off on the J722S/AM67A.
> > > 
> > > Thanks.
> > > 
> > > > I have not checked if there is anything wrong with the undelayed
> > > > mode that might explain why TI doesn't want to support it, but
> > > > traffic appears to flow through the interface without issue if I
> > > > disable the MAC-side and enable the PHY-side delay.
> > > 
> > > I cannot say this is true for TI, but i've often had vendors say that
> > > they want the MAC to do the delay so you can use a PHY which does not
> > > implement delays. However, every single RGMII PHY driver in Linux
> > > supports all four RGMII modes. So it is a bit of a pointless argument.
> > > 
> > > And MAC vendors want to make full use of the hardware they have, so
> > > naturally want to do the delay in the MAC because they can.
> > > 
> > > TI is a bit unusual in this, in that they force the delay on. So that
> > > adds a little bit of weight towards maybe there being a design issue
> > > with it turned off.
> > 
> > Based on internal discussions with the SoC and Documentation teams,
> > disabling TX delay in the MAC (CPSW) is not officially supported by
> > TI. The RGMII switching characteristics have been validated only with
> > the TX delay enabled - users are therefore expected not to disable it.
> 
> Of all the myriad settings of the SoC, this was the one which was
> not validated? Anyway, TI should really get that communicated in a
> proper way because in the e2e forum you'll get the exact opposite
> answer, which is, it is a documentation issue. And also, the
> original document available to TI engineers apparently has that setting
> documented (judging by the screenshot in the e2e forum).
> 
> > Disabling the TX delay may or may not result in an operational system.
> > This holds true for all SoCs with various CPSW instances that are
> > programmed by the am65-cpsw-nuss.c driver along with the phy-gmii-sel.c
> > driver.
> 
> In that case u-boot shall be fixed, soon. And to workaround older
> u-boot versions, linux shall always enable that delay, like Andrew
> proposed.

I can submit my patch for U-Boot some time this week, probably tomorrow. Do you
also want me to take care of the Linux side for enabling the MAC delay?

Best,
Matthias


> 
> > In addition to the above, I would like to point out the source of
> > confusion. When the am65-cpsw-nuss.c driver was written(2020), the
> > documentation indicated that the internal delay could be disabled.
> > Later on, the documentation was updated to indicate that internal
> > delay cannot (should not) be disabled by marking the feature reserved.
> 
> > This was done to be consistent with the hardware validation performed.
> > As a result, older documentation contains references to the possibility
> > of disabling the internal delay whereas newer documentation doesn't.
> 
> See above, that seems to be still the case.
> 
> -michael

-- 
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/