lkml.org 
[lkml]   [2022]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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!

\
 
 \ /
  Last update: 2022-06-10 18:37    [W:0.110 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site