| Message ID | 20230724101829.9431-1-ante.knezic@helmholz.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: mv88e6xxx: enable automedia on 6190x and 6390x devices | expand |
On Mon, Jul 24, 2023 at 12:18:29PM +0200, Ante Knezic wrote: > Marvell 6190x and 6390x devices support using unusued lanes of > ports 9 and 10 as automedia PHY/SERDES ports. In order to be > able to use them as automedia ports, serdes lanes must be > properly initialized so we allow setting the desired cmode to > be later used by the phylink_pcs infrastructure. By auto-media, you mean both a copper PHY and an SFP? And whichever gets link first wins the MAC? auto-media has been discussed a few times, and rejected, since Linux has no concept of multiple 'phy like devices' connected to one MAC. How are you representing this in DT? I assume you have both an SFP socket, and a phy-handle pointing to a PHY? phylink will not drive both at the same time. So you cannot have them admin up at the same time? How do you get the SFP out of TX disable, when phylink sees a PHY? What does ethtool return? What the PHY is advertising as its link modes? Or nothing since an SFP does not advertise speeds? Andrew
On Mon, 24 Jul 2023 20:34:27 +0200 Anrew Lunn wrote: >By auto-media, you mean both a copper PHY and an SFP? And whichever >gets link first wins the MAC? > Yes, that is correct. On Mon, 24 Jul 2023 20:34:27 +0200 Anrew Lunn wrote: >auto-media has been discussed a few times, and rejected, since Linux >has no concept of multiple 'phy like devices' connected to one MAC. > >How are you representing this in DT? I assume you have both an SFP >socket, and a phy-handle pointing to a PHY? phylink will not drive >both at the same time. So you cannot have them admin up at the same >time? How do you get the SFP out of TX disable, when phylink sees a >PHY? What does ethtool return? What the PHY is advertising as its link >modes? Or nothing since an SFP does not advertise speeds? Patch simply covers the automedia aspect of the device while the exact mode is specified by the DT. So for example if you would like to connect an SFP to port 3 of the device you would create a "regular" sfp node just like for ports 9/10 along the lines of: port@3 { reg = <3>; label = "SFP"; phy-mode = "1000base-x"; managed = "in-band-status"; sfp = <&sfp1>; }; From then on, phylink will handle the sfp just as if it was connected to ports 9/10 - the ethtool reports advertised and supported link mode as 1000baseX, "Port" is "FIBRE", etc. Patch looks for "1000base-x" phy-mode in the dt node so in case it is not found the device can be linked only against a copper PHY. The "real" automedia you are refering to is of course not covered here and maybe the commit message is a "bit" misleading.
On Tue, Jul 25, 2023 at 11:57:12AM +0200, Ante Knezic wrote: > On Mon, 24 Jul 2023 20:34:27 +0200 Anrew Lunn wrote: > >By auto-media, you mean both a copper PHY and an SFP? And whichever > >gets link first wins the MAC? > > > > Yes, that is correct. > > On Mon, 24 Jul 2023 20:34:27 +0200 Anrew Lunn wrote: > >auto-media has been discussed a few times, and rejected, since Linux > >has no concept of multiple 'phy like devices' connected to one MAC. > > > >How are you representing this in DT? I assume you have both an SFP > >socket, and a phy-handle pointing to a PHY? phylink will not drive > >both at the same time. So you cannot have them admin up at the same > >time? How do you get the SFP out of TX disable, when phylink sees a > >PHY? What does ethtool return? What the PHY is advertising as its link > >modes? Or nothing since an SFP does not advertise speeds? > > Patch simply covers the automedia aspect of the device while the > exact mode is specified by the DT. So i would not call this automedia. You are not supporting both copper and SFP at the same time, and you are not supporting first win. You are just allowing the lower ports to use the SERDES interfaces for SFPs. > So for example if you would like > to connect an SFP to port 3 of the device you would create a "regular" > sfp node just like for ports 9/10 along the lines of: > port@3 { > reg = <3>; > label = "SFP"; > phy-mode = "1000base-x"; > managed = "in-band-status"; > sfp = <&sfp1>; > }; > > >From then on, phylink will handle the sfp just as if it was connected > to ports 9/10 - the ethtool reports advertised and supported link mode > as 1000baseX, "Port" is "FIBRE", etc. > > Patch looks for "1000base-x" phy-mode in the dt node so in case it > is not found the device can be linked only against a copper PHY. So this used to work. It got broken at some point, and i have a much simpler patch in one of the branches: https://github.com/lunn/linux/commit/74e2c2a9a56fd4e2baeee4d5fbe897c21f394ede Please try this and see if it works for you. Andrew
On Tue, 25 Jul 2023 23:22:06 +0200, Andrew Lunn wrote: >On Tue, Jul 25, 2023 at 11:57:12AM +0200, Ante Knezic wrote: >> On Mon, 24 Jul 2023 20:34:27 +0200 Anrew Lunn wrote: >> >By auto-media, you mean both a copper PHY and an SFP? And whichever >> >gets link first wins the MAC? >> > >> >> Yes, that is correct. >> >> On Mon, 24 Jul 2023 20:34:27 +0200 Anrew Lunn wrote: >> >auto-media has been discussed a few times, and rejected, since Linux >> >has no concept of multiple 'phy like devices' connected to one MAC. >> > >> >How are you representing this in DT? I assume you have both an SFP >> >socket, and a phy-handle pointing to a PHY? phylink will not drive >> >both at the same time. So you cannot have them admin up at the same >> >time? How do you get the SFP out of TX disable, when phylink sees a >> >PHY? What does ethtool return? What the PHY is advertising as its link >> >modes? Or nothing since an SFP does not advertise speeds? >> >> Patch simply covers the automedia aspect of the device while the >> exact mode is specified by the DT. > >So i would not call this automedia. You are not supporting both copper >and SFP at the same time, and you are not supporting first win. You >are just allowing the lower ports to use the SERDES interfaces for >SFPs. > That is true, but in order to be able to use SERDES interface for lower ports the switch needs to be operating in automedia mode. Automedia here simply means power up serdes lane and connect to whichever links first, but there is no way to deny linking the PHY and allow only SERDES (as far as I am aware). I agree that commit message is misleading and wrongly assumes that reader has any knowledge of chip details. >> So for example if you would like >> to connect an SFP to port 3 of the device you would create a "regular" >> sfp node just like for ports 9/10 along the lines of: >> port@3 { >> reg = <3>; >> label = "SFP"; >> phy-mode = "1000base-x"; >> managed = "in-band-status"; >> sfp = <&sfp1>; >> }; >> >> >From then on, phylink will handle the sfp just as if it was connected >> to ports 9/10 - the ethtool reports advertised and supported link mode >> as 1000baseX, "Port" is "FIBRE", etc. >> >> Patch looks for "1000base-x" phy-mode in the dt node so in case it >> is not found the device can be linked only against a copper PHY. > >So this used to work. It got broken at some point, and i have a much >simpler patch in one of the branches: > >https://github.com/lunn/linux/commit/74e2c2a9a56fd4e2baeee4d5fbe897c21f394ede > >Please try this and see if it works for you. > The provided patch does work, however I believe there are some drawbacks to such simplification: Firstly, it writes data to READ ONLY bits of the Port Status Register. As indicated by the datasheet, the CMODE is R/W only for ports 9 and 10, other ports actually reflect the actual connection via CMODE bits. Secondly, there is no filtering on setting the ports phy mode. You could be setting cmode of port 3 to sgmii for example, which is not supported. And thirdly, during probing of the device set_cmode is called with argument PHY_INTERFACE_MODE_NA from mv88e6xxx_setup_port. As we would now allow lower ports to set the cmode, it would be automatically assigned a cmode of 1000BASE-X because of: static int mv88e6xxx_port_set_cmode(struct mv88e6xxx_chip *chip, int port, phy_interface_t mode, bool force) { u16 cmode; u16 reg; int err; /* Default to a slow mode, so freeing up SERDES interfaces for * other ports which might use them for SFPs. */ if (mode == PHY_INTERFACE_MODE_NA) mode = PHY_INTERFACE_MODE_1000BASEX; So ultimately, when the pcs->init is called it sees that cmode for all ports is set to 1000BASE-X and proceeds with intialisation (assuming ports 9 or 10 are configured in XAUI/RXAUI depending on which lane is being accessed). If however you believe the above points are irrelevant and there is nothing to be gained by the proposed patch than there is nothing more to be done here.
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c index 5394a8cf7bf1..060cbf8c703c 100644 --- a/drivers/net/dsa/mv88e6xxx/port.c +++ b/drivers/net/dsa/mv88e6xxx/port.c @@ -12,6 +12,7 @@ #include <linux/if_bridge.h> #include <linux/phy.h> #include <linux/phylink.h> +#include <linux/of_net.h> #include "chip.h" #include "global2.h" @@ -596,9 +597,42 @@ static int mv88e6xxx_port_set_cmode(struct mv88e6xxx_chip *chip, int port, return 0; } +static int mv88e6390x_port_automedia(struct mv88e6xxx_chip *chip, int port, + phy_interface_t mode) +{ + phy_interface_t of_mode; + struct dsa_port *dp; + int err; + + if (mode == PHY_INTERFACE_MODE_NA) { + dp = dsa_to_port(chip->ds, port); + err = of_get_phy_mode(dp->dn, &of_mode); + if (err) + return -EOPNOTSUPP; + + if (of_mode != PHY_INTERFACE_MODE_1000BASEX) + return -EOPNOTSUPP; + + /* Physical cmode value is writable only for ports 9 and 10 + * but we need the desired cmode in order to properly prepare + * phylink infrastructure. The physical cmode value will be + * updated by the switch itself once the automedia port is + * linked. + * Automedia ports support only 1000BASE-X cmode. + */ + chip->ports[port].cmode = MV88E6XXX_PORT_STS_CMODE_1000BASEX; + return 0; + } + + return -EOPNOTSUPP; +} + int mv88e6390x_port_set_cmode(struct mv88e6xxx_chip *chip, int port, phy_interface_t mode) { + if (port >= 2 && port <= 7) + return mv88e6390x_port_automedia(chip, port, mode); + if (port != 9 && port != 10) return -EOPNOTSUPP;
Marvell 6190x and 6390x devices support using unusued lanes of ports 9 and 10 as automedia PHY/SERDES ports. In order to be able to use them as automedia ports, serdes lanes must be properly initialized so we allow setting the desired cmode to be later used by the phylink_pcs infrastructure. Signed-off-by: Ante Knezic <ante.knezic@helmholz.de> --- drivers/net/dsa/mv88e6xxx/port.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)