[PATCH] net: ti: am65-cpsw-nuss: allow skipping phy_config() for speed
Nora Schiffer
nora.schiffer at ew.tq-group.com
Mon Jun 15 13:16:19 CEST 2026
On Mon, 2026-06-15 at 13:21 +0530, Siddharth Vadapalli wrote:
> On 15/06/26 12:59, Nora Schiffer wrote:
> > On Fri, 2026-06-12 at 10:03 +0530, Siddharth Vadapalli wrote:
> > > On 11/06/26 18:34, Jai Luthra wrote:
> > >
> > > Hello Jai,
> > >
> > > > Hi Siddharth,
> > > >
> > > > Thank you for the patch.
> > > >
> > > > Quoting Siddharth Vadapalli (2026-06-11 17:36:50)
> > > > > Currently, phy_config() is invoked every time during am65_cpsw_start()
> > > > > which guarantees robustness and also allows recovering from link
> > > > > failures by resetting the Ethernet PHY. While this is beneficial for
> > > > > system stability, for use-cases where speed is prioritized over
> > > > > robustness, the existing implementation is insufficient.
> > > > >
> > > > > To support use-cases prioritizing speed while continuing to retain the
> > > > > existing behavior for robustness, sample the environment variable named
> > > > > "am65_cpsw_phy_config_once" to determine whether phy_config() should
> > > > > continue to be invoked once per invocation of am65_cpsw_start(), OR, it
> > > > > should only be invoked across invocations. Since the environment variable
> > > > > is not set by default, robustness is prioritized over speed. Use-cases
> > > > > which require speed can set "am65_cpsw_phy_config_once" to any of:
> > > > > '1', 'y', 'Y', 't' and 'T'
> > > > > based on the implementation of the env_get_yesno() helper function.
> > > > >
> > > > > Signed-off-by: Siddharth Vadapalli <s-vadapalli at ti.com>
> > > > > ---
> > > > >
> > > > > Hello,
> > > > >
> > > > > This patch is based on commit
> > > > > 3cdce049f90 erge tag 'u-boot-rockchip-20260610' of https://source.denx.de/u-boot/custodians/u-boot-rockchip
> > > > > of the master branch of U-Boot.
> > > > >
> > > > > Patch has been tested on J784S4 EVM which uses MCU CPSW2G for Ethernet
> > > > > functionality. The following test logs demonstrate the default behavior
> > > > > being retained in the absence of 'am65_cpsw_phy_config_once' environment
> > > > > variable being set to a logical true, followed by setting the environment
> > > > > variable to a logical true ('y') and observing the PHY Autonegotiation
> > > > > process being skipped, followed by setting the environment variable to a
> > > > > logical false ('n' but could be anything that isn't logically true based
> > > > > on the env_get_yesno() helper function) and verifying that we go back to
> > > > > the default behavior of performing PHY Autonegotiation again:
> > > > > https://gist.github.com/Siddharth-Vadapalli-at-TI/6229b5a3b7d80cd0c2037a6364406bbe
> > > > >
> > > > > Regards,
> > > > > Siddharth.
> > > > >
> > > > > drivers/net/ti/am65-cpsw-nuss.c | 32 +++++++++++++++++++++++++++-----
> > > > > 1 file changed, 27 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
> > > > > index 84a3e44ecbc..a5cca52f674 100644
> > > > > --- a/drivers/net/ti/am65-cpsw-nuss.c
> > > > > +++ b/drivers/net/ti/am65-cpsw-nuss.c
> > > > > @@ -18,6 +18,7 @@
> > > > > #include <dm/pinctrl.h>
> > > > > #include <dma-uclass.h>
> > > > > #include <dm/of_access.h>
> > > > > +#include <env.h>
> > > > > #include <miiphy.h>
> > > > > #include <net.h>
> > > > > #include <phy.h>
> > > > > @@ -104,6 +105,7 @@ struct am65_cpsw_port {
> > > > > fdt_addr_t macsl_base;
> > > > > bool disabled;
> > > > > u32 mac_control;
> > > > > + bool phy_configured;
> > > > > };
> > > > >
> > > > > struct am65_cpsw_common {
> > > > > @@ -313,7 +315,7 @@ static int am65_cpsw_start(struct udevice *dev)
> > > > > struct am65_cpsw_port *port = &common->ports[priv->port_id];
> > > > > struct am65_cpsw_port *port0 = &common->ports[0];
> > > > > struct ti_udma_drv_chan_cfg_data *dma_rx_cfg_data;
> > > > > - int ret, i;
> > > > > + int ret, i, skip_phy_config_env;
> > > > >
> > > > > if (common->started)
> > > > > return 0;
> > > > > @@ -426,10 +428,30 @@ static int am65_cpsw_start(struct udevice *dev)
> > > > > port->port_sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
> > > > > }
> > > > >
> > > > > - ret = phy_config(priv->phydev);
> > > > > - if (ret < 0) {
> > > > > - dev_err(dev, "phy_config failed: %d", ret);
> > > > > - goto err_dis_rx;
> > > > > + /*
> > > > > + * Invoking phy_config() every time that am65_cpsw_start() is executed will
> > > > > + * make the link robust. However, it delays every networking command such as
> > > > > + * 'dhcp' and 'tftp'. For use-cases that prioritize speed over robustness,
> > > > > + * phy_config() should be invoked only once. To support both use-cases,
> > > > > + * sample the environment variable 'am65_cpsw_phy_config_once' to switch
> > > > > + * between:
> > > > > + * a) Invoke phy_config() every time - Default behavior for robustness
> > > > > + * b) Invoke phy_config() once per driver probe - User configurable behavior
> > > > > + * for speed.
> > > > > + */
> > > > > + skip_phy_config_env = env_get_yesno("am65_cpsw_phy_config_once");
> > > > > +
> > > > > + /* If environment variable is not defined, assume it to be false. */
> > > > > + if (skip_phy_config_env == -1)
> > > > > + skip_phy_config_env = 0;
> > > > > +
> > > > > + if (!port->phy_configured || !skip_phy_config_env) {
> > > > > + ret = phy_config(priv->phydev);
> > > > > + if (ret < 0) {
> > > > > + dev_err(dev, "phy_config failed: %d", ret);
> > > > > + goto err_dis_rx;
> > > > > + }
> > > > > + port->phy_configured = true;
> > > > > }
> > > >
> > > > Tested-by: Jai Luthra <jai.luthra at ideasonboard.com> [on BeagleY-AI]
> > >
> > > Thank you for testing the patch.
> > >
> > > >
> > > > Although I was surprised there's no way to check the status of the phy link
> > > > through some controller bit that can maybe mark phy as unconfigured on next
> > > > attempt? Apologies if that's stupid, I don't understand networking =)
> > > You do ask a good question. Maybe I should have clarified that in response
> > > to Nora's question.
> > >
> > > The issue discovered during automation is that the PHY reports the Link
> > > being UP, but doesn't send any packets out. Based on the statistics in the
> > > MAC (Network Controller / CPSW), the packets are being sent from the MAC to
> > > the PHY, but the PHY isn't transmitting them on the Wire. At this stage,
> > > simply resetting the Ethernet PHY by writing to its registers using the mii
> > > command fixes the issue. Therefore, it seems to be a rare situation wherein
> > > the PHY ends up in a bad state and the Link Status alone isn't sufficient
> > > to figure out whether the PHY is working. Hence, resetting the PHY on every
> > > networking command and starting from scratch not only reduces the
> > > occurrences of the issue, but also helps us recover from the issue by
> > > rerunning the networking commands.
> >
> > It would be really good to figure out what is going on here. If the PHY can get
> > into a state where it incorrectly reports a link, shouldn't the OS be affected
> > as well and not just U-Boot? We don't randomly reset the PHY under Linux either
> > after all.
>
> While that's true, users do have an option to restart the Auto-Negotiation
> process via 'ethtool' command in Linux, and its equivalent isn't possible
> without invoking a 'phy_config' in am65_cpsw_start in U-Boot. I did try
> figuring out what might be going wrong with the PHY but wasn't able to
> root-cause it. I don't know the internals of the PHY so I cannot comment on
> it. In my experimentation, resetting the Ethernet PHY fixed the issue and I
> chose to allow users to do the same so that they don't have to reset the
> SoC for Ethernet Functionality at U-Boot in case it fails.
>
> >
> > I accept that this option may be a valid stopgap measure lacking a proper
> > solution, but it does seem to me like it allows you to choose between
> > "unreliable" and "unusable" for some applications like the mentioned PXE boot,
> > and neither sounds desirable...
>
> The terms "unreliable" and "unusable" don't seem right to me for the PXE
> boot issue that you are referring to.
You are right, "unreliable" is not entirely accurate, at least not when the PXE
boot is run right after a full board reset which should get the PHY into a
working state.
I would argue that PXE boot taking up to a minute instead of a few seconds
because it resets the PHY and reestablishes the link ~10 times could be called
"unusable".
>
> Invoking "phy_config" on every invocation of "am65_cpsw_start" seems to
> cause PXE boot to fail and to address this you can set
> "am65_cpsw_phy_config_once" to true. Eventually, if network functionality
> fails, you only need to set "am65_cpsw_phy_config_once" to false, rerun the
> networking command once, and set "am65_cpsw_phy_config_once" to true again,
> thereby recovering from the issue.
>
> It will help if you clarify why providing the above option to users is a
> bad idea. I am not forcing users to choose either of them at build time.
>
> The alternative you are proposing seems to be that of reverting an earlier
> commit (as you have done in your vendor tree), but that causes stability
> issues. Therefore, to address your use-case, instead of reverting the
> commit, you can still set "am65_cpsw_phy_config_once" to true to get the
> exact same behavior as reverting an earlier commit. Hence, I am failing to
> understand your concern with the flexibility offered by this patch.
My main concern is that this is only a workaround, not a real solution - I'm
neutral wrt whether this patch should be applied or not. If it is applied, we
are likely going to set the variable in our default env in future TQ releases.
One question is why this variable and behavior should be specific to the am65
Ethernet driver, when it looks like it is actually a PHY issue - and if it is
made more generic, what the default behavior should be.
Best,
Nora
>
> Kindly let me know.
>
> Regards,
> Siddharth.
--
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/
More information about the U-Boot
mailing list