netdev - Re: [PATCH net-next 5/6] net: phy: dp83869: Support SGMII SFP modules

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: <b3ff54a1-5242-46d7-8d9d-d469c06a7f7b@lunn.ch>
Date: Tue, 2 Jul 2024 15:21:09 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Romain Gantois <romain.gantois@...tlin.com>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 5/6] net: phy: dp83869: Support SGMII SFP modules

On Tue, Jul 02, 2024 at 10:11:07AM +0200, Romain Gantois wrote:
> Hello Andrew, thanks for the review!
> 
> I think this particular patch warrants that I explain myself a bit more.
> 
> Some SGMII SFP modules will work fine once they're inserted and the appropriate 
> probe() function has been called by the SFP PHY driver. However, this is not 
> necessarily the case, as some SFP PHYs require further configuration before the 
> link can be brought up (e.g. calling phy_init_hw() on them which will 
> eventually call things like config_init()).
> 
> This configuration usually doesn't happen before the PHY device is attached to 
> a network device. In this case, the DP83869 PHY is placed between the MAC and 
> the SFP PHY. Thus, the DP83869 is attached to a network device while the SFP 
> PHY is not. This means that the DP83869 driver basically takes on the role of 
> the MAC driver in some aspects.
> 
> In this patch, I used the connect_phy() callback as a way to get a handle to 
> the downstream SFP PHY. This callback is only implemented by phylink so far.
> 
> I used the module_start() callback to initialize the SFP PHY hardware and 
> start it's state machine.

The SFP PHY is however a PHY which phylib is managing. And you have
phylink on top of that, which knows about both PHYs. Architecturally,
i really think phylink should be dealing with all this
configuration.

The MAC driver has told phylink its pause capabilities.
phylink_bringup_phy() will tell phylib these capabilities by calling
phy_support_asym_pause(). Why does this not work for the SFP PHY?

phylink knows when the SFP PHY is plugged in, and knows if the link is
admin up. It should be starting the state machine, not the PHY.

Do you have access to a macchiatobin? I suggest you play with one, see
how the marvell PHY driver works when you plug in a copper SFP module.

	Andrew