| Message ID | 20231108113445.24825-2-quic_luoj@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add qca8084 ethernet phy driver | expand |
Hello, On Wed, 8 Nov 2023 19:34:43 +0800 Luo Jie <quic_luoj@quicinc.com> wrote: > Add qca8084 PHY support, which is four-port PHY with maximum > link capability 2.5G, the features of each port is almost same > as QCA8081 and slave seed config is not needed. > > Three kind of interface modes supported by qca8084. > PHY_INTERFACE_MODE_QUSGMII, PHY_INTERFACE_MODE_2500BASEX and > PHY_INTERFACE_MODE_SGMII. > > The PCS(serdes) and clock are also needed to be configured to > bringup qca8084 PHY, which will be added in the pcs driver. > > The additional CDT configurations used for qca8084. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- > drivers/net/phy/at803x.c | 48 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) [...] > @@ -1824,6 +1828,21 @@ static int qca808x_read_status(struct phy_device *phydev) > return ret; > > if (phydev->link) { > + /* There are two PCSs available for QCA8084, which support the following > + * interface modes. > + * > + * 1. PHY_INTERFACE_MODE_QUSGMII utilizes PCS1 for all available 4 ports, > + * which is for all link speeds. > + * > + * 2. PHY_INTERFACE_MODE_2500BASEX utilizes PCS0 for the fourth port, > + * which is only for the link speed 2500M same as QCA8081. > + * > + * 3. PHY_INTERFACE_MODE_SGMII utilizes PCS0 for the fourth port, > + * which is for the link speed 10M, 100M and 1000M same as QCA8081. > + */ > + if (phydev->interface == PHY_INTERFACE_MODE_QUSGMII) > + return 0; > + What I understand from this is that this PHY can be used either as a switch, in which case port 4 would be connected to the host interface at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link speed would be limited to 1G per-port, is that correct ? However the get_features function seems to build the supported modes set by reading some capabilities registers : static int qca808x_get_features(struct phy_device *phydev) { [...] ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_CHIP_TYPE); if (ret < 0) return ret; if (QCA808X_PHY_CHIP_TYPE_1G & ret) linkmode_clear_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported); [...] } Wouldn't port 4 report 2.5G capabilities then ? Maybe you need to mask-out the 2.5G bit if the interface is qusgmii. Best regards, Maxime
On 11/8/2023 8:12 PM, Maxime Chevallier wrote: > Hello, > > On Wed, 8 Nov 2023 19:34:43 +0800 > Luo Jie <quic_luoj@quicinc.com> wrote: > >> Add qca8084 PHY support, which is four-port PHY with maximum >> link capability 2.5G, the features of each port is almost same >> as QCA8081 and slave seed config is not needed. >> >> Three kind of interface modes supported by qca8084. >> PHY_INTERFACE_MODE_QUSGMII, PHY_INTERFACE_MODE_2500BASEX and >> PHY_INTERFACE_MODE_SGMII. >> >> The PCS(serdes) and clock are also needed to be configured to >> bringup qca8084 PHY, which will be added in the pcs driver. >> >> The additional CDT configurations used for qca8084. >> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> >> --- >> drivers/net/phy/at803x.c | 48 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 48 insertions(+) > > [...] > >> @@ -1824,6 +1828,21 @@ static int qca808x_read_status(struct phy_device *phydev) >> return ret; >> >> if (phydev->link) { >> + /* There are two PCSs available for QCA8084, which support the following >> + * interface modes. >> + * >> + * 1. PHY_INTERFACE_MODE_QUSGMII utilizes PCS1 for all available 4 ports, >> + * which is for all link speeds. >> + * >> + * 2. PHY_INTERFACE_MODE_2500BASEX utilizes PCS0 for the fourth port, >> + * which is only for the link speed 2500M same as QCA8081. >> + * >> + * 3. PHY_INTERFACE_MODE_SGMII utilizes PCS0 for the fourth port, >> + * which is for the link speed 10M, 100M and 1000M same as QCA8081. >> + */ >> + if (phydev->interface == PHY_INTERFACE_MODE_QUSGMII) >> + return 0; >> + > > What I understand from this is that this PHY can be used either as a > switch, in which case port 4 would be connected to the host interface > at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link > speed would be limited to 1G per-port, is that correct ? When the PHY works on the interface mode QUSGMII for quad-phy, all 4 PHYs can support to the max link speed 2.5G, actually the PHY can support to max link speed 2.5G for all supported interface modes including qusgmii and sgmii. > > However the get_features function seems to build the supported modes > set by reading some capabilities registers : > > static int qca808x_get_features(struct phy_device *phydev) > { > [...] > ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_CHIP_TYPE); > if (ret < 0) > return ret; > > if (QCA808X_PHY_CHIP_TYPE_1G & ret) > linkmode_clear_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported); > [...] > } > > Wouldn't port 4 report 2.5G capabilities then ? Maybe you need to > mask-out the 2.5G bit if the interface is qusgmii. > > Best regards, > > Maxime All ports including port4 support 2.5G capability for the supported interface mode. Thanks Maxime for the review.
Hello, On Thu, 9 Nov 2023 16:32:36 +0800 Jie Luo <quic_luoj@quicinc.com> wrote: [...] > > What I understand from this is that this PHY can be used either as a > > switch, in which case port 4 would be connected to the host interface > > at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link > > speed would be limited to 1G per-port, is that correct ? > > When the PHY works on the interface mode QUSGMII for quad-phy, all 4 > PHYs can support to the max link speed 2.5G, actually the PHY can > support to max link speed 2.5G for all supported interface modes > including qusgmii and sgmii. I'm a bit confused then, as the USGMII spec says that Quad USGMII really is for quad 10/100/1000 speeds, using 10b/8b encoding. Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps with 66b/64b encoding ? Thanks, Maxime
On 11/9/2023 5:16 PM, Maxime Chevallier wrote: > Hello, > > On Thu, 9 Nov 2023 16:32:36 +0800 > Jie Luo <quic_luoj@quicinc.com> wrote: > > [...] > >>> What I understand from this is that this PHY can be used either as a >>> switch, in which case port 4 would be connected to the host interface >>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link >>> speed would be limited to 1G per-port, is that correct ? >> >> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 >> PHYs can support to the max link speed 2.5G, actually the PHY can >> support to max link speed 2.5G for all supported interface modes >> including qusgmii and sgmii. > > I'm a bit confused then, as the USGMII spec says that Quad USGMII really > is for quad 10/100/1000 speeds, using 10b/8b encoding. > > Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps > with 66b/64b encoding ? > > Thanks, > > Maxime Hi Maxime, Yes, for quad PHY mode, it is using 66b/64 encoding. it seems that PHY_INTERFACE_MODE_USXGMII is for single port, so i take the interface name PHY_INTERFACE_MODE_QUSGMII for quad PHYs here. can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this case(qca8084 quad PHY mode)? Thanks, Jie.
On 11/10/2023 4:53 PM, Jie Luo wrote: > > > On 11/9/2023 5:16 PM, Maxime Chevallier wrote: >> Hello, >> >> On Thu, 9 Nov 2023 16:32:36 +0800 >> Jie Luo <quic_luoj@quicinc.com> wrote: >> >> [...] >> >>>> What I understand from this is that this PHY can be used either as a >>>> switch, in which case port 4 would be connected to the host interface >>>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link >>>> speed would be limited to 1G per-port, is that correct ? >>> >>> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 >>> PHYs can support to the max link speed 2.5G, actually the PHY can >>> support to max link speed 2.5G for all supported interface modes >>> including qusgmii and sgmii. >> >> I'm a bit confused then, as the USGMII spec says that Quad USGMII really >> is for quad 10/100/1000 speeds, using 10b/8b encoding. >> >> Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps >> with 66b/64b encoding ? >> >> Thanks, >> >> Maxime > > Hi Maxime, > Yes, for quad PHY mode, it is using 66b/64 encoding. > > it seems that PHY_INTERFACE_MODE_USXGMII is for single port, > so i take the interface name PHY_INTERFACE_MODE_QUSGMII for > quad PHYs here. > > can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this > case(qca8084 quad PHY mode)? > > Thanks, > Jie. one more thing, if we use the PHY_INTERFACE_MODE_USXGMII for the quad PHY here, the MAC serdes can't distinguish the actual mode PHY_INTERFACE_MODE_USXGMII and 10G-QXGMII(qca8084 quad phy mode), the MAC serdes has the different configurations for usxgmii(10g single port) and qxsgmii(quad PHY).
On Fri, 10 Nov 2023 16:53:39 +0800 Jie Luo <quic_luoj@quicinc.com> wrote: > On 11/9/2023 5:16 PM, Maxime Chevallier wrote: > > Hello, > > > > On Thu, 9 Nov 2023 16:32:36 +0800 > > Jie Luo <quic_luoj@quicinc.com> wrote: > > > > [...] > > > >>> What I understand from this is that this PHY can be used either as a > >>> switch, in which case port 4 would be connected to the host interface > >>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link > >>> speed would be limited to 1G per-port, is that correct ? > >> > >> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 > >> PHYs can support to the max link speed 2.5G, actually the PHY can > >> support to max link speed 2.5G for all supported interface modes > >> including qusgmii and sgmii. > > > > I'm a bit confused then, as the USGMII spec says that Quad USGMII really > > is for quad 10/100/1000 speeds, using 10b/8b encoding. > > > > Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps > > with 66b/64b encoding ? > > > > Thanks, > > > > Maxime > > Hi Maxime, > Yes, for quad PHY mode, it is using 66b/64 encoding. > > it seems that PHY_INTERFACE_MODE_USXGMII is for single port, > so i take the interface name PHY_INTERFACE_MODE_QUSGMII for > quad PHYs here. I see, when I added the QUSGMII mode I wrongly stated that it came from the USXGMII spec where it really comes from USGMII, my bad. > can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this > case(qca8084 quad PHY mode)? From what I can see, the USXGMII mode in the kernel is used as the single-port 10G mode of usxgmii. You might need to create a new mode for quad usxgmii at 10G, the spec calls it 10G-QXGMII I think, but as the spec defines quite a lot of modes, should we define all of them or rely on some other parameters to select the actual mode ? Andrew, Heiner, Russell, what do you think ? Maxime > Thanks, > Jie.
On Fri, 10 Nov 2023 17:17:58 +0800 Jie Luo <quic_luoj@quicinc.com> wrote: > On 11/10/2023 4:53 PM, Jie Luo wrote: > > > > > > On 11/9/2023 5:16 PM, Maxime Chevallier wrote: > >> Hello, > >> > >> On Thu, 9 Nov 2023 16:32:36 +0800 > >> Jie Luo <quic_luoj@quicinc.com> wrote: > >> > >> [...] > >> > >>>> What I understand from this is that this PHY can be used either as a > >>>> switch, in which case port 4 would be connected to the host interface > >>>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link > >>>> speed would be limited to 1G per-port, is that correct ? > >>> > >>> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 > >>> PHYs can support to the max link speed 2.5G, actually the PHY can > >>> support to max link speed 2.5G for all supported interface modes > >>> including qusgmii and sgmii. > >> > >> I'm a bit confused then, as the USGMII spec says that Quad USGMII really > >> is for quad 10/100/1000 speeds, using 10b/8b encoding. > >> > >> Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps > >> with 66b/64b encoding ? > >> > >> Thanks, > >> > >> Maxime > > > > Hi Maxime, > > Yes, for quad PHY mode, it is using 66b/64 encoding. > > > > it seems that PHY_INTERFACE_MODE_USXGMII is for single port, > > so i take the interface name PHY_INTERFACE_MODE_QUSGMII for > > quad PHYs here. > > > > can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this > > case(qca8084 quad PHY mode)? > > > > Thanks, > > Jie. > > one more thing, if we use the PHY_INTERFACE_MODE_USXGMII for > the quad PHY here, the MAC serdes can't distinguish the actual > mode PHY_INTERFACE_MODE_USXGMII and 10G-QXGMII(qca8084 quad phy mode), > the MAC serdes has the different configurations for usxgmii(10g single > port) and qxsgmii(quad PHY). Yes you do need a way to know which mode to use, what I'm wondering is that the usxgmii spec actually defines something like 9 different modes ( 1/2/4/8 ports, with a total bandwidth ranging from 2.5Gbps to 20 Gbps ), should we define a phy mode for all of these variants, or should we have another way of getting the mode variant (like, saying I want to use usxgmii, in 4 ports mode, with the serdes at 10.3125Gbps). That being said, QUSGMII already exists to define a specific variant of USGMII, so maybe adding 10G-QXGMII is fine... Also, net-next is still currently closed.
On 11/10/2023 5:33 PM, Maxime Chevallier wrote: > On Fri, 10 Nov 2023 17:17:58 +0800 > Jie Luo <quic_luoj@quicinc.com> wrote: > >> On 11/10/2023 4:53 PM, Jie Luo wrote: >>> >>> >>> On 11/9/2023 5:16 PM, Maxime Chevallier wrote: >>>> Hello, >>>> >>>> On Thu, 9 Nov 2023 16:32:36 +0800 >>>> Jie Luo <quic_luoj@quicinc.com> wrote: >>>> >>>> [...] >>>> >>>>>> What I understand from this is that this PHY can be used either as a >>>>>> switch, in which case port 4 would be connected to the host interface >>>>>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link >>>>>> speed would be limited to 1G per-port, is that correct ? >>>>> >>>>> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 >>>>> PHYs can support to the max link speed 2.5G, actually the PHY can >>>>> support to max link speed 2.5G for all supported interface modes >>>>> including qusgmii and sgmii. >>>> >>>> I'm a bit confused then, as the USGMII spec says that Quad USGMII really >>>> is for quad 10/100/1000 speeds, using 10b/8b encoding. >>>> >>>> Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps >>>> with 66b/64b encoding ? >>>> >>>> Thanks, >>>> >>>> Maxime >>> >>> Hi Maxime, >>> Yes, for quad PHY mode, it is using 66b/64 encoding. >>> >>> it seems that PHY_INTERFACE_MODE_USXGMII is for single port, >>> so i take the interface name PHY_INTERFACE_MODE_QUSGMII for >>> quad PHYs here. >>> >>> can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this >>> case(qca8084 quad PHY mode)? >>> >>> Thanks, >>> Jie. >> >> one more thing, if we use the PHY_INTERFACE_MODE_USXGMII for >> the quad PHY here, the MAC serdes can't distinguish the actual >> mode PHY_INTERFACE_MODE_USXGMII and 10G-QXGMII(qca8084 quad phy mode), >> the MAC serdes has the different configurations for usxgmii(10g single >> port) and qxsgmii(quad PHY). > > Yes you do need a way to know which mode to use, what I'm wondering is > that the usxgmii spec actually defines something like 9 different modes > ( 1/2/4/8 ports, with a total bandwidth ranging from 2.5Gbps to 20 Gbps > ), should we define a phy mode for all of these variants, or should we > have another way of getting the mode variant (like, saying I want to > use usxgmii, in 4 ports mode, with the serdes at 10.3125Gbps). > > That being said, QUSGMII already exists to define a specific variant of > USGMII, so maybe adding 10G-QXGMII is fine... Yes, Maxime, I agree with this solution, the name 10G-QXGMII is exactly the working mode of qca8084 quad phy mode. > > Also, net-next is still currently closed. Ok, thanks for this reminder.
On Fri, Nov 10, 2023 at 05:56:09PM +0800, Jie Luo wrote: > > > > > > > What I understand from this is that this PHY can be used either as a > > > > > > > switch, in which case port 4 would be connected to the host interface > > > > > > > at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link > > > > > > > speed would be limited to 1G per-port, is that correct ? > > > > > > > > > > > > When the PHY works on the interface mode QUSGMII for quad-phy, all 4 > > > > > > PHYs can support to the max link speed 2.5G, actually the PHY can > > > > > > support to max link speed 2.5G for all supported interface modes > > > > > > including qusgmii and sgmii. > > > > > > > > > > I'm a bit confused then, as the USGMII spec says that Quad USGMII really > > > > > is for quad 10/100/1000 speeds, using 10b/8b encoding. > > > > > > > > > > Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps > > > > > with 66b/64b encoding ? > > > > > > > > Hi Maxime, > > > > Yes, for quad PHY mode, it is using 66b/64 encoding. > > > > > > > > it seems that PHY_INTERFACE_MODE_USXGMII is for single port, > > > > so i take the interface name PHY_INTERFACE_MODE_QUSGMII for > > > > quad PHYs here. > > > > > > > > can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this > > > > case(qca8084 quad PHY mode)? > > > > > > > > Thanks, > > > > Jie. > > > > > > one more thing, if we use the PHY_INTERFACE_MODE_USXGMII for > > > the quad PHY here, the MAC serdes can't distinguish the actual > > > mode PHY_INTERFACE_MODE_USXGMII and 10G-QXGMII(qca8084 quad phy mode), > > > the MAC serdes has the different configurations for usxgmii(10g single > > > port) and qxsgmii(quad PHY). > > > > Yes you do need a way to know which mode to use, what I'm wondering is > > that the usxgmii spec actually defines something like 9 different modes > > ( 1/2/4/8 ports, with a total bandwidth ranging from 2.5Gbps to 20 Gbps > > ), should we define a phy mode for all of these variants, or should we > > have another way of getting the mode variant (like, saying I want to > > use usxgmii, in 4 ports mode, with the serdes at 10.3125Gbps). > > > > That being said, QUSGMII already exists to define a specific variant of > > USGMII, so maybe adding 10G-QXGMII is fine... > > Yes, Maxime, I agree with this solution, the name 10G-QXGMII is exactly > the working mode of qca8084 quad phy mode. FWIW, NXP has these 2 patches in its trees. One day I was going to make an attempt at upstreaming them, but the Aquantia PHY driver is a bit of a sticky topic since I could find no way to distinguish between the single-port and multi-port variants of USXGMII in this PHY IP, and the AQR412 uses a multi-port mode which is now treated as USXGMII by Linux. Anyway, if you do make progress with this ahead of me, please be aware that these patches exist, and I would appreciate if you tried to keep the name as "10g-qxgmii" so that I don't have to modify the (downstream) device trees again :) >From 8f4c8227362bb7acad09cd756390f1efd3311395 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: Fri, 6 Oct 2023 23:42:35 +0300 Subject: [PATCH] net: dsa: felix: introduce phy-mode = "10g-qxgmii" to replace "usxgmii" The "USXGMII" mode that the Felix switch ports support on LS1028A is not quite USXGMII, it is defined by the USXGMII multiport specification document as 10G-QXGMII. It uses the same signaling as USXGMII, but it multiplexes 4 ports over the link, resulting in a maximum speed of 2.5G per port. This change is needed in preparation for the lynx-10g driver on LS1028A, which will make a more clear distinction between usxgmii (supported on lane 0) and 10g-qxgmii (supported on lane 1). These protocols have their configuration in different PCCR registers (PCCRB vs PCCR9). We touch the entire kernel side: the phylib core, phylink, the Felix switch driver, the Lynx PCS and the AQR412 driver. The existing LS1028A-QDS device trees will continue to work with "usxgmii", and will be updated to "10g-qxgmii" as a separate patch. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- .../bindings/net/ethernet-controller.yaml | 1 + Documentation/networking/phy.rst | 6 ++++++ drivers/net/dsa/ocelot/felix.c | 1 + drivers/net/dsa/ocelot/felix.h | 3 ++- drivers/net/dsa/ocelot/felix_vsc9959.c | 3 ++- drivers/net/pcs/pcs-lynx.c | 8 ++++++-- drivers/net/phy/aquantia_main.c | 15 +++++++++++++++ drivers/net/phy/phy-core.c | 1 + drivers/net/phy/phylink.c | 12 ++++++++++-- include/linux/phy.h | 4 ++++ include/linux/phylink.h | 1 + 11 files changed, 49 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index 9f6a5ccbcefe..044880d804db 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -104,6 +104,7 @@ properties: - usxgmii - 10gbase-r - 25gbase-r + - 10g-qxgmii phy-mode: $ref: "#/properties/phy-connection-type" diff --git a/Documentation/networking/phy.rst b/Documentation/networking/phy.rst index 1283240d7620..f64641417c54 100644 --- a/Documentation/networking/phy.rst +++ b/Documentation/networking/phy.rst @@ -327,6 +327,12 @@ Some of the interface modes are described below: This is the Penta SGMII mode, it is similar to QSGMII but it combines 5 SGMII lines into a single link compared to 4 on QSGMII. +``PHY_INTERFACE_MODE_10G_QXGMII`` + Represents the 10G-QXGMII PHY-MAC interface as defined by the Cisco USXGMII + Multiport Copper Interface document. It supports 4 ports over a 10.3125 GHz + SerDes lane, each port having speeds of 2.5G / 1G / 100M / 10M achieved + through symbol replication. The PCS expects the standard USXGMII code word. + Pause frames / flow control =========================== diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index 9a3e5ec16972..17fa31b58be4 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -1232,6 +1232,7 @@ static const u32 felix_phy_match_table[PHY_INTERFACE_MODE_MAX] = { [PHY_INTERFACE_MODE_SGMII] = OCELOT_PORT_MODE_SGMII, [PHY_INTERFACE_MODE_QSGMII] = OCELOT_PORT_MODE_QSGMII, [PHY_INTERFACE_MODE_USXGMII] = OCELOT_PORT_MODE_USXGMII, + [PHY_INTERFACE_MODE_10G_QXGMII] = OCELOT_PORT_MODE_10G_QXGMII, [PHY_INTERFACE_MODE_1000BASEX] = OCELOT_PORT_MODE_1000BASEX, [PHY_INTERFACE_MODE_2500BASEX] = OCELOT_PORT_MODE_2500BASEX, }; diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h index 1d4befe7cfe8..4cf849e5782f 100644 --- a/drivers/net/dsa/ocelot/felix.h +++ b/drivers/net/dsa/ocelot/felix.h @@ -12,8 +12,9 @@ #define OCELOT_PORT_MODE_SGMII BIT(1) #define OCELOT_PORT_MODE_QSGMII BIT(2) #define OCELOT_PORT_MODE_2500BASEX BIT(3) -#define OCELOT_PORT_MODE_USXGMII BIT(4) +#define OCELOT_PORT_MODE_USXGMII BIT(4) /* compatibility */ #define OCELOT_PORT_MODE_1000BASEX BIT(5) +#define OCELOT_PORT_MODE_10G_QXGMII BIT(6) struct device_node; diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c index 3c5509e75a54..8ae9c2e340ab 100644 --- a/drivers/net/dsa/ocelot/felix_vsc9959.c +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c @@ -34,7 +34,8 @@ OCELOT_PORT_MODE_QSGMII | \ OCELOT_PORT_MODE_1000BASEX | \ OCELOT_PORT_MODE_2500BASEX | \ - OCELOT_PORT_MODE_USXGMII) + OCELOT_PORT_MODE_USXGMII | \ + OCELOT_PORT_MODE_10G_QXGMII) static const u32 vsc9959_port_modes[VSC9959_NUM_PORTS] = { VSC9959_PORT_MODE_SERDES, diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c index 7e1e8f61f043..f3ab17ef4e31 100644 --- a/drivers/net/pcs/pcs-lynx.c +++ b/drivers/net/pcs/pcs-lynx.c @@ -95,6 +95,7 @@ static void lynx_pcs_get_state(struct phylink_pcs *pcs, lynx_pcs_get_state_2500basex(lynx->mdio, state); break; case PHY_INTERFACE_MODE_USXGMII: + case PHY_INTERFACE_MODE_10G_QXGMII: lynx_pcs_get_state_usxgmii(lynx->mdio, state); break; case PHY_INTERFACE_MODE_10GBASER: @@ -151,6 +152,7 @@ static int lynx_pcs_config_giga(struct mdio_device *pcs, } static int lynx_pcs_config_usxgmii(struct mdio_device *pcs, + phy_interface_t interface, const unsigned long *advertising, unsigned int neg_mode) { @@ -158,7 +160,8 @@ static int lynx_pcs_config_usxgmii(struct mdio_device *pcs, int addr = pcs->addr; if (neg_mode != PHYLINK_PCS_NEG_INBAND_ENABLED) { - dev_err(&pcs->dev, "USXGMII only supports in-band AN for now\n"); + dev_err(&pcs->dev, "%s only supports in-band AN for now\n", + phy_modes(interface)); return -EOPNOTSUPP; } @@ -189,7 +192,8 @@ static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode, } break; case PHY_INTERFACE_MODE_USXGMII: - return lynx_pcs_config_usxgmii(lynx->mdio, advertising, + case PHY_INTERFACE_MODE_10G_QXGMII: + return lynx_pcs_config_usxgmii(lynx->mdio, ifmode, advertising, neg_mode); case PHY_INTERFACE_MODE_10GBASER: case PHY_INTERFACE_MODE_25GBASER: diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c index 5fc93bbfbe49..4f5e6575be7b 100644 --- a/drivers/net/phy/aquantia_main.c +++ b/drivers/net/phy/aquantia_main.c @@ -512,6 +512,20 @@ static int aqr107_read_status(struct phy_device *phydev) if (!phydev->link || phydev->autoneg == AUTONEG_DISABLE) return 0; + /* Quad port PHYs like AQR412 have 4 system interfaces, but they can + * also be used with a single system interface over which all 4 ports + * are multiplexed (10G-QXGMII). To the MDIO registers, this mode is + * indistinguishable from USXGMII (which implies a single 10G port), + * which is problematic because the detection logic below would + * overwrite phydev->interface with a wrong value. If the device tree + * is configured for "10g-qxgmii", always trust that value, since it is + * very unlikely that the PHY firmware was configured for protocol + * switching depending on link speed (all USXGMII variants are capable + * of symbol replication). + */ + if (phydev->interface == PHY_INTERFACE_MODE_10G_QXGMII) + goto skip_iface_detection; + val = phy_read_mmd(phydev, MDIO_MMD_PHYXS, MDIO_PHYXS_VEND_IF_STATUS); if (val < 0) return val; @@ -546,6 +560,7 @@ static int aqr107_read_status(struct phy_device *phydev) break; } +skip_iface_detection: /* Read possibly downshifted rate from vendor register */ return aqr107_read_rate(phydev); } diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c index cfa6f08d8ca3..95222680a82d 100644 --- a/drivers/net/phy/phy-core.c +++ b/drivers/net/phy/phy-core.c @@ -142,6 +142,7 @@ int phy_interface_num_ports(phy_interface_t interface) return 1; case PHY_INTERFACE_MODE_QSGMII: case PHY_INTERFACE_MODE_QUSGMII: + case PHY_INTERFACE_MODE_10G_QXGMII: return 4; case PHY_INTERFACE_MODE_PSGMII: return 5; diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 3bf923f88494..dd0ac7139aa4 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -191,6 +191,7 @@ static unsigned int phylink_pcs_neg_mode(unsigned int mode, phy_interface_t inte case PHY_INTERFACE_MODE_QSGMII: case PHY_INTERFACE_MODE_QUSGMII: case PHY_INTERFACE_MODE_USXGMII: + case PHY_INTERFACE_MODE_10G_QXGMII: /* These protocols are designed for use with a PHY which * communicates its negotiation result back to the MAC via * inband communication. Note: there exist PHYs that run @@ -284,6 +285,7 @@ static int phylink_interface_max_speed(phy_interface_t interface) case PHY_INTERFACE_MODE_2500BASEX: case PHY_INTERFACE_MODE_2500SGMII: + case PHY_INTERFACE_MODE_10G_QXGMII: return SPEED_2500; case PHY_INTERFACE_MODE_5GBASER: @@ -553,7 +555,11 @@ unsigned long phylink_get_capabilities(phy_interface_t interface, switch (interface) { case PHY_INTERFACE_MODE_USXGMII: - caps |= MAC_10000FD | MAC_5000FD | MAC_2500FD; + caps |= MAC_10000FD | MAC_5000FD; + fallthrough; + + case PHY_INTERFACE_MODE_10G_QXGMII: + caps |= MAC_2500FD; fallthrough; case PHY_INTERFACE_MODE_RGMII_TXID: @@ -969,6 +975,7 @@ static int phylink_parse_mode(struct phylink *pl, case PHY_INTERFACE_MODE_5GBASER: case PHY_INTERFACE_MODE_25GBASER: case PHY_INTERFACE_MODE_USXGMII: + case PHY_INTERFACE_MODE_10G_QXGMII: case PHY_INTERFACE_MODE_10GKR: case PHY_INTERFACE_MODE_10GBASER: case PHY_INTERFACE_MODE_XLGMII: @@ -1804,7 +1811,8 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy, if (phy->is_c45 && config.rate_matching == RATE_MATCH_NONE && interface != PHY_INTERFACE_MODE_RXAUI && interface != PHY_INTERFACE_MODE_XAUI && - interface != PHY_INTERFACE_MODE_USXGMII) + interface != PHY_INTERFACE_MODE_USXGMII && + interface != PHY_INTERFACE_MODE_10G_QXGMII) config.interface = PHY_INTERFACE_MODE_NA; else config.interface = interface; diff --git a/include/linux/phy.h b/include/linux/phy.h index 184b04422877..0eeeea50d9a2 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -125,6 +125,7 @@ extern const int phy_10gbit_features_array[1]; * @PHY_INTERFACE_MODE_10GKR: 10GBASE-KR - with Clause 73 AN * @PHY_INTERFACE_MODE_QUSGMII: Quad Universal SGMII * @PHY_INTERFACE_MODE_1000BASEKX: 1000Base-KX - with Clause 73 AN + * @PHY_INTERFACE_MODE_10G_QXGMII: 10G-QXGMII - 4 ports over 10G USXGMII * @PHY_INTERFACE_MODE_MAX: Book keeping * * Describes the interface between the MAC and PHY. @@ -166,6 +167,7 @@ typedef enum { PHY_INTERFACE_MODE_QUSGMII, PHY_INTERFACE_MODE_1000BASEKX, PHY_INTERFACE_MODE_2500SGMII, + PHY_INTERFACE_MODE_10G_QXGMII, PHY_INTERFACE_MODE_MAX, } phy_interface_t; @@ -289,6 +291,8 @@ static inline const char *phy_modes(phy_interface_t interface) return "qusgmii"; case PHY_INTERFACE_MODE_2500SGMII: return "sgmii-2500"; + case PHY_INTERFACE_MODE_10G_QXGMII: + return "10g-qxgmii"; default: return "unknown"; } diff --git a/include/linux/phylink.h b/include/linux/phylink.h index 871db640ceb6..336780e76e34 100644 --- a/include/linux/phylink.h +++ b/include/linux/phylink.h @@ -648,6 +648,7 @@ static inline int phylink_get_link_timer_ns(phy_interface_t interface) case PHY_INTERFACE_MODE_SGMII: case PHY_INTERFACE_MODE_QSGMII: case PHY_INTERFACE_MODE_USXGMII: + case PHY_INTERFACE_MODE_10G_QXGMII: return 1600000; case PHY_INTERFACE_MODE_1000BASEX:
On 11/12/2023 6:54 AM, Vladimir Oltean wrote: > On Fri, Nov 10, 2023 at 05:56:09PM +0800, Jie Luo wrote: >>>>>>>> What I understand from this is that this PHY can be used either as a >>>>>>>> switch, in which case port 4 would be connected to the host interface >>>>>>>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link >>>>>>>> speed would be limited to 1G per-port, is that correct ? >>>>>>> >>>>>>> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 >>>>>>> PHYs can support to the max link speed 2.5G, actually the PHY can >>>>>>> support to max link speed 2.5G for all supported interface modes >>>>>>> including qusgmii and sgmii. >>>>>> >>>>>> I'm a bit confused then, as the USGMII spec says that Quad USGMII really >>>>>> is for quad 10/100/1000 speeds, using 10b/8b encoding. >>>>>> >>>>>> Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps >>>>>> with 66b/64b encoding ? >>>>> >>>>> Hi Maxime, >>>>> Yes, for quad PHY mode, it is using 66b/64 encoding. >>>>> >>>>> it seems that PHY_INTERFACE_MODE_USXGMII is for single port, >>>>> so i take the interface name PHY_INTERFACE_MODE_QUSGMII for >>>>> quad PHYs here. >>>>> >>>>> can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this >>>>> case(qca8084 quad PHY mode)? >>>>> >>>>> Thanks, >>>>> Jie. >>>> >>>> one more thing, if we use the PHY_INTERFACE_MODE_USXGMII for >>>> the quad PHY here, the MAC serdes can't distinguish the actual >>>> mode PHY_INTERFACE_MODE_USXGMII and 10G-QXGMII(qca8084 quad phy mode), >>>> the MAC serdes has the different configurations for usxgmii(10g single >>>> port) and qxsgmii(quad PHY). >>> >>> Yes you do need a way to know which mode to use, what I'm wondering is >>> that the usxgmii spec actually defines something like 9 different modes >>> ( 1/2/4/8 ports, with a total bandwidth ranging from 2.5Gbps to 20 Gbps >>> ), should we define a phy mode for all of these variants, or should we >>> have another way of getting the mode variant (like, saying I want to >>> use usxgmii, in 4 ports mode, with the serdes at 10.3125Gbps). >>> >>> That being said, QUSGMII already exists to define a specific variant of >>> USGMII, so maybe adding 10G-QXGMII is fine... >> >> Yes, Maxime, I agree with this solution, the name 10G-QXGMII is exactly >> the working mode of qca8084 quad phy mode. > > FWIW, NXP has these 2 patches in its trees. One day I was going to make > an attempt at upstreaming them, but the Aquantia PHY driver is a bit of > a sticky topic since I could find no way to distinguish between the > single-port and multi-port variants of USXGMII in this PHY IP, and the > AQR412 uses a multi-port mode which is now treated as USXGMII by Linux. > > Anyway, if you do make progress with this ahead of me, please be aware > that these patches exist, and I would appreciate if you tried to keep > the name as "10g-qxgmii" so that I don't have to modify the (downstream) > device trees again :) > > From 8f4c8227362bb7acad09cd756390f1efd3311395 Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Fri, 6 Oct 2023 23:42:35 +0300 > Subject: [PATCH] net: dsa: felix: introduce phy-mode = "10g-qxgmii" to replace > "usxgmii" > > The "USXGMII" mode that the Felix switch ports support on LS1028A is not > quite USXGMII, it is defined by the USXGMII multiport specification > document as 10G-QXGMII. It uses the same signaling as USXGMII, but it > multiplexes 4 ports over the link, resulting in a maximum speed of 2.5G > per port. > > This change is needed in preparation for the lynx-10g driver on LS1028A, > which will make a more clear distinction between usxgmii (supported on > lane 0) and 10g-qxgmii (supported on lane 1). These protocols have their > configuration in different PCCR registers (PCCRB vs PCCR9). > > We touch the entire kernel side: the phylib core, phylink, the Felix > switch driver, the Lynx PCS and the AQR412 driver. The existing > LS1028A-QDS device trees will continue to work with "usxgmii", and will > be updated to "10g-qxgmii" as a separate patch. Sure Vladimir, Thanks for sharing this patch. BTW, When do you upstream this patch? or Maybe you can upstream the separate patch for introducing the new interface mode 10g-qxgmii firstly? if that, i can also update qca8084 phy driver based on your patch. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > .../bindings/net/ethernet-controller.yaml | 1 + > Documentation/networking/phy.rst | 6 ++++++ > drivers/net/dsa/ocelot/felix.c | 1 + > drivers/net/dsa/ocelot/felix.h | 3 ++- > drivers/net/dsa/ocelot/felix_vsc9959.c | 3 ++- > drivers/net/pcs/pcs-lynx.c | 8 ++++++-- > drivers/net/phy/aquantia_main.c | 15 +++++++++++++++ > drivers/net/phy/phy-core.c | 1 + > drivers/net/phy/phylink.c | 12 ++++++++++-- > include/linux/phy.h | 4 ++++ > include/linux/phylink.h | 1 + > 11 files changed, 49 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > index 9f6a5ccbcefe..044880d804db 100644 > --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml > +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > @@ -104,6 +104,7 @@ properties: > - usxgmii > - 10gbase-r > - 25gbase-r > + - 10g-qxgmii > > phy-mode: > $ref: "#/properties/phy-connection-type" > diff --git a/Documentation/networking/phy.rst b/Documentation/networking/phy.rst > index 1283240d7620..f64641417c54 100644 > --- a/Documentation/networking/phy.rst > +++ b/Documentation/networking/phy.rst > @@ -327,6 +327,12 @@ Some of the interface modes are described below: > This is the Penta SGMII mode, it is similar to QSGMII but it combines 5 > SGMII lines into a single link compared to 4 on QSGMII. > > +``PHY_INTERFACE_MODE_10G_QXGMII`` > + Represents the 10G-QXGMII PHY-MAC interface as defined by the Cisco USXGMII > + Multiport Copper Interface document. It supports 4 ports over a 10.3125 GHz > + SerDes lane, each port having speeds of 2.5G / 1G / 100M / 10M achieved > + through symbol replication. The PCS expects the standard USXGMII code word. > + > Pause frames / flow control > =========================== > > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c > index 9a3e5ec16972..17fa31b58be4 100644 > --- a/drivers/net/dsa/ocelot/felix.c > +++ b/drivers/net/dsa/ocelot/felix.c > @@ -1232,6 +1232,7 @@ static const u32 felix_phy_match_table[PHY_INTERFACE_MODE_MAX] = { > [PHY_INTERFACE_MODE_SGMII] = OCELOT_PORT_MODE_SGMII, > [PHY_INTERFACE_MODE_QSGMII] = OCELOT_PORT_MODE_QSGMII, > [PHY_INTERFACE_MODE_USXGMII] = OCELOT_PORT_MODE_USXGMII, > + [PHY_INTERFACE_MODE_10G_QXGMII] = OCELOT_PORT_MODE_10G_QXGMII, > [PHY_INTERFACE_MODE_1000BASEX] = OCELOT_PORT_MODE_1000BASEX, > [PHY_INTERFACE_MODE_2500BASEX] = OCELOT_PORT_MODE_2500BASEX, > }; > diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h > index 1d4befe7cfe8..4cf849e5782f 100644 > --- a/drivers/net/dsa/ocelot/felix.h > +++ b/drivers/net/dsa/ocelot/felix.h > @@ -12,8 +12,9 @@ > #define OCELOT_PORT_MODE_SGMII BIT(1) > #define OCELOT_PORT_MODE_QSGMII BIT(2) > #define OCELOT_PORT_MODE_2500BASEX BIT(3) > -#define OCELOT_PORT_MODE_USXGMII BIT(4) > +#define OCELOT_PORT_MODE_USXGMII BIT(4) /* compatibility */ > #define OCELOT_PORT_MODE_1000BASEX BIT(5) > +#define OCELOT_PORT_MODE_10G_QXGMII BIT(6) > > struct device_node; > > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c > index 3c5509e75a54..8ae9c2e340ab 100644 > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c > @@ -34,7 +34,8 @@ > OCELOT_PORT_MODE_QSGMII | \ > OCELOT_PORT_MODE_1000BASEX | \ > OCELOT_PORT_MODE_2500BASEX | \ > - OCELOT_PORT_MODE_USXGMII) > + OCELOT_PORT_MODE_USXGMII | \ > + OCELOT_PORT_MODE_10G_QXGMII) > > static const u32 vsc9959_port_modes[VSC9959_NUM_PORTS] = { > VSC9959_PORT_MODE_SERDES, > diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c > index 7e1e8f61f043..f3ab17ef4e31 100644 > --- a/drivers/net/pcs/pcs-lynx.c > +++ b/drivers/net/pcs/pcs-lynx.c > @@ -95,6 +95,7 @@ static void lynx_pcs_get_state(struct phylink_pcs *pcs, > lynx_pcs_get_state_2500basex(lynx->mdio, state); > break; > case PHY_INTERFACE_MODE_USXGMII: > + case PHY_INTERFACE_MODE_10G_QXGMII: > lynx_pcs_get_state_usxgmii(lynx->mdio, state); > break; > case PHY_INTERFACE_MODE_10GBASER: > @@ -151,6 +152,7 @@ static int lynx_pcs_config_giga(struct mdio_device *pcs, > } > > static int lynx_pcs_config_usxgmii(struct mdio_device *pcs, > + phy_interface_t interface, > const unsigned long *advertising, > unsigned int neg_mode) > { > @@ -158,7 +160,8 @@ static int lynx_pcs_config_usxgmii(struct mdio_device *pcs, > int addr = pcs->addr; > > if (neg_mode != PHYLINK_PCS_NEG_INBAND_ENABLED) { > - dev_err(&pcs->dev, "USXGMII only supports in-band AN for now\n"); > + dev_err(&pcs->dev, "%s only supports in-band AN for now\n", > + phy_modes(interface)); > return -EOPNOTSUPP; > } > > @@ -189,7 +192,8 @@ static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode, > } > break; > case PHY_INTERFACE_MODE_USXGMII: > - return lynx_pcs_config_usxgmii(lynx->mdio, advertising, > + case PHY_INTERFACE_MODE_10G_QXGMII: > + return lynx_pcs_config_usxgmii(lynx->mdio, ifmode, advertising, > neg_mode); > case PHY_INTERFACE_MODE_10GBASER: > case PHY_INTERFACE_MODE_25GBASER: > diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c > index 5fc93bbfbe49..4f5e6575be7b 100644 > --- a/drivers/net/phy/aquantia_main.c > +++ b/drivers/net/phy/aquantia_main.c > @@ -512,6 +512,20 @@ static int aqr107_read_status(struct phy_device *phydev) > if (!phydev->link || phydev->autoneg == AUTONEG_DISABLE) > return 0; > > + /* Quad port PHYs like AQR412 have 4 system interfaces, but they can > + * also be used with a single system interface over which all 4 ports > + * are multiplexed (10G-QXGMII). To the MDIO registers, this mode is > + * indistinguishable from USXGMII (which implies a single 10G port), > + * which is problematic because the detection logic below would > + * overwrite phydev->interface with a wrong value. If the device tree > + * is configured for "10g-qxgmii", always trust that value, since it is > + * very unlikely that the PHY firmware was configured for protocol > + * switching depending on link speed (all USXGMII variants are capable > + * of symbol replication). > + */ > + if (phydev->interface == PHY_INTERFACE_MODE_10G_QXGMII) > + goto skip_iface_detection; > + > val = phy_read_mmd(phydev, MDIO_MMD_PHYXS, MDIO_PHYXS_VEND_IF_STATUS); > if (val < 0) > return val; > @@ -546,6 +560,7 @@ static int aqr107_read_status(struct phy_device *phydev) > break; > } > > +skip_iface_detection: > /* Read possibly downshifted rate from vendor register */ > return aqr107_read_rate(phydev); > } > diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c > index cfa6f08d8ca3..95222680a82d 100644 > --- a/drivers/net/phy/phy-core.c > +++ b/drivers/net/phy/phy-core.c > @@ -142,6 +142,7 @@ int phy_interface_num_ports(phy_interface_t interface) > return 1; > case PHY_INTERFACE_MODE_QSGMII: > case PHY_INTERFACE_MODE_QUSGMII: > + case PHY_INTERFACE_MODE_10G_QXGMII: > return 4; > case PHY_INTERFACE_MODE_PSGMII: > return 5; > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index 3bf923f88494..dd0ac7139aa4 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -191,6 +191,7 @@ static unsigned int phylink_pcs_neg_mode(unsigned int mode, phy_interface_t inte > case PHY_INTERFACE_MODE_QSGMII: > case PHY_INTERFACE_MODE_QUSGMII: > case PHY_INTERFACE_MODE_USXGMII: > + case PHY_INTERFACE_MODE_10G_QXGMII: > /* These protocols are designed for use with a PHY which > * communicates its negotiation result back to the MAC via > * inband communication. Note: there exist PHYs that run > @@ -284,6 +285,7 @@ static int phylink_interface_max_speed(phy_interface_t interface) > > case PHY_INTERFACE_MODE_2500BASEX: > case PHY_INTERFACE_MODE_2500SGMII: > + case PHY_INTERFACE_MODE_10G_QXGMII: > return SPEED_2500; > > case PHY_INTERFACE_MODE_5GBASER: > @@ -553,7 +555,11 @@ unsigned long phylink_get_capabilities(phy_interface_t interface, > > switch (interface) { > case PHY_INTERFACE_MODE_USXGMII: > - caps |= MAC_10000FD | MAC_5000FD | MAC_2500FD; > + caps |= MAC_10000FD | MAC_5000FD; > + fallthrough; > + > + case PHY_INTERFACE_MODE_10G_QXGMII: > + caps |= MAC_2500FD; > fallthrough; > > case PHY_INTERFACE_MODE_RGMII_TXID: > @@ -969,6 +975,7 @@ static int phylink_parse_mode(struct phylink *pl, > case PHY_INTERFACE_MODE_5GBASER: > case PHY_INTERFACE_MODE_25GBASER: > case PHY_INTERFACE_MODE_USXGMII: > + case PHY_INTERFACE_MODE_10G_QXGMII: > case PHY_INTERFACE_MODE_10GKR: > case PHY_INTERFACE_MODE_10GBASER: > case PHY_INTERFACE_MODE_XLGMII: > @@ -1804,7 +1811,8 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy, > if (phy->is_c45 && config.rate_matching == RATE_MATCH_NONE && > interface != PHY_INTERFACE_MODE_RXAUI && > interface != PHY_INTERFACE_MODE_XAUI && > - interface != PHY_INTERFACE_MODE_USXGMII) > + interface != PHY_INTERFACE_MODE_USXGMII && > + interface != PHY_INTERFACE_MODE_10G_QXGMII) > config.interface = PHY_INTERFACE_MODE_NA; > else > config.interface = interface; > diff --git a/include/linux/phy.h b/include/linux/phy.h > index 184b04422877..0eeeea50d9a2 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -125,6 +125,7 @@ extern const int phy_10gbit_features_array[1]; > * @PHY_INTERFACE_MODE_10GKR: 10GBASE-KR - with Clause 73 AN > * @PHY_INTERFACE_MODE_QUSGMII: Quad Universal SGMII > * @PHY_INTERFACE_MODE_1000BASEKX: 1000Base-KX - with Clause 73 AN > + * @PHY_INTERFACE_MODE_10G_QXGMII: 10G-QXGMII - 4 ports over 10G USXGMII > * @PHY_INTERFACE_MODE_MAX: Book keeping > * > * Describes the interface between the MAC and PHY. > @@ -166,6 +167,7 @@ typedef enum { > PHY_INTERFACE_MODE_QUSGMII, > PHY_INTERFACE_MODE_1000BASEKX, > PHY_INTERFACE_MODE_2500SGMII, > + PHY_INTERFACE_MODE_10G_QXGMII, > PHY_INTERFACE_MODE_MAX, > } phy_interface_t; > > @@ -289,6 +291,8 @@ static inline const char *phy_modes(phy_interface_t interface) > return "qusgmii"; > case PHY_INTERFACE_MODE_2500SGMII: > return "sgmii-2500"; > + case PHY_INTERFACE_MODE_10G_QXGMII: > + return "10g-qxgmii"; > default: > return "unknown"; > } > diff --git a/include/linux/phylink.h b/include/linux/phylink.h > index 871db640ceb6..336780e76e34 100644 > --- a/include/linux/phylink.h > +++ b/include/linux/phylink.h > @@ -648,6 +648,7 @@ static inline int phylink_get_link_timer_ns(phy_interface_t interface) > case PHY_INTERFACE_MODE_SGMII: > case PHY_INTERFACE_MODE_QSGMII: > case PHY_INTERFACE_MODE_USXGMII: > + case PHY_INTERFACE_MODE_10G_QXGMII: > return 1600000; > > case PHY_INTERFACE_MODE_1000BASEX:
On Sun, Nov 12, 2023 at 07:27:50PM +0800, Jie Luo wrote: > Sure Vladimir, Thanks for sharing this patch. > > BTW, When do you upstream this patch? or Maybe you can upstream the > separate patch for introducing the new interface mode 10g-qxgmii firstly? if > that, i can also update qca8084 phy driver based on > your patch. I've removed the driver changes from the patch and formatted it on net-next. There's also one more dependency patch. Both are attached to this email. I don't think I will find the time to upstream them by the time you need them. I think it would be best if you could take these patches, add your Signed-off-by: tag below mine, and submit them as the first 2 patches of your own series.
On 11/13/2023 7:58 AM, Vladimir Oltean wrote: > On Sun, Nov 12, 2023 at 07:27:50PM +0800, Jie Luo wrote: >> Sure Vladimir, Thanks for sharing this patch. >> >> BTW, When do you upstream this patch? or Maybe you can upstream the >> separate patch for introducing the new interface mode 10g-qxgmii firstly? if >> that, i can also update qca8084 phy driver based on >> your patch. > > I've removed the driver changes from the patch and formatted it on > net-next. There's also one more dependency patch. Both are attached to > this email. > > I don't think I will find the time to upstream them by the time you > need them. I think it would be best if you could take these patches, > add your Signed-off-by: tag below mine, and submit them as the first 2 > patches of your own series. OK, Vladimir, will take these two patches in the next updated patch set.
On Mon, Nov 13, 2023 at 01:58:52AM +0200, Vladimir Oltean wrote: > On Sun, Nov 12, 2023 at 07:27:50PM +0800, Jie Luo wrote: > > Sure Vladimir, Thanks for sharing this patch. > > > > BTW, When do you upstream this patch? or Maybe you can upstream the > > separate patch for introducing the new interface mode 10g-qxgmii firstly? if > > that, i can also update qca8084 phy driver based on > > your patch. > > I've removed the driver changes from the patch and formatted it on > net-next. There's also one more dependency patch. Both are attached to > this email. > > I don't think I will find the time to upstream them by the time you > need them. I think it would be best if you could take these patches, > add your Signed-off-by: tag below mine, and submit them as the first 2 > patches of your own series. > From 17fd68123d78f39a971f800de6da66522f71dc71 Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Tue, 3 Oct 2023 22:16:25 +0300 > Subject: [PATCH 1/2] net: phylink: move phylink_pcs_neg_mode() to phylink.c > > Russell points out that there is no user of phylink_pcs_neg_mode() > outside of phylink.c, nor is there planned to be any, so we can just > move it there. Looks familiar... http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=c2aa9d3846c218d28a8a3457b0447998b0d84c5d
On Fri, Nov 10, 2023 at 10:18:41AM +0100, Maxime Chevallier wrote: > On Fri, 10 Nov 2023 16:53:39 +0800 > Jie Luo <quic_luoj@quicinc.com> wrote: > > > On 11/9/2023 5:16 PM, Maxime Chevallier wrote: > > > Hello, > > > > > > On Thu, 9 Nov 2023 16:32:36 +0800 > > > Jie Luo <quic_luoj@quicinc.com> wrote: > > > > > > [...] > > > > > >>> What I understand from this is that this PHY can be used either as a > > >>> switch, in which case port 4 would be connected to the host interface > > >>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link > > >>> speed would be limited to 1G per-port, is that correct ? > > >> > > >> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 > > >> PHYs can support to the max link speed 2.5G, actually the PHY can > > >> support to max link speed 2.5G for all supported interface modes > > >> including qusgmii and sgmii. > > > > > > I'm a bit confused then, as the USGMII spec says that Quad USGMII really > > > is for quad 10/100/1000 speeds, using 10b/8b encoding. > > > > > > Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps > > > with 66b/64b encoding ? > > > > > > Thanks, > > > > > > Maxime > > > > Hi Maxime, > > Yes, for quad PHY mode, it is using 66b/64 encoding. > > > > it seems that PHY_INTERFACE_MODE_USXGMII is for single port, > > so i take the interface name PHY_INTERFACE_MODE_QUSGMII for > > quad PHYs here. > > I see, when I added the QUSGMII mode I wrongly stated that it came from > the USXGMII spec where it really comes from USGMII, my bad. > > > can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this > > case(qca8084 quad PHY mode)? > > From what I can see, the USXGMII mode in the kernel is used as the > single-port 10G mode of usxgmii. You might need to create a new mode > for quad usxgmii at 10G, the spec calls it 10G-QXGMII I think, but as > the spec defines quite a lot of modes, should we define all of them or > rely on some other parameters to select the actual mode ? > > Andrew, Heiner, Russell, what do you think ? Looking at the Cisco USXGMII Multiport Copper Interface specification, you appear to be correct with the "10G-QXGMII" name. I note that it is also specified that "System Interface operates in full duplex mode only." and makes no other significant mention of duplex, so it's not clear whether half duplex is supported on the media side. Figure 2 in this document is the significant one that we need to consider, as we're talking about N network interfaces connecting to a system interface that then connects to a PHY with multiple ports. Given our model, I think it's quite right to use "10G-QXGMII" because that describes the protocol over the system interface that will be used. However, we need to consider whether this is the only information we need, or whether we need to also be thinking about expanding the "pcs" property to something such as: pcs = <&usxgmiim_pcs PORT>; where PORT is the port number on the USXGMII PHY as described by figure 2. It seems to me that a driver for this USXGMII PHY would need to know the port information that a network interface is connected to.
On Mon, Nov 13, 2023 at 01:42:20PM +0000, Russell King (Oracle) wrote: > On Mon, Nov 13, 2023 at 01:58:52AM +0200, Vladimir Oltean wrote: > > From 17fd68123d78f39a971f800de6da66522f71dc71 Mon Sep 17 00:00:00 2001 > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > Date: Tue, 3 Oct 2023 22:16:25 +0300 > > Subject: [PATCH 1/2] net: phylink: move phylink_pcs_neg_mode() to phylink.c > > > > Russell points out that there is no user of phylink_pcs_neg_mode() > > outside of phylink.c, nor is there planned to be any, so we can just > > move it there. > > Looks familiar... > > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=c2aa9d3846c218d28a8a3457b0447998b0d84c5d Well, yeah, I did mention that the patch was written at your suggestion, and there aren't that many options in which that patch can be written. I didn't look at your trees, and I made that change as part of a much larger effort which involves phylink, which I will email you separately about. I will gladly drop my ownership on the first patch and ask Luo Jie to pick your version instead, if this is what you're implying from the 2 word reply.
On 11/13/2023 11:11 PM, Russell King (Oracle) wrote: > On Fri, Nov 10, 2023 at 10:18:41AM +0100, Maxime Chevallier wrote: >> On Fri, 10 Nov 2023 16:53:39 +0800 >> Jie Luo <quic_luoj@quicinc.com> wrote: >> >>> On 11/9/2023 5:16 PM, Maxime Chevallier wrote: >>>> Hello, >>>> >>>> On Thu, 9 Nov 2023 16:32:36 +0800 >>>> Jie Luo <quic_luoj@quicinc.com> wrote: >>>> >>>> [...] >>>> >>>>>> What I understand from this is that this PHY can be used either as a >>>>>> switch, in which case port 4 would be connected to the host interface >>>>>> at up to 2.5G, or as a quad-phy, but since it uses QUSGMII the link >>>>>> speed would be limited to 1G per-port, is that correct ? >>>>> >>>>> When the PHY works on the interface mode QUSGMII for quad-phy, all 4 >>>>> PHYs can support to the max link speed 2.5G, actually the PHY can >>>>> support to max link speed 2.5G for all supported interface modes >>>>> including qusgmii and sgmii. >>>> >>>> I'm a bit confused then, as the USGMII spec says that Quad USGMII really >>>> is for quad 10/100/1000 speeds, using 10b/8b encoding. >>>> >>>> Aren't you using the USXGMII mode instead, which can convey 4 x 2.5Gbps >>>> with 66b/64b encoding ? >>>> >>>> Thanks, >>>> >>>> Maxime >>> >>> Hi Maxime, >>> Yes, for quad PHY mode, it is using 66b/64 encoding. >>> >>> it seems that PHY_INTERFACE_MODE_USXGMII is for single port, >>> so i take the interface name PHY_INTERFACE_MODE_QUSGMII for >>> quad PHYs here. >> >> I see, when I added the QUSGMII mode I wrongly stated that it came from >> the USXGMII spec where it really comes from USGMII, my bad. >> >>> can we apply PHY_INTERFACE_MODE_USXGMII to quad PHYs in this >>> case(qca8084 quad PHY mode)? >> >> From what I can see, the USXGMII mode in the kernel is used as the >> single-port 10G mode of usxgmii. You might need to create a new mode >> for quad usxgmii at 10G, the spec calls it 10G-QXGMII I think, but as >> the spec defines quite a lot of modes, should we define all of them or >> rely on some other parameters to select the actual mode ? >> >> Andrew, Heiner, Russell, what do you think ? > > Looking at the Cisco USXGMII Multiport Copper Interface specification, > you appear to be correct with the "10G-QXGMII" name. I note that it is > also specified that "System Interface operates in full duplex mode > only." and makes no other significant mention of duplex, so it's not > clear whether half duplex is supported on the media side. > Yes, the half duplex is not supported by PCS. > Figure 2 in this document is the significant one that we need to > consider, as we're talking about N network interfaces connecting to a > system interface that then connects to a PHY with multiple ports. > > Given our model, I think it's quite right to use "10G-QXGMII" because > that describes the protocol over the system interface that will be > used. However, we need to consider whether this is the only information > we need, or whether we need to also be thinking about expanding the > "pcs" property to something such as: > > pcs = <&usxgmiim_pcs PORT>; > > where PORT is the port number on the USXGMII PHY as described by figure > 2. It seems to me that a driver for this USXGMII PHY would need to know > the port information that a network interface is connected to. > That's true. There are 4 channels of 10G-QXGMII PCS for mapping with quad PHY ports, the port ID information of PCS needs be delivered to driver. In the PCS driver, it seems we need to know the current channel of PCS taking the link status change when the PHY link updated.
On Mon, Nov 13, 2023 at 09:51:20PM +0200, Vladimir Oltean wrote: > On Mon, Nov 13, 2023 at 01:42:20PM +0000, Russell King (Oracle) wrote: > > On Mon, Nov 13, 2023 at 01:58:52AM +0200, Vladimir Oltean wrote: > > > From 17fd68123d78f39a971f800de6da66522f71dc71 Mon Sep 17 00:00:00 2001 > > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > > Date: Tue, 3 Oct 2023 22:16:25 +0300 > > > Subject: [PATCH 1/2] net: phylink: move phylink_pcs_neg_mode() to phylink.c > > > > > > Russell points out that there is no user of phylink_pcs_neg_mode() > > > outside of phylink.c, nor is there planned to be any, so we can just > > > move it there. > > > > Looks familiar... > > > > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=c2aa9d3846c218d28a8a3457b0447998b0d84c5d > > Well, yeah, I did mention that the patch was written at your suggestion, > and there aren't that many options in which that patch can be written. > I didn't look at your trees, and I made that change as part of a much > larger effort which involves phylink, which I will email you separately > about. > > I will gladly drop my ownership on the first patch and ask Luo Jie to > pick your version instead, if this is what you're implying from the 2 > word reply. The reason that I hadn't submitted it was because I didn't want to move the function out of the header file until the next LTS was released. It seems 6.6 was announced as a LTS on the 17th November, so I'm happier to now proceed with moving this into phylink.c. as phylink_pcs_neg_mode() was merged in 6.5-rc1, it will have had three kernel cycles - including one each side of the LTS release which I think is reasonable. I will send my patch hopefully sometime this week so it's in 6.8 depending on pressures.
On 1/2/2024 10:37 PM, Russell King (Oracle) wrote: > On Mon, Nov 13, 2023 at 09:51:20PM +0200, Vladimir Oltean wrote: >> On Mon, Nov 13, 2023 at 01:42:20PM +0000, Russell King (Oracle) wrote: >>> On Mon, Nov 13, 2023 at 01:58:52AM +0200, Vladimir Oltean wrote: >>>> From 17fd68123d78f39a971f800de6da66522f71dc71 Mon Sep 17 00:00:00 2001 >>>> From: Vladimir Oltean <vladimir.oltean@nxp.com> >>>> Date: Tue, 3 Oct 2023 22:16:25 +0300 >>>> Subject: [PATCH 1/2] net: phylink: move phylink_pcs_neg_mode() to phylink.c >>>> >>>> Russell points out that there is no user of phylink_pcs_neg_mode() >>>> outside of phylink.c, nor is there planned to be any, so we can just >>>> move it there. >>> >>> Looks familiar... >>> >>> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=c2aa9d3846c218d28a8a3457b0447998b0d84c5d >> >> Well, yeah, I did mention that the patch was written at your suggestion, >> and there aren't that many options in which that patch can be written. >> I didn't look at your trees, and I made that change as part of a much >> larger effort which involves phylink, which I will email you separately >> about. >> >> I will gladly drop my ownership on the first patch and ask Luo Jie to >> pick your version instead, if this is what you're implying from the 2 >> word reply. > > The reason that I hadn't submitted it was because I didn't want to move > the function out of the header file until the next LTS was released. > > It seems 6.6 was announced as a LTS on the 17th November, so I'm happier > to now proceed with moving this into phylink.c. > > as phylink_pcs_neg_mode() was merged in 6.5-rc1, it will have had three > kernel cycles - including one each side of the LTS release which I think > is reasonable. > > I will send my patch hopefully sometime this week so it's in 6.8 > depending on pressures. > Thanks for updating the information.
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index 37fb033e1c29..f2a0d1688159 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -176,6 +176,7 @@ #define AT8030_PHY_ID_MASK 0xffffffef #define QCA8081_PHY_ID 0x004dd101 +#define QCA8084_PHY_ID 0x004dd180 #define QCA8327_A_PHY_ID 0x004dd033 #define QCA8327_B_PHY_ID 0x004dd034 @@ -1760,6 +1761,9 @@ static bool qca808x_is_prefer_master(struct phy_device *phydev) static bool qca808x_has_fast_retrain_or_slave_seed(struct phy_device *phydev) { + if (phydev_id_compare(phydev, QCA8084_PHY_ID)) + return false; + return linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported); } @@ -1824,6 +1828,21 @@ static int qca808x_read_status(struct phy_device *phydev) return ret; if (phydev->link) { + /* There are two PCSs available for QCA8084, which support the following + * interface modes. + * + * 1. PHY_INTERFACE_MODE_QUSGMII utilizes PCS1 for all available 4 ports, + * which is for all link speeds. + * + * 2. PHY_INTERFACE_MODE_2500BASEX utilizes PCS0 for the fourth port, + * which is only for the link speed 2500M same as QCA8081. + * + * 3. PHY_INTERFACE_MODE_SGMII utilizes PCS0 for the fourth port, + * which is for the link speed 10M, 100M and 1000M same as QCA8081. + */ + if (phydev->interface == PHY_INTERFACE_MODE_QUSGMII) + return 0; + if (phydev->speed == SPEED_2500) phydev->interface = PHY_INTERFACE_MODE_2500BASEX; else @@ -1958,6 +1977,14 @@ static int qca808x_cable_test_start(struct phy_device *phydev) phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807a, 0xc060); phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807e, 0xb060); + if (phydev_id_compare(phydev, QCA8084_PHY_ID)) { + /* Adjust the positive and negative pulse thereshold of CDT */ + phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8075, 0xa060); + + /* Disable the near echo bypass */ + phy_modify_mmd(phydev, MDIO_MMD_PCS, 0x807f, BIT(15), 0); + } + return 0; } @@ -2227,6 +2254,26 @@ static struct phy_driver at803x_driver[] = { .cable_test_start = qca808x_cable_test_start, .cable_test_get_status = qca808x_cable_test_get_status, .link_change_notify = qca808x_link_change_notify, +}, { + /* Qualcomm QCA8084 */ + PHY_ID_MATCH_MODEL(QCA8084_PHY_ID), + .name = "Qualcomm QCA8084", + .flags = PHY_POLL_CABLE_TEST, + .probe = at803x_probe, + .config_intr = at803x_config_intr, + .handle_interrupt = at803x_handle_interrupt, + .get_tunable = at803x_get_tunable, + .set_tunable = at803x_set_tunable, + .set_wol = at803x_set_wol, + .get_wol = at803x_get_wol, + .get_features = qca808x_get_features, + .config_aneg = at803x_config_aneg, + .suspend = genphy_suspend, + .resume = genphy_resume, + .read_status = qca808x_read_status, + .soft_reset = qca808x_soft_reset, + .cable_test_start = qca808x_cable_test_start, + .cable_test_get_status = qca808x_cable_test_get_status, }, }; module_phy_driver(at803x_driver); @@ -2242,6 +2289,7 @@ static struct mdio_device_id __maybe_unused atheros_tbl[] = { { PHY_ID_MATCH_EXACT(QCA8327_B_PHY_ID) }, { PHY_ID_MATCH_EXACT(QCA9561_PHY_ID) }, { PHY_ID_MATCH_EXACT(QCA8081_PHY_ID) }, + { PHY_ID_MATCH_MODEL(QCA8084_PHY_ID) }, { } };
Add qca8084 PHY support, which is four-port PHY with maximum link capability 2.5G, the features of each port is almost same as QCA8081 and slave seed config is not needed. Three kind of interface modes supported by qca8084. PHY_INTERFACE_MODE_QUSGMII, PHY_INTERFACE_MODE_2500BASEX and PHY_INTERFACE_MODE_SGMII. The PCS(serdes) and clock are also needed to be configured to bringup qca8084 PHY, which will be added in the pcs driver. The additional CDT configurations used for qca8084. Signed-off-by: Luo Jie <quic_luoj@quicinc.com> --- drivers/net/phy/at803x.c | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)