On 30/07/2024 12:12, Russell King (Oracle) wrote: ...
Hmm. dwmac-tegra.c sets STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP, which means that the serdes won't be powered up until after the PHY has indicated that link is up. If the serdes is not powered up, then the MAC facing interface on the PHY won't come up. Hence, the code you're adding will, in all probability, merely add a two second delay to each and every time the PHY is polled for its status when the PHY indicates that the media link is up until such time that the stmmac side has processed that the link has come up. I also note that mgbe_uphy_lane_bringup_serdes_up() polls the link status on the MAC PCS side, waiting for the link to become ready on that side. So, what you have is: - you bring the interface up. The serdes interface remains powered down. - phylib starts polling the PHY. - the PHY indicates the media link is up. - your new code polls the PHY's MAC facing interface for link up, but because the serdes interface is powered down, it ends up timing out after two seconds and then proceeds. - phylib notifies phylink that the PHY has link. - phylink brings the PCS and MAC side(s) up, calling stmmac_mac_link_up(). - stmmac_mac_link_up() calls mgbe_uphy_lane_bringup_serdes_up() which then does receive lane calibration (which is likely the reason why this is delayed to link-up, so the PHY is giving a valid serdes stream for the calibration to use.) - mgbe_uphy_lane_bringup_serdes_up() enables the data path, and clears resets, and then waits for the serdes link with the PHY to come up. While stmmac_mac_link_up() is running, phylib will continue to try to poll the PHY for its status once every second, and each time it does if the PHY's MAC facing link reports that it's down, the phylib locks will be held for _two_ seconds each time. That will mean you won't be able to bring the interface down until those two seconds time out. So, I think one needs to go back and properly understand what is going on to figure out what is going wrong.
Yes agree.
You will likely find that inserting a two second delay at the start of mgbe_uphy_lane_bringup_serdes_up() is just as effective at solving the issue - although I am not suggesting that would be an acceptable solution. It would help to confirm that the reasoning is correct.
I was wondering about something along those lines. We will go back and look at this.
Thanks Jon -- nvpublic
