From: Maxime Chevallier <maxime.chevallier@bootlin.com> To: "Russell King (Oracle)" <linux@armlinux.org.uk> Cc: davem@davemloft.net, Rob Herring <robh+dt@kernel.org>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, thomas.petazzoni@bootlin.com, Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>, Heiner Kallweit <hkallweit1@gmail.com>, linux-arm-kernel@lists.infradead.org, Richard Cochran <richardcochran@gmail.com>, Horatiu.Vultur@microchip.com, Allan.Nielsen@microchip.com, UNGLinuxDriver@microchip.com Subject: Re: [PATCH net-next 3/6] net: lan966x: Add QUSGMII support for lan966x Date: Wed, 27 Jul 2022 15:48:44 +0200 [thread overview] Message-ID: <20220727154844.3eb3c3d6@pc-10.home> (raw) In-Reply-To: <YoZTj69Une9aKd2C@shell.armlinux.org.uk> Hello Russell, On Thu, 19 May 2022 15:26:23 +0100 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > Hi, > > On Thu, May 19, 2022 at 03:56:44PM +0200, Maxime Chevallier wrote: > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > > b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h index > > e6642083ab9e..304c784f48f6 100644 --- > > a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h +++ > > b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h @@ -452,4 > > +452,10 @@ static inline void lan_rmw(u32 val, u32 mask, struct > > lan966x *lan966x, gcnt, gwidth, raddr, rinst, rcnt, rwidth)); } > > > > +static inline bool lan966x_is_qsgmii(phy_interface_t mode) > > +{ > > + return (mode == PHY_INTERFACE_MODE_QSGMII) || > > + (mode == PHY_INTERFACE_MODE_QUSGMII); > > +} > > Maybe linux/phy.h should provide a helper, something like: > > phy_interface_serdes_lanes() > > that returns how many serdes lanes the interface mode uses? Sorry about the delayed answer, I was resuming the work on this, and realised that although a helper would be indeed great, especially for generic PHY drivers, it won't help much in this case since QSGMII/QUSGMII both use 1 serdes lane, as SGMII and such. If I'm not mistaken, QSGMII is SGMII clocked at 5Gbps with a specific preamble allowing to identify the src/dst port. We could however imagine a helper identifying the number of links, or lanes (or another terminology) that is carried by a given mode. I know that besides QSGMII for 4 ports, there exists PSGMII for 5 ports, and OSGMII for 8 ports, so this would definitely prove useful in the future. Sorry if this ends-up being a misunderstanding on the terminology, we're probably already talking about the same thing, but I think that "serdes lane" would better describe the number of physical differential pairs that creates the link (like, 1 for SGMII, 2 for RXAUI, 4 for XAUI and so on). maybe something like phy_interface_lines() or phy_interface_num_ports() or simply phy_interface_lanes() > > diff --git > > a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c > > b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c index > > 38a7e95d69b4..96708352f53e 100644 --- > > a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c +++ > > b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c @@ > > -28,11 +28,18 @@ static int lan966x_phylink_mac_prepare(struct > > phylink_config *config, phy_interface_t iface) { struct > > lan966x_port *port = netdev_priv(to_net_dev(config->dev)); > > + phy_interface_t serdes_mode = iface; > > int err; > > > > if (port->serdes) { > > + /* As far as the SerDes is concerned, QUSGMII is > > the same as > > + * QSGMII. > > + */ > > + if (lan966x_is_qsgmii(iface)) > > + serdes_mode = PHY_INTERFACE_MODE_QSGMII; > > + > > err = phy_set_mode_ext(port->serdes, > > PHY_MODE_ETHERNET, > > - iface); > > + serdes_mode); > > I don't think that the ethernet MAC driver should be changing the > interface mode before passing it down to the generic PHY layer - > phy_set_mode_ext() is defined to take the phy interface mode, and any > aliasing of modes should really be up to the generic PHY driver not > the ethernet MAC driver. Indeed, I'll split the series so that we first add support for the new mode, and then send separate series for the generic PHY driver on one side, and inband extensions on the other one. Thanks, Maxime > Thanks. >
WARNING: multiple messages have this Message-ID (diff)
From: Maxime Chevallier <maxime.chevallier@bootlin.com> To: "Russell King (Oracle)" <linux@armlinux.org.uk> Cc: davem@davemloft.net, Rob Herring <robh+dt@kernel.org>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, thomas.petazzoni@bootlin.com, Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>, Heiner Kallweit <hkallweit1@gmail.com>, linux-arm-kernel@lists.infradead.org, Richard Cochran <richardcochran@gmail.com>, Horatiu.Vultur@microchip.com, Allan.Nielsen@microchip.com, UNGLinuxDriver@microchip.com Subject: Re: [PATCH net-next 3/6] net: lan966x: Add QUSGMII support for lan966x Date: Wed, 27 Jul 2022 15:48:44 +0200 [thread overview] Message-ID: <20220727154844.3eb3c3d6@pc-10.home> (raw) In-Reply-To: <YoZTj69Une9aKd2C@shell.armlinux.org.uk> Hello Russell, On Thu, 19 May 2022 15:26:23 +0100 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > Hi, > > On Thu, May 19, 2022 at 03:56:44PM +0200, Maxime Chevallier wrote: > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > > b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h index > > e6642083ab9e..304c784f48f6 100644 --- > > a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h +++ > > b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h @@ -452,4 > > +452,10 @@ static inline void lan_rmw(u32 val, u32 mask, struct > > lan966x *lan966x, gcnt, gwidth, raddr, rinst, rcnt, rwidth)); } > > > > +static inline bool lan966x_is_qsgmii(phy_interface_t mode) > > +{ > > + return (mode == PHY_INTERFACE_MODE_QSGMII) || > > + (mode == PHY_INTERFACE_MODE_QUSGMII); > > +} > > Maybe linux/phy.h should provide a helper, something like: > > phy_interface_serdes_lanes() > > that returns how many serdes lanes the interface mode uses? Sorry about the delayed answer, I was resuming the work on this, and realised that although a helper would be indeed great, especially for generic PHY drivers, it won't help much in this case since QSGMII/QUSGMII both use 1 serdes lane, as SGMII and such. If I'm not mistaken, QSGMII is SGMII clocked at 5Gbps with a specific preamble allowing to identify the src/dst port. We could however imagine a helper identifying the number of links, or lanes (or another terminology) that is carried by a given mode. I know that besides QSGMII for 4 ports, there exists PSGMII for 5 ports, and OSGMII for 8 ports, so this would definitely prove useful in the future. Sorry if this ends-up being a misunderstanding on the terminology, we're probably already talking about the same thing, but I think that "serdes lane" would better describe the number of physical differential pairs that creates the link (like, 1 for SGMII, 2 for RXAUI, 4 for XAUI and so on). maybe something like phy_interface_lines() or phy_interface_num_ports() or simply phy_interface_lanes() > > diff --git > > a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c > > b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c index > > 38a7e95d69b4..96708352f53e 100644 --- > > a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c +++ > > b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c @@ > > -28,11 +28,18 @@ static int lan966x_phylink_mac_prepare(struct > > phylink_config *config, phy_interface_t iface) { struct > > lan966x_port *port = netdev_priv(to_net_dev(config->dev)); > > + phy_interface_t serdes_mode = iface; > > int err; > > > > if (port->serdes) { > > + /* As far as the SerDes is concerned, QUSGMII is > > the same as > > + * QSGMII. > > + */ > > + if (lan966x_is_qsgmii(iface)) > > + serdes_mode = PHY_INTERFACE_MODE_QSGMII; > > + > > err = phy_set_mode_ext(port->serdes, > > PHY_MODE_ETHERNET, > > - iface); > > + serdes_mode); > > I don't think that the ethernet MAC driver should be changing the > interface mode before passing it down to the generic PHY layer - > phy_set_mode_ext() is defined to take the phy interface mode, and any > aliasing of modes should really be up to the generic PHY driver not > the ethernet MAC driver. Indeed, I'll split the series so that we first add support for the new mode, and then send separate series for the generic PHY driver on one side, and inband extensions on the other one. Thanks, Maxime > Thanks. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-07-27 13:48 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-19 13:56 [PATCH net-next 0/6] net: Introduce Ethernet Inband Extensions Maxime Chevallier 2022-05-19 13:56 ` Maxime Chevallier 2022-05-19 13:56 ` [PATCH net-next 1/6] net: phy: Introduce QUSGMII PHY mode Maxime Chevallier 2022-05-19 13:56 ` Maxime Chevallier 2022-05-19 13:56 ` [PATCH net-next 2/6] dt-bindings: net: ethernet-controller: add QUSGMII mode Maxime Chevallier 2022-05-19 13:56 ` Maxime Chevallier 2022-06-01 21:10 ` Rob Herring 2022-06-01 21:10 ` Rob Herring 2022-05-19 13:56 ` [PATCH net-next 3/6] net: lan966x: Add QUSGMII support for lan966x Maxime Chevallier 2022-05-19 13:56 ` Maxime Chevallier 2022-05-19 14:26 ` Russell King (Oracle) 2022-05-19 14:26 ` Russell King (Oracle) 2022-07-27 13:48 ` Maxime Chevallier [this message] 2022-07-27 13:48 ` Maxime Chevallier 2022-05-19 13:56 ` [PATCH net-next 4/6] net: phy: Add support for inband extensions Maxime Chevallier 2022-05-19 13:56 ` Maxime Chevallier 2022-05-19 14:10 ` Andrew Lunn 2022-05-19 14:10 ` Andrew Lunn 2022-05-19 14:28 ` Andrew Lunn 2022-05-19 14:28 ` Andrew Lunn 2022-05-19 13:56 ` [PATCH net-next 5/6] net: lan966x: Allow using PCH extension for PTP Maxime Chevallier 2022-05-19 13:56 ` Maxime Chevallier 2022-05-19 13:56 ` [PATCH net-next 6/6] net: phy: micrel: Add QUSGMII support and PCH extension Maxime Chevallier 2022-05-19 13:56 ` Maxime Chevallier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220727154844.3eb3c3d6@pc-10.home \ --to=maxime.chevallier@bootlin.com \ --cc=Allan.Nielsen@microchip.com \ --cc=Horatiu.Vultur@microchip.com \ --cc=UNGLinuxDriver@microchip.com \ --cc=andrew@lunn.ch \ --cc=davem@davemloft.net \ --cc=devicetree@vger.kernel.org \ --cc=f.fainelli@gmail.com \ --cc=hkallweit1@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=netdev@vger.kernel.org \ --cc=richardcochran@gmail.com \ --cc=robh+dt@kernel.org \ --cc=thomas.petazzoni@bootlin.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.