Messages in this thread | | | Date | Fri, 10 Jun 2022 17:36:39 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly |
| |
On Fri, Jun 10, 2022 at 05:38:29PM +0200, Alvin Šipraga wrote: > Finally, rtl8365mb_phylink_get_caps() is fixed up to return supported > capabilities based on the external interface properties described above. > This allows for ports with an external interface to be treated as DSA > user ports, and for ports with an internal PHY to be treated as DSA CPU > ports.
I've needed to read that a few times... and I'm still not sure. You seem to be saying that: - ports with an internal PHY (which presumably provide baseT connections?) are used as DSA CPU ports. - ports with an external interface supporting a range of RGMII, SGMII and HSGMII interface modes are DSA user ports.
With Marvell switches, it's the other way around - the ports with an internal PHY are normally DSA user ports. Other ports can be a user, inter-switch or CPU port.
So, I'm slightly confused by your description.
In any ase, looking at the get_caps() (and only that):
> @@ -953,7 +1026,13 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > struct phylink_config *config) > { > - if (dsa_is_user_port(ds, port)) { > + const struct rtl8365mb_extint *extint = > + rtl8365mb_get_port_extint(ds->priv, port); > + > + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > + MAC_10 | MAC_100 | MAC_1000FD;
MAC capabilities are constant across all interfaces - okay.
> + > + if (!extint) { > __set_bit(PHY_INTERFACE_MODE_INTERNAL, > config->supported_interfaces); > > @@ -962,12 +1041,16 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > */ > __set_bit(PHY_INTERFACE_MODE_GMII, > config->supported_interfaces); > - } else if (dsa_is_cpu_port(ds, port)) { > - phy_interface_set_rgmii(config->supported_interfaces); > + return;
Internal ports need to support phylib, so both internal and gmii interface modes - okay.
> } > > - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > - MAC_10 | MAC_100 | MAC_1000FD; > + /* Populate according to the modes supported by _this driver_, > + * not necessarily the modes supported by the hardware, some of > + * which remain unimplemented. > + */ > + > + if (extint->supported_interfaces & RTL8365MB_PHY_INTERFACE_MODE_RGMII) > + phy_interface_set_rgmii(config->supported_interfaces);
External ports that support RGMII get all RGMII modes - also okay.
So, for the get_cops() function, I'm fine with this new code, and for this alone:
Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
I haven't looked at the remainder of the changes.
Thanks.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
| |