[PATCH] net: ti: am65-cpsw-nuss: allow skipping phy_config() for speed

[Jai Luthra]

Hi Siddharth,

Quoting Siddharth Vadapalli (2026-06-12 10:03:31)
> 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.
> 
> Since certain users may prefer speed over stability, or alternatively, 
> since users may prefer speed by default with the option to recover on the 
> same boot in case they end up in such a situation, using an environment 
> variable seemed to be the most flexible approach (build-time configs will 
> only provide speed or stability but won't allow the user to switch between 
> them at runtime).
> 
> I hope this clarifies your question. Let me know otherwise.
> 

Thanks, that makes a lot of sense.

Given the failure happens once in two weeks with hundred of tests, I have a
slight preference for keeping the default to not call phy_config()
everytime, or maybe just documenting this environment variable prominently
somewhere, as most developers with a board on their desk would prefer a
broken link every two weeks as opposed to slow transfers.

But I understand anywhere with automation it's okay to spend a little more
time and keep it robust.

Thanks,
    Jai

> Regards,
> Siddharth.