netdev - Re: [PATCH net-next v2 4/4] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

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]

Date: Thu, 13 Jul 2023 13:41:04 +0200
From: Stefan Eichenberger <eichest@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
	hkallweit1@...il.com, francesco.dolcini@...adex.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com
Subject: Re: [PATCH net-next v2 4/4] net: phy: marvell-88q2xxx: add driver
 for the Marvell 88Q2110 PHY

Hi Russell,

Thanks for your reply.

On Thu, Jul 13, 2023 at 11:14:23AM +0100, Russell King (Oracle) wrote:
> On Thu, Jul 13, 2023 at 11:42:12AM +0200, Stefan Eichenberger wrote:
> > With this we will detect link loss in polling mode and read the realtime
> > status in non-polling mode. Compared to genphy_c45_read_link we will not
> > immediately return "link up" in non polling mode but always do the
> > second read to get the realtime link status.
> 
> Why do you think that's better? "Link" only latches low, and the
> entire point of that behaviour is so that management software can
> detect when the link has failed at some point since it last read
> the link status.
> 
> There is only any point in re-reading the status register if on the
> first read it reports that the link has failed, and only then if we
> already knew that the link has failed.
> 
> If we're using interrupt mode, then we need the current link status
> but that's only because of the way phylib has been written.
> 

I agree with you. I missunderstood how the link status worked. I thought
it will always show the status since the last read, independent of
whether it went up or down. But it only latches link down.

> > If we are only interested in the link status we could also skip the
> > remote and local receiver check. However, as I understand the software
> > initialization guide it could be that the receivers are not ready in
> > that moment.
> 
> With copper PHYs, link up status means that the link is ready to pass
> data. Is this not the case with T1 PHYs?

If I interpret their documentation correctly, they differentiate between
PMA and PCS. The PMA link status could be up while the PCS link status
is not. Maybe when the cable test is running this happens. Therefore, I
keep the read for MDIO_MMD_AN_MV_STAT for now.

Regards,
Stefan