On Mon, 17 Feb 2025 14:21:29 +0000 "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote: > On Mon, Feb 17, 2025 at 09:29:11AM +0100, Maxime Chevallier wrote: > > Hello Russell, > > > > On Sat, 15 Feb 2025 18:57:01 +0000 > > "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote: > > > > > On Thu, Feb 13, 2025 at 11:15:53AM +0100, Maxime Chevallier wrote: > > > > Some PHY devices may be used as media-converters to drive SFP ports (for > > > > example, to allow using SFP when the SoC can only output RGMII). This is > > > > already supported to some extend by allowing PHY drivers to registers > > > > themselves as being SFP upstream. > > > > > > > > However, the logic to drive the SFP can actually be split to a per-port > > > > control logic, allowing support for multi-port PHYs, or PHYs that can > > > > either drive SFPs or Copper. > > > > > > > > To that extent, create a phy_port when registering an SFP bus onto a > > > > PHY. This port is considered a "serdes" port, in that it can feed data > > > > to anther entity on the link. The PHY driver needs to specify the > > > > various PHY_INTERFACE_MODE_XXX that this port supports. > > > > > > > > Signed-off-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx> > > > > > > With this change, using phy_port requires phylink to also be built in > > > an appropriate manner. Currently, phylink depends on phylib. phy_port > > > becomes part of phylib. This patch makes phylib depend on phylink, > > > thereby creating a circular dependency when modular. > > > > > > I think a different approach is needed here. > > > > That's true. > > > > One way to avoid that would be to extract out of phylink/phylib all the > > functions for linkmode handling that aren't tied to phylink/phylib > > directly, but are about managing the capabilities of each interface, > > linkmode, speed, duplex, etc. For phylink, that would be : > > > > phylink_merge_link_mode > > phylink_get_capabilities > > phylink_cap_from_speed_duplex > > phylink_limit_mac_speed > > phylink_caps_to_linkmodes > > phylink_interface_max_speed > > phylink_interface_signal_rate > > phylink_is_empty_linkmode > > phylink_an_mode_str > > phylink_set_port_modes > > > > For now all these are phylink internal and that makes sense, but if we want > > phy-driven SFP support, stackable PHYs and so on, we'll need some ways for > > the PHY to expose its media-side capabilities, and we'd reuse these. > > > > These would go into linkmode.c/h for example, and we'd have a shared set > > of helpers that we can use in phylink, phylib and phy_port. > > > > Before I go around and rearrange that, are you OK with this approach ? > > I'm not convinced. If you're thinking of that level of re-use, you're > probably going to miss out on a lot of logic that's in phylink. Maybe > there should be a way to re-use phylink in its entirety between the > PHY and SFP. > > Some of the above (that deal only with linkmodes) would make sense > to move out though. Yeah I'm thinking about moving only stuff that is phylink-independent and only deals with linkmodes indeed. I'll spin a quick series to see what it looks like then :) Thanks, Maxime