On 30/07/2024 13:25, Jon Hunter wrote:
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.
Just to circle back on this. After reviewing the link bring-up sequence
in the mgbe_uphy_lane_bringup_serdes_up() function, the Tegra hardware
team identified some specific sequence and timing requirements we needed
to correct. Paritosh has posted a fix for this [0] and now we have
verified that this fixes the issue we were seeing without making this
change.
Thanks
Jon
[0]
https://lore.kernel.org/linux-tegra/20240923134410.2111640-1-paritoshd@xxxxxxxxxx/
--
nvpublic
