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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9dbb7ecc-6073-4948-8db0-2ed584847f7e@nvidia.com>
Date: Tue, 30 Jul 2024 13:25:32 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Revanth Kumar Uppala <ruppala@...dia.com>, "andrew@...n.ch"
 <andrew@...n.ch>, "hkallweit1@...il.com" <hkallweit1@...il.com>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH 3/4] net: phy: aquantia: Poll for TX ready at PHY system
 side


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