| Message ID | 20190118152352.26417-6-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: phy: Add support for 2.5GBASET PHYs | expand |
On Fri, Jan 18, 2019 at 04:23:50PM +0100, Maxime Chevallier wrote: > As per 802.3bz, if bit 14 of (1.11) "PMA Extended Abilities" indicates > whether or not we should read register (1.21) "2.52/5G PMA Extended > Abilities", which contains information on the support of 2.5GBASET and > 5GBASET. > > After testing on several variants of PHYS of this family, it appears > that bit 14 in (1.11) isn't always set when it should be. > > PHYs 88X3310 (on MacchiatoBin) and 88E2010 do support 2.5G and 5GBASET, > but don't have 1.11.14 set. Their register 1.21 is filled with the > correct values, indicating 2.5G and 5G support. > > PHYs 88X2110 do have their 1.11.14 bit set, as it should. Hi Maxime Is there anything about this in any Errata? We potentially have an issue if Marvell have any PHYs in this family which don't support 2.5G/5G. Maybe this workaround needs to check the IDs and only enable it on device we know are broken. Andrew
Hello Andrew, On Sun, 20 Jan 2019 20:08:09 +0100 Andrew Lunn <andrew@lunn.ch> wrote: >On Fri, Jan 18, 2019 at 04:23:50PM +0100, Maxime Chevallier wrote: >> As per 802.3bz, if bit 14 of (1.11) "PMA Extended Abilities" indicates >> whether or not we should read register (1.21) "2.52/5G PMA Extended >> Abilities", which contains information on the support of 2.5GBASET and >> 5GBASET. >> >> After testing on several variants of PHYS of this family, it appears >> that bit 14 in (1.11) isn't always set when it should be. >> >> PHYs 88X3310 (on MacchiatoBin) and 88E2010 do support 2.5G and 5GBASET, >> but don't have 1.11.14 set. Their register 1.21 is filled with the >> correct values, indicating 2.5G and 5G support. >> >> PHYs 88X2110 do have their 1.11.14 bit set, as it should. > >Hi Maxime > >Is there anything about this in any Errata? I haven't seen any Errata on that unfortunately. I also thought about reading (1.4) "PMA/PMD Speed Ability", but the 2.5G and 5G speeds are also reported as not being supported on the 88X3310. >We potentially have an issue if Marvell have any PHYs in this family >which don't support 2.5G/5G. Maybe this workaround needs to check the >IDs and only enable it on device we know are broken. I agree with you, this might be a better way to handle that issue. For now, I've only seen that issue on the 3310 and 2010, with PHY IDs respectively 002b09aa and 002b09ab. I 'll add a test for ids '002b09aX', hopefully there won't be any PHYs with these IDs that don't support 2.5/5G. In that case, there's no need for a separate mv2110_config_init in patch 7. Thanks, Maxime. > Andrew
On Mon, Jan 21, 2019 at 11:35:31AM +0100, Maxime Chevallier wrote: > Hello Andrew, > > On Sun, 20 Jan 2019 20:08:09 +0100 > Andrew Lunn <andrew@lunn.ch> wrote: > > >On Fri, Jan 18, 2019 at 04:23:50PM +0100, Maxime Chevallier wrote: > >> As per 802.3bz, if bit 14 of (1.11) "PMA Extended Abilities" indicates > >> whether or not we should read register (1.21) "2.52/5G PMA Extended > >> Abilities", which contains information on the support of 2.5GBASET and > >> 5GBASET. > >> > >> After testing on several variants of PHYS of this family, it appears > >> that bit 14 in (1.11) isn't always set when it should be. > >> > >> PHYs 88X3310 (on MacchiatoBin) and 88E2010 do support 2.5G and 5GBASET, > >> but don't have 1.11.14 set. Their register 1.21 is filled with the > >> correct values, indicating 2.5G and 5G support. > >> > >> PHYs 88X2110 do have their 1.11.14 bit set, as it should. > > > >Hi Maxime > > > >Is there anything about this in any Errata? > > I haven't seen any Errata on that unfortunately. > > I also thought about reading (1.4) "PMA/PMD Speed Ability", but the > 2.5G and 5G speeds are also reported as not being supported on the > 88X3310. It's entirely possible that the 3310 switches to different hardware blocks for 2.5G and 5G speeds, and reading _just_ the 1.4 register is not sufficient. The 88x3310 is multiple PHY devices in one package, with a CPU that manages the routing between each individual device.
Hello Russell, On Mon, 21 Jan 2019 10:52:06 +0000 Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: >On Mon, Jan 21, 2019 at 11:35:31AM +0100, Maxime Chevallier wrote: >> Hello Andrew, >> >> On Sun, 20 Jan 2019 20:08:09 +0100 >> Andrew Lunn <andrew@lunn.ch> wrote: >> >> >On Fri, Jan 18, 2019 at 04:23:50PM +0100, Maxime Chevallier wrote: >> >> As per 802.3bz, if bit 14 of (1.11) "PMA Extended Abilities" indicates >> >> whether or not we should read register (1.21) "2.52/5G PMA Extended >> >> Abilities", which contains information on the support of 2.5GBASET and >> >> 5GBASET. >> >> >> >> After testing on several variants of PHYS of this family, it appears >> >> that bit 14 in (1.11) isn't always set when it should be. >> >> >> >> PHYs 88X3310 (on MacchiatoBin) and 88E2010 do support 2.5G and 5GBASET, >> >> but don't have 1.11.14 set. Their register 1.21 is filled with the >> >> correct values, indicating 2.5G and 5G support. >> >> >> >> PHYs 88X2110 do have their 1.11.14 bit set, as it should. >> > >> >Hi Maxime >> > >> >Is there anything about this in any Errata? >> >> I haven't seen any Errata on that unfortunately. >> >> I also thought about reading (1.4) "PMA/PMD Speed Ability", but the >> 2.5G and 5G speeds are also reported as not being supported on the >> 88X3310. > >It's entirely possible that the 3310 switches to different hardware >blocks for 2.5G and 5G speeds, and reading _just_ the 1.4 register >is not sufficient. I agree with you but in that particular case, I think we are reading from the correct device. The datasheet itself says that we should be reading 1.4 and 1.11 as we expect, with 2.5G/5G support being set (these registers are read-only, and the datasheet's values aren't what we actually read). >The 88x3310 is multiple PHY devices in one package, with a CPU that >manages the routing between each individual device. I also just checked register 3.4 "PCS Speed Ability", and 3.8 "PCS Status 2" which also contains informations on 2.5G/5G abilities, but there's the same issue here. Maxime
On Mon, Jan 21, 2019 at 01:29:45PM +0100, Maxime Chevallier wrote: > Hello Russell, > > On Mon, 21 Jan 2019 10:52:06 +0000 > Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > >It's entirely possible that the 3310 switches to different hardware > >blocks for 2.5G and 5G speeds, and reading _just_ the 1.4 register > >is not sufficient. > > I agree with you but in that particular case, I think we are reading > from the correct device. The datasheet itself says that we should be > reading 1.4 and 1.11 as we expect, with 2.5G/5G support being set (these > registers are read-only, and the datasheet's values aren't what we > actually read). No, you missed what I was saying. The 88x3310 is a hybrid device. It contains multiple instances of each individual device at different offsets in each MMD address space. For example, there isn't just one PCS in MMD 3, there are three: offset 0x0000: 10GBASE-T PCS offset 0x1000: 10GBASE-R, 10GBASE-X, 40GBASE-R PCS offset 0x2000: 1000baseX-FD PCS Which get used depends on what is active. For example, on the Macchiatobin, the PCS at 0x1000 gets used for the SFP port, and the PCS at 0x0000 gets used for the copper port. The PCS at 0x2000 doesn't ever seem to become active. MMD 4 (PHYXS) is very similar, and MMD 7 (AN) has four instances. Different AN instances get used for 10GBASE-T, 10GBASE-KR, SGMII etc. Each of these blocks either comes from Marvell or another manufacturer. The 3310 is a mismash of IPs bolted together, with a bunch of routing between them. It is not simply a Clause 45 compliant PHY (it isn't compliant, because C45 requires certain registers to be reserved, but the 3310 has incomplete address decoding - visible as repeats in the address space of each MMD.) The exception seems to be the PMA/PMD MMD which I've only discovered a single instance. > >The 88x3310 is multiple PHY devices in one package, with a CPU that > >manages the routing between each individual device. > > I also just checked register 3.4 "PCS Speed Ability", and 3.8 "PCS > Status 2" which also contains informations on 2.5G/5G abilities, but > there's the same issue here. What I'm saying is, there is much more to this PHY than just the 802.3 register layout because of there being multiple instances. It _may_ be that a different set of instances gets used to the ones at offset 0 for any particular set of speeds. I can't look at this at the moment because I am unable to get access to the 802.3bz specifications - I try to get them through the IEEE get program website, and I can get to the 802.3 Ethernet specs, click on either of the 802.3bz amendment 7 links, and I just get a couple of spinning blobs in Firefox - the page never finishes loading. So, I can't get the register information for NBASE-T and compare it to the various MMD instances in the 88x3310.
Hello Russell, On Mon, 21 Jan 2019 13:00:30 +0000 Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: >On Mon, Jan 21, 2019 at 01:29:45PM +0100, Maxime Chevallier wrote: >> Hello Russell, >> >> On Mon, 21 Jan 2019 10:52:06 +0000 >> Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: >> >It's entirely possible that the 3310 switches to different hardware >> >blocks for 2.5G and 5G speeds, and reading _just_ the 1.4 register >> >is not sufficient. >> >> I agree with you but in that particular case, I think we are reading >> from the correct device. The datasheet itself says that we should be >> reading 1.4 and 1.11 as we expect, with 2.5G/5G support being set (these >> registers are read-only, and the datasheet's values aren't what we >> actually read). > >No, you missed what I was saying. > >The 88x3310 is a hybrid device. It contains multiple instances of >each individual device at different offsets in each MMD address space. Ah I see, I indeed thought you refered to the MMDs. [...] >The exception seems to be the PMA/PMD MMD which I've only discovered >a single instance. Yes there only seems to be one. There are some other registers in the 1.0xCxxx range, but those who are documented don't help a lot with determing wether or not these modes are supported. I wonder if these values are correctly reported in newer PHY firmware revisions. I've checked other PCS instances, but it seems the one at 3.0x0xxx is the one used in 2.5/5GBASET. I've tested with other PHYs from this family, it looks like they are derivatives of the 33x0 design, with the addition/removal of internal IPs. Since the 2110 returns the correct values and has a similar design, with the PMA returning the correct abilities, I think we are reading from the correct instance. Thanks, Maxime
On Mon, Jan 28, 2019 at 03:26:21PM +0100, Maxime Chevallier wrote: > Hello Russell, > > On Mon, 21 Jan 2019 13:00:30 +0000 > Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > > >On Mon, Jan 21, 2019 at 01:29:45PM +0100, Maxime Chevallier wrote: > >> Hello Russell, > >> > >> On Mon, 21 Jan 2019 10:52:06 +0000 > >> Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > >> >It's entirely possible that the 3310 switches to different hardware > >> >blocks for 2.5G and 5G speeds, and reading _just_ the 1.4 register > >> >is not sufficient. > >> > >> I agree with you but in that particular case, I think we are reading > >> from the correct device. The datasheet itself says that we should be > >> reading 1.4 and 1.11 as we expect, with 2.5G/5G support being set (these > >> registers are read-only, and the datasheet's values aren't what we > >> actually read). > > > >No, you missed what I was saying. > > > >The 88x3310 is a hybrid device. It contains multiple instances of > >each individual device at different offsets in each MMD address space. > > Ah I see, I indeed thought you refered to the MMDs. > > [...] > > >The exception seems to be the PMA/PMD MMD which I've only discovered > >a single instance. > > Yes there only seems to be one. There are some other registers in the > 1.0xCxxx range, but those who are documented don't help a lot with > determing wether or not these modes are supported. > > I wonder if these values are correctly reported in newer PHY firmware > revisions. > > I've checked other PCS instances, but it seems the one at 3.0x0xxx is > the one used in 2.5/5GBASET. In the following, I use "subdevice N" to mean instance "N + 1". So the instance at address 0 is the main device and the following instance is subdevice 1. Looking at the PCS, none of them have the 2.5G/5G bits set in the 3310 which is rather interesting, except the one at 3.0 has the EEE bits set in 3.21. You are quite correct that the first instance is used for 2.5G copper, but PCS subdevice 2 also changes as a result of switching down to 2.5G (its transmit fault status clears.) It looks like PhyXS subdevice 3 is used (which is a SGMII/1000base-X MAC facing PHY), is switched to fixed speed mode. It doesn't have what looks like a sensible advertisement either, so my guess is that this will need the MAC side to be configured _not_ to expect the in-band control word in this mode. In other words, it may be SGMII or 1000base-X upclocked to 2.5G speeds - which it is doesn't matter because the difference is in the control word which looks like it's omitted, or if it isn't, doesn't contain useful information. Note that mvpp2 is slightly buggy in this area, and I have a patch set that fixes it up - patches can be found in my "mvpp2" branch, which I'm waiting for my problem reporter to report back after his testing. If you use this patch set, as long as you don't use in-band mode, it should be fine.
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c index f45ddf3bc138..0a35bf0fac47 100644 --- a/drivers/net/phy/marvell10g.c +++ b/drivers/net/phy/marvell10g.c @@ -251,7 +251,7 @@ static int mv3310_resume(struct phy_device *phydev) static int mv3310_config_init(struct phy_device *phydev) { - int ret; + int ret, val; /* Check that the PHY interface type is compatible */ if (phydev->interface != PHY_INTERFACE_MODE_SGMII && @@ -265,6 +265,23 @@ static int mv3310_config_init(struct phy_device *phydev) if (ret) return ret; + /* Some PHYs in the Alaska family such as the 88X3310 and the 88E2010 + * don't set bit 14 in PMA Extended Abilities (1.11), although they do + * support 2.5GBASET and 5GBASET. Their 2.5/5G PMA Extended abilities + * (1.21) still have a meaningful value, so read it anyway to make sure + * we enable support for these modes if needed. + */ + val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE); + if (val < 0) + return val; + + if (val & MDIO_PMA_NG_EXTABLE_2_5GBT) + __set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, + phydev->supported); + if (val & MDIO_PMA_NG_EXTABLE_5GBT) + __set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, + phydev->supported); + /* Make sure we advertise all the supported modes, and not just the * default one specified in the driver's .features. */
As per 802.3bz, if bit 14 of (1.11) "PMA Extended Abilities" indicates whether or not we should read register (1.21) "2.52/5G PMA Extended Abilities", which contains information on the support of 2.5GBASET and 5GBASET. After testing on several variants of PHYS of this family, it appears that bit 14 in (1.11) isn't always set when it should be. PHYs 88X3310 (on MacchiatoBin) and 88E2010 do support 2.5G and 5GBASET, but don't have 1.11.14 set. Their register 1.21 is filled with the correct values, indicating 2.5G and 5G support. PHYs 88X2110 do have their 1.11.14 bit set, as it should. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- drivers/net/phy/marvell10g.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)