| Message ID | 20180316103351.16616-3-antoine.tenart@bootlin.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: mvpp2: phylink conversion | expand |
On Fri, Mar 16, 2018 at 11:33:43AM +0100, Antoine Tenart wrote: > The PHY mode 10GKR can use in-band negotiation. This patches allows this > mode to be used with MLO_AN_INBAND in phylink. > > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com> > --- > drivers/net/phy/phylink.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index 51a011a349fe..7224b005f0dd 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -768,7 +768,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn, > /* Fixed links and 802.3z are handled without needing a PHY */ > if (pl->link_an_mode == MLO_AN_FIXED || > (pl->link_an_mode == MLO_AN_INBAND && > - phy_interface_mode_is_8023z(pl->link_interface))) > + (phy_interface_mode_is_8023z(pl->link_interface) || > + pl->link_interface == PHY_INTERFACE_MODE_10GKR))) There is no inband negotiation like there is with 802.3z or SGMII, so this makes no sense.
Hi Russell, On Fri, Mar 16, 2018 at 03:53:07PM +0000, Russell King - ARM Linux wrote: > On Fri, Mar 16, 2018 at 11:33:43AM +0100, Antoine Tenart wrote: > > The PHY mode 10GKR can use in-band negotiation. This patches allows this > > mode to be used with MLO_AN_INBAND in phylink. > > > > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com> > > --- > > drivers/net/phy/phylink.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > index 51a011a349fe..7224b005f0dd 100644 > > --- a/drivers/net/phy/phylink.c > > +++ b/drivers/net/phy/phylink.c > > @@ -768,7 +768,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn, > > /* Fixed links and 802.3z are handled without needing a PHY */ > > if (pl->link_an_mode == MLO_AN_FIXED || > > (pl->link_an_mode == MLO_AN_INBAND && > > - phy_interface_mode_is_8023z(pl->link_interface))) > > + (phy_interface_mode_is_8023z(pl->link_interface) || > > + pl->link_interface == PHY_INTERFACE_MODE_10GKR))) > > There is no inband negotiation like there is with 802.3z or SGMII, > so this makes no sense. Oh, that's what I feared. I read some docs but probably will need more :) Anyway, the reason to use in-band negotiation was also to avoid using fixed-link. It would work but always report the link is up, which for the user isn't a great experience as we have a way to detect this. What would you suggest to achieve this in a reasonable way? Thanks, Antoine
On Mon, Mar 19, 2018 at 09:52:52AM +0100, Antoine Tenart wrote: > Hi Russell, > > On Fri, Mar 16, 2018 at 03:53:07PM +0000, Russell King - ARM Linux wrote: > > On Fri, Mar 16, 2018 at 11:33:43AM +0100, Antoine Tenart wrote: > > > The PHY mode 10GKR can use in-band negotiation. This patches allows this > > > mode to be used with MLO_AN_INBAND in phylink. > > > > > > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com> > > > --- > > > drivers/net/phy/phylink.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > > index 51a011a349fe..7224b005f0dd 100644 > > > --- a/drivers/net/phy/phylink.c > > > +++ b/drivers/net/phy/phylink.c > > > @@ -768,7 +768,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn, > > > /* Fixed links and 802.3z are handled without needing a PHY */ > > > if (pl->link_an_mode == MLO_AN_FIXED || > > > (pl->link_an_mode == MLO_AN_INBAND && > > > - phy_interface_mode_is_8023z(pl->link_interface))) > > > + (phy_interface_mode_is_8023z(pl->link_interface) || > > > + pl->link_interface == PHY_INTERFACE_MODE_10GKR))) > > > > There is no inband negotiation like there is with 802.3z or SGMII, > > so this makes no sense. > > Oh, that's what I feared. I read some docs but probably will need more > :) > > Anyway, the reason to use in-band negotiation was also to avoid using > fixed-link. It would work but always report the link is up, which for > the user isn't a great experience as we have a way to detect this. > > What would you suggest to achieve this in a reasonable way? The intention of this test in phylink_of_phy_connect() is to avoid failing when there is no requirement for a PHY to be present (such as a fixed link, or an 802.3z link.) However, with 10G PHYs such as the 3310, we need the PHY so we can read the speed from it, and so know whether to downgrade the MAC to SGMII mode, or having downgraded the MAC, upgrade it back to 10G mode when the PHY switches to 10G. I'm guessing that you're wanting this for the DB boards, but I don't see why. Do they not have PHYs?
Hi Russell, On Mon, Mar 19, 2018 at 11:12:05AM +0000, Russell King - ARM Linux wrote: > On Mon, Mar 19, 2018 at 09:52:52AM +0100, Antoine Tenart wrote: > > > > Anyway, the reason to use in-band negotiation was also to avoid using > > fixed-link. It would work but always report the link is up, which for > > the user isn't a great experience as we have a way to detect this. > > > > What would you suggest to achieve this in a reasonable way? > > The intention of this test in phylink_of_phy_connect() is to avoid > failing when there is no requirement for a PHY to be present (such as > a fixed link, or an 802.3z link.) However, with 10G PHYs such as the > 3310, we need the PHY so we can read the speed from it, and so know > whether to downgrade the MAC to SGMII mode, or having downgraded the > MAC, upgrade it back to 10G mode when the PHY switches to 10G. > > I'm guessing that you're wanting this for the DB boards, but I don't > see why. Do they not have PHYs? You guessed right, that's exactly my use case. The DB boards (7k and 8k) have 10G interfaces without PHYs. I could describe them as fixed-link (it works), but it would be better not to require a PHY in phylink_of_phy_connect() for such interfaces. That's why I used in-band AN, which is wrong, but we still probably need to add a check to allow such setups. I'm all ears for suggestions as I do not have the full picture of all the supported modes and their requirements. Thanks! Antoine
> > > There is no inband negotiation like there is with 802.3z or SGMII, > > > so this makes no sense. > > > > Oh, that's what I feared. I read some docs but probably will need more > > :) > > > > Anyway, the reason to use in-band negotiation was also to avoid using > > fixed-link. It would work but always report the link is up, which for > > the user isn't a great experience as we have a way to detect this. > > > > What would you suggest to achieve this in a reasonable way? > > The intention of this test in phylink_of_phy_connect() is to avoid failing > when there is no requirement for a PHY to be present (such as a fixed link, or > an 802.3z link.) However, with 10G PHYs such as the 3310, we need the PHY > so we can read the speed from it, and so know whether to downgrade the > MAC to SGMII mode, or having downgraded the MAC, upgrade it back to 10G > mode when the PHY switches to 10G. > > I'm guessing that you're wanting this for the DB boards, but I don't see why. > Do they not have PHYs? New Solid Run board MACCHIATObin Single Shot doesn't has 3310 PHY either, like DB boards. https://www.cnx-software.com/2017/12/20/solidrun-macchiatobin-single-shot-networking-board-launched-for-269-and-up/ Stefan, Best Regards.
Hi Antoine > You guessed right, that's exactly my use case. The DB boards (7k and 8k) > have 10G interfaces without PHYs. If they don't have PHYs, how are the connected to the outside world? Andrew
The DTS-patch for this board (in "old" format) is attached Yan Markman Tel. 05-44732819 -----Original Message----- From: Stefan Chulski Sent: Monday, March 19, 2018 2:58 PM To: Russell King - ARM Linux <linux@armlinux.org.uk>; Antoine Tenart <antoine.tenart@bootlin.com> Cc: davem@davemloft.net; kishon@ti.com; gregory.clement@bootlin.com; andrew@lunn.ch; jason@lakedaemon.net; sebastian.hesselbarth@gmail.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; thomas.petazzoni@bootlin.com; maxime.chevallier@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai <nadavh@marvell.com>; Yan Markman <ymarkman@marvell.com>; mw@semihalf.com; linux-arm-kernel@lists.infradead.org Subject: RE: [EXT] Re: [PATCH net-next 02/10] net: phy: phylink: allow 10GKR interface to use in-band negotiation > > > There is no inband negotiation like there is with 802.3z or SGMII, > > > so this makes no sense. > > > > Oh, that's what I feared. I read some docs but probably will need > > more > > :) > > > > Anyway, the reason to use in-band negotiation was also to avoid > > using fixed-link. It would work but always report the link is up, > > which for the user isn't a great experience as we have a way to detect this. > > > > What would you suggest to achieve this in a reasonable way? > > The intention of this test in phylink_of_phy_connect() is to avoid > failing when there is no requirement for a PHY to be present (such as > a fixed link, or an 802.3z link.) However, with 10G PHYs such as the > 3310, we need the PHY so we can read the speed from it, and so know > whether to downgrade the MAC to SGMII mode, or having downgraded the > MAC, upgrade it back to 10G mode when the PHY switches to 10G. > > I'm guessing that you're wanting this for the DB boards, but I don't see why. > Do they not have PHYs? New Solid Run board MACCHIATObin Single Shot doesn't has 3310 PHY either, like DB boards. https://www.cnx-software.com/2017/12/20/solidrun-macchiatobin-single-shot-networking-board-launched-for-269-and-up/ Stefan, Best Regards.
> -----Original Message----- > From: Andrew Lunn [mailto:andrew@lunn.ch] > Sent: Monday, March 19, 2018 3:00 PM > To: Antoine Tenart <antoine.tenart@bootlin.com> > Cc: Russell King - ARM Linux <linux@armlinux.org.uk>; > davem@davemloft.net; kishon@ti.com; gregory.clement@bootlin.com; > jason@lakedaemon.net; sebastian.hesselbarth@gmail.com; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > thomas.petazzoni@bootlin.com; maxime.chevallier@bootlin.com; > miquel.raynal@bootlin.com; Nadav Haklai <nadavh@marvell.com>; Stefan > Chulski <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>; > mw@semihalf.com; linux-arm-kernel@lists.infradead.org > Subject: [EXT] Re: [PATCH net-next 02/10] net: phy: phylink: allow 10GKR > interface to use in-band negotiation > > External Email > > ---------------------------------------------------------------------- > Hi Antoine > > > You guessed right, that's exactly my use case. The DB boards (7k and > > 8k) have 10G interfaces without PHYs. > > If they don't have PHYs, how are the connected to the outside world? > > Andrew By external SFP or direct attached cable. Stefan.
On Mon, Mar 19, 2018 at 01:01:07PM +0000, Yan Markman wrote: > The DTS-patch for this board (in "old" format) is attached > > > Yan Markman > Tel. 05-44732819 > > > -----Original Message----- > From: Stefan Chulski > Sent: Monday, March 19, 2018 2:58 PM > To: Russell King - ARM Linux <linux@armlinux.org.uk>; Antoine Tenart <antoine.tenart@bootlin.com> > Cc: davem@davemloft.net; kishon@ti.com; gregory.clement@bootlin.com; andrew@lunn.ch; jason@lakedaemon.net; sebastian.hesselbarth@gmail.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; thomas.petazzoni@bootlin.com; maxime.chevallier@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai <nadavh@marvell.com>; Yan Markman <ymarkman@marvell.com>; mw@semihalf.com; linux-arm-kernel@lists.infradead.org > Subject: RE: [EXT] Re: [PATCH net-next 02/10] net: phy: phylink: allow 10GKR interface to use in-band negotiation > > > > > There is no inband negotiation like there is with 802.3z or SGMII, > > > > so this makes no sense. > > > > > > Oh, that's what I feared. I read some docs but probably will need > > > more > > > :) > > > > > > Anyway, the reason to use in-band negotiation was also to avoid > > > using fixed-link. It would work but always report the link is up, > > > which for the user isn't a great experience as we have a way to detect this. > > > > > > What would you suggest to achieve this in a reasonable way? > > > > The intention of this test in phylink_of_phy_connect() is to avoid > > failing when there is no requirement for a PHY to be present (such as > > a fixed link, or an 802.3z link.) However, with 10G PHYs such as the > > 3310, we need the PHY so we can read the speed from it, and so know > > whether to downgrade the MAC to SGMII mode, or having downgraded the > > MAC, upgrade it back to 10G mode when the PHY switches to 10G. > > > > I'm guessing that you're wanting this for the DB boards, but I don't see why. > > Do they not have PHYs? > > New Solid Run board MACCHIATObin Single Shot doesn't has 3310 PHY either, like DB boards. > https://www.cnx-software.com/2017/12/20/solidrun-macchiatobin-single-shot-networking-board-launched-for-269-and-up/ Correct, but this DTS is wrong. It connects to a SFP cage, and as SFP cages are supported in mainline now, there's no need to mess around with fixed links or similar. I haven't tested phylink in that configuration yet as SolidRun haven't sent me a SingleShot board yet - and I need any board I do get to have the pull-up resistors on the I2C lines of the correct value, because I'm not risking corruption of the EEPROMs in my SFP* modules.
> > If they don't have PHYs, how are the connected to the outside world? > > > > Andrew > > By external SFP or direct attached cable. Maybe i'm missing something, but don't you just need to add an SFP device in the device tree. The SFP code and PHYLINK will work together, query what the SFP module is, use the GPIOs to determine link up/down and module present, and tell the MAC how to configure the MAC-SFP link? Andrew
Hi Andrew, On Mon, Mar 19, 2018 at 01:59:53PM +0100, Andrew Lunn wrote: > > If they don't have PHYs, how are the connected to the outside world? On 7k/8k you have the following scheme for 10G only interfaces: MAC -- Comphy -- PHY -- SFP cage -- ... Or MAC -- Comphy -- SFP cage -- ... The comphy provides serdes lanes, and can be configured in various modes (SGMII, 2500SGMII, 10GKR...). Antoine
On Mon, Mar 19, 2018 at 02:08:23PM +0100, Andrew Lunn wrote: > > > If they don't have PHYs, how are the connected to the outside world? > > > > By external SFP or direct attached cable. > > Maybe i'm missing something, but don't you just need to add an SFP > device in the device tree. The SFP code and PHYLINK will work > together, query what the SFP module is, use the GPIOs to determine > link up/down and module present, and tell the MAC how to configure the > MAC-SFP link? This would definitively work for SFP connectors with a PHY, but would that work when using direct attached cable? As in the second case phylink wouldn't be able to ask the SFP PHY for the link state. Antoine
On Mon, Mar 19, 2018 at 02:10:09PM +0100, Antoine Tenart wrote: > Hi Andrew, > > On Mon, Mar 19, 2018 at 01:59:53PM +0100, Andrew Lunn wrote: > > > > If they don't have PHYs, how are the connected to the outside world? > > On 7k/8k you have the following scheme for 10G only interfaces: > > MAC -- Comphy -- PHY -- SFP cage -- ... > > Or > > MAC -- Comphy -- SFP cage -- ... > > The comphy provides serdes lanes, and can be configured in various > modes (SGMII, 2500SGMII, 10GKR...). Right - the correct mode is dependent on the SFP module plugged into the cage. Trying to describe this by ignoring the SFP cage isn't going to work out well for end-user functionality, though is fine if you're just hacking a configuration to test (which would not be suitable for mainline kernels!) As I've recently replied to Yan, this is a configuration I haven't tested yet, and it's entirely possible that phylink may need some tweaks for it. What you have is a very similar setup to what is on Clearfog with its SFP cage, where the SFP cage is connected directly to the Armada 388. That only has to deal with 2500base-X / 1000base-X / SGMII and not 10G. What I want is to avoid hacks as much as possible here - if there is a short-coming with SFP/phylink here, we need to address that properly.
> -----Original Message----- > From: Andrew Lunn [mailto:andrew@lunn.ch] > Sent: Monday, March 19, 2018 3:08 PM > To: Stefan Chulski <stefanc@marvell.com> > Cc: Antoine Tenart <antoine.tenart@bootlin.com>; Russell King - ARM Linux > <linux@armlinux.org.uk>; davem@davemloft.net; kishon@ti.com; > gregory.clement@bootlin.com; jason@lakedaemon.net; > sebastian.hesselbarth@gmail.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; thomas.petazzoni@bootlin.com; > maxime.chevallier@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai > <nadavh@marvell.com>; Yan Markman <ymarkman@marvell.com>; > mw@semihalf.com; linux-arm-kernel@lists.infradead.org > Subject: Re: [EXT] Re: [PATCH net-next 02/10] net: phy: phylink: allow 10GKR > interface to use in-band negotiation > > > > If they don't have PHYs, how are the connected to the outside world? > > > > > > Andrew > > > > By external SFP or direct attached cable. > > Maybe i'm missing something, but don't you just need to add an SFP device > in the device tree. The SFP code and PHYLINK will work together, query what > the SFP module is, use the GPIOs to determine link up/down and module > present, and tell the MAC how to configure the MAC-SFP link? > > Andrew phylink pool SFP loss signal to determine link up/down? Stefan, Best Regards.
> phylink pool SFP loss signal to determine link up/down?
Yes, the SFP driver will act on a GPIO for link up/down, and a GPIO
for module prescience. When it detects an SFP module hotplugged, it
reads the EEPROM to find out what sort of module it is, and what mode
should be used between the MAC and SFP module. It will also handle if
the SFP module is copper, and has a PHY in it...
Andrew
> This would definitively work for SFP connectors with a PHY, but would > that work when using direct attached cable? Then you need to know what is on the other end of the cable? A switch? A PHY? and SFP module? Andrew
Hi Russell, On Mon, Mar 19, 2018 at 01:18:05PM +0000, Russell King - ARM Linux wrote: > On Mon, Mar 19, 2018 at 02:10:09PM +0100, Antoine Tenart wrote: > > > > On 7k/8k you have the following scheme for 10G only interfaces: > > > > MAC -- Comphy -- PHY -- SFP cage -- ... > > > > Or > > > > MAC -- Comphy -- SFP cage -- ... > > > > The comphy provides serdes lanes, and can be configured in various > > modes (SGMII, 2500SGMII, 10GKR...). > > Right - the correct mode is dependent on the SFP module plugged into > the cage. Trying to describe this by ignoring the SFP cage isn't > going to work out well for end-user functionality, though is fine if > you're just hacking a configuration to test (which would not be > suitable for mainline kernels!) > > As I've recently replied to Yan, this is a configuration I haven't > tested yet, and it's entirely possible that phylink may need some > tweaks for it. > > What you have is a very similar setup to what is on Clearfog with > its SFP cage, where the SFP cage is connected directly to the > Armada 388. That only has to deal with 2500base-X / 1000base-X / > SGMII and not 10G. > > What I want is to avoid hacks as much as possible here - if there is > a short-coming with SFP/phylink here, we need to address that > properly. OK. So the proper solution would be to properly describe the SFP cages in the device tree (and check if phylink deals with it nicely). I'll update the patches in this way. Thanks for the feedback, Antoine
On Mon, Mar 19, 2018 at 01:19:24PM +0000, Stefan Chulski wrote: > > > > -----Original Message----- > > From: Andrew Lunn [mailto:andrew@lunn.ch] > > Sent: Monday, March 19, 2018 3:08 PM > > To: Stefan Chulski <stefanc@marvell.com> > > Cc: Antoine Tenart <antoine.tenart@bootlin.com>; Russell King - ARM Linux > > <linux@armlinux.org.uk>; davem@davemloft.net; kishon@ti.com; > > gregory.clement@bootlin.com; jason@lakedaemon.net; > > sebastian.hesselbarth@gmail.com; netdev@vger.kernel.org; linux- > > kernel@vger.kernel.org; thomas.petazzoni@bootlin.com; > > maxime.chevallier@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai > > <nadavh@marvell.com>; Yan Markman <ymarkman@marvell.com>; > > mw@semihalf.com; linux-arm-kernel@lists.infradead.org > > Subject: Re: [EXT] Re: [PATCH net-next 02/10] net: phy: phylink: allow 10GKR > > interface to use in-band negotiation > > > > > > If they don't have PHYs, how are the connected to the outside world? > > > > > > > > Andrew > > > > > > By external SFP or direct attached cable. > > > > Maybe i'm missing something, but don't you just need to add an SFP device > > in the device tree. The SFP code and PHYLINK will work together, query what > > the SFP module is, use the GPIOs to determine link up/down and module > > present, and tell the MAC how to configure the MAC-SFP link? > > > > Andrew > > phylink pool SFP loss signal to determine link up/down? No. Phylink was merged along with SFP cage support, which includes a DT description for SFP cages, including the various signals for the cage. The SFP layer will take care of monitoring the cage and conveying state information to Phylink. Phylink's job is to work out how any SFP module, PHY and MAC should be configured and to determine the link state based on information supplied by or requested from each depending on the configured state. Please do not try and support SFP cages as fixed links. They aren't, and such an approach will always have sub-standard link state monitoring. In an optical setup, the SFP LOS signal just indicates whether there is sufficient optical power present at the receiver. It doesn't indicate whether there is a valid signal there, or whether the chip at the other end of the serdes link can decode the signal. That has to come from the upstream chip, whether it be a PHY or a MAC. SFP optical modules do not perform protocol validation - they merely convert the serdes electrical signal into a light signal and back again, with varying amounts of monitoring on board.
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 51a011a349fe..7224b005f0dd 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -768,7 +768,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn, /* Fixed links and 802.3z are handled without needing a PHY */ if (pl->link_an_mode == MLO_AN_FIXED || (pl->link_an_mode == MLO_AN_INBAND && - phy_interface_mode_is_8023z(pl->link_interface))) + (phy_interface_mode_is_8023z(pl->link_interface) || + pl->link_interface == PHY_INTERFACE_MODE_10GKR))) return 0; phy_node = of_parse_phandle(dn, "phy-handle", 0);
The PHY mode 10GKR can use in-band negotiation. This patches allows this mode to be used with MLO_AN_INBAND in phylink. Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com> --- drivers/net/phy/phylink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)