Re: [PATCH 3/4] net: phy: aquantia: Poll for TX ready at PHY system side

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux