| Message ID | 20240605232608.65471-1-fujita.tomonori@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | add ethernet driver for Tehuti Networks TN40xx chips | expand |
On 06.06.2024 01.26, FUJITA Tomonori wrote: > This patchset adds a new 10G ethernet driver for Tehuti Networks > TN40xx chips. Note in mainline, there is a driver for Tehuti Networks > (drivers/net/ethernet/tehuti/tehuti.[hc]), which supports TN30xx > chips. > > Multiple vendors (DLink, Asus, Edimax, QNAP, etc) developed adapters > based on TN40xx chips. Tehuti Networks went out of business but the > drivers are still distributed under GPL2 with some of the hardware > (and also available on some sites). With some changes, I try to > upstream this driver with a new PHY driver in Rust. > > The major change is replacing the PHY abstraction layer in the original > driver with phylink. TN40xx chips are used with various PHY hardware > (AMCC QT2025, TI TLK10232, Aqrate AQR105, and Marvell MV88X3120, > MV88X3310, and MV88E2010). > > I've also been working on a new PHY driver for QT2025 in Rust [1]. For > now, I enable only adapters using QT2025 PHY in the PCI ID table of > this driver. I've tested this driver and the QT2025 PHY driver with > Edimax EN-9320 10G adapter and 10G-SR SFP+. In mainline, there are PHY > drivers for AQR105 and Marvell PHYs, which could work for some TN40xx > adapters with this driver. > > To make reviewing easier, this patchset has only basic functions. Once > merged, I'll submit features like ethtool support. > > v9: > - move phylink_connect_phy() to simplify the ndo_open callback > v8: https://lore.kernel.org/netdev/20240603064955.58327-1-fujita.tomonori@gmail.com/ > - remove phylink_mac_change() call > - fix phylink_start() usage (call it after the driver is ready to operate). > - simplify the way to get the private struct from phylink_config pointer > - fix netif_stop_queue usage in mac_link_down callback > - remove MLO_AN_PHY usage > v7: https://lore.kernel.org/netdev/20240527203928.38206-7-fujita.tomonori@gmail.com/ > - use page pool API for rx allocation > - fix NAPI API misuse > - fix error checking of mdio write > v6: https://lore.kernel.org/netdev/20240512085611.79747-2-fujita.tomonori@gmail.com/ > - use the firmware for TN30xx chips > - move link up/down code to phylink's mac_link_up/mac_link_down callbacks > - clean up mdio access code > v5: https://lore.kernel.org/netdev/20240508113947.68530-1-fujita.tomonori@gmail.com/ > - remove dma_set_mask_and_coherent fallback > - count tx_dropped > - use ndo_get_stats64 instead of ndo_get_stats > - remove unnecessary __packed attribute > - fix NAPI API usage > - rename tn40_recycle_skb to tn40_recycle_rx_buffer > - avoid high order page allocation (the maximum is order-1 now) > v4: https://lore.kernel.org/netdev/20240501230552.53185-1-fujita.tomonori@gmail.com/ > - fix warning on 32bit build > - fix inline warnings > - fix header file inclusion > - fix TN40_NDEV_TXQ_LEN > - remove 'select PHYLIB' in Kconfig > - fix access to phydev > - clean up readx_poll_timeout_atomic usage > v3: https://lore.kernel.org/netdev/20240429043827.44407-1-fujita.tomonori@gmail.com/ > - remove driver version > - use prefixes tn40_/TN40_ for all function, struct and define names > v2: https://lore.kernel.org/netdev/20240425010354.32605-1-fujita.tomonori@gmail.com/ > - split mdio patch into mdio and phy support > - add phylink support > - clean up mdio read/write > - use the standard bit operation macros > - use upper_32/lower_32_bits macro > - use tn40_ prefix instead of bdx_ > - fix Sparse errors > - fix compiler warnings > - fix style issues > v1: https://lore.kernel.org/netdev/20240415104352.4685-1-fujita.tomonori@gmail.com/ > > [1] https://lore.kernel.org/netdev/20240415104701.4772-1-fujita.tomonori@gmail.com/ > > FUJITA Tomonori (6): > net: tn40xx: add pci driver for Tehuti Networks TN40xx chips > net: tn40xx: add register defines > net: tn40xx: add basic Tx handling > net: tn40xx: add basic Rx handling > net: tn40xx: add mdio bus support > net: tn40xx: add phylink support > > MAINTAINERS | 8 +- > drivers/net/ethernet/tehuti/Kconfig | 15 + > drivers/net/ethernet/tehuti/Makefile | 3 + > drivers/net/ethernet/tehuti/tn40.c | 1771 +++++++++++++++++++++++ > drivers/net/ethernet/tehuti/tn40.h | 233 +++ > drivers/net/ethernet/tehuti/tn40_mdio.c | 143 ++ > drivers/net/ethernet/tehuti/tn40_phy.c | 76 + > drivers/net/ethernet/tehuti/tn40_regs.h | 245 ++++ > 8 files changed, 2493 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/ethernet/tehuti/tn40.c > create mode 100644 drivers/net/ethernet/tehuti/tn40.h > create mode 100644 drivers/net/ethernet/tehuti/tn40_mdio.c > create mode 100644 drivers/net/ethernet/tehuti/tn40_phy.c > create mode 100644 drivers/net/ethernet/tehuti/tn40_regs.h > > > base-commit: c790275b5edf5d8280ae520bda7c1f37da460c00 Hi Tomonori, feel free to add my Reviewed-by: Hans-Frieder Vogt <hfdevel@gmx.net> to your patch series. I have also tested your driver, however since I have 10GBASE-T cards with x3310 and aqr105 phys I had to add a few lines (very few!) to make them work. Therefore, formally I cannot claim to have tested exactly your patches. Once your driver is out, I will post patches for supporting the other phys. Thanks for taking the effort of mainlining this driver! Best regards, Hans
On Sun, Jun 09, 2024 at 11:10:54AM +0200, Hans-Frieder Vogt wrote: > I have also tested your driver, however since I have 10GBASE-T cards > with x3310 and aqr105 phys I had to add a few lines (very few!) to make > them work. Therefore, formally I cannot claim to have tested exactly > your patches. > Once your driver is out, I will post patches for supporting the other phys. > Thanks for taking the effort of mainlining this driver! Still, it would be useful to know what those changes were, even if they aren't formal patches. Thanks.
Hi, On Sun, 9 Jun 2024 11:10:54 +0200 Hans-Frieder Vogt <hfdevel@gmx.net> wrote: >> FUJITA Tomonori (6): >> net: tn40xx: add pci driver for Tehuti Networks TN40xx chips >> net: tn40xx: add register defines >> net: tn40xx: add basic Tx handling >> net: tn40xx: add basic Rx handling >> net: tn40xx: add mdio bus support >> net: tn40xx: add phylink support >> >> MAINTAINERS | 8 +- >> drivers/net/ethernet/tehuti/Kconfig | 15 + >> drivers/net/ethernet/tehuti/Makefile | 3 + >> drivers/net/ethernet/tehuti/tn40.c | 1771 +++++++++++++++++++++++ >> drivers/net/ethernet/tehuti/tn40.h | 233 +++ >> drivers/net/ethernet/tehuti/tn40_mdio.c | 143 ++ >> drivers/net/ethernet/tehuti/tn40_phy.c | 76 + >> drivers/net/ethernet/tehuti/tn40_regs.h | 245 ++++ >> 8 files changed, 2493 insertions(+), 1 deletion(-) >> create mode 100644 drivers/net/ethernet/tehuti/tn40.c >> create mode 100644 drivers/net/ethernet/tehuti/tn40.h >> create mode 100644 drivers/net/ethernet/tehuti/tn40_mdio.c >> create mode 100644 drivers/net/ethernet/tehuti/tn40_phy.c >> create mode 100644 drivers/net/ethernet/tehuti/tn40_regs.h >> >> >> base-commit: c790275b5edf5d8280ae520bda7c1f37da460c00 > > Hi Tomonori, > > feel free to add my > > Reviewed-by: Hans-Frieder Vogt <hfdevel@gmx.net> Thanks a lot! > to your patch series. > I have also tested your driver, however since I have 10GBASE-T cards > with x3310 and aqr105 phys I had to add a few lines (very few!) to > make > them work. Therefore, formally I cannot claim to have tested exactly > your patches. With few changes, the driver works in-tree Aquantia PHYs driver (drivers/net/phy/aquantia)? > Once your driver is out, I will post patches for supporting the other > phys. > Thanks for taking the effort of mainlining this driver! Great!
On 09.06.2024 12.42, Russell King (Oracle) wrote: > On Sun, Jun 09, 2024 at 11:10:54AM +0200, Hans-Frieder Vogt wrote: >> I have also tested your driver, however since I have 10GBASE-T cards >> with x3310 and aqr105 phys I had to add a few lines (very few!) to make >> them work. Therefore, formally I cannot claim to have tested exactly >> your patches. >> Once your driver is out, I will post patches for supporting the other phys. >> Thanks for taking the effort of mainlining this driver! > Still, it would be useful to know what those changes were, even if they > aren't formal patches. > > Thanks. > Sure. So, here, for info (and later addition to the driver), small adjustments needed in the tn40 driver to support cards with Marvell 88X3310 and Aquantia/Marvell AQR105: First the trivial change (I just also added also the e2010 phy, because it seems to have a lot of commonality with the x3310, but I don't have a card with this phy, so it is atm just speculation. Same applies to the Promise card): in tn40.c: @@ -1752,6 +1754,10 @@ static const struct pci_device_id tn40_i PCI_VENDOR_ID_ASUSTEK, 0x8709) }, { PCI_DEVICE_SUB(PCI_VENDOR_ID_TEHUTI, 0x4022, PCI_VENDOR_ID_EDIMAX, 0x8103) }, + { PCI_VDEVICE(TEHUTI, 0x4025), 0 }, /* AQR105 */ + { PCI_VDEVICE(TEHUTI, 0x4027), 0 }, /* MV88X3310 */ + { PCI_VDEVICE(TEHUTI, 0x4527), 0 }, /* MV88E2010 */ + { PCI_VDEVICE(PROMISE, 0x7203), 0 }, /* AQR105 */ { } }; then the AQR105 seems to need the MDIO bus set at 1MHz to allow for detection: @@ -1681,6 +1681,8 @@ static int tn40_probe(struct pci_dev *pd goto err_unset_drvdata; } + tn40_mdio_set_speed(priv, TN40_MDIO_SPEED_1MHZ); + ret = tn40_mdiobus_init(priv); if (ret) { dev_err(&pdev->dev, "failed to initialize mdio bus.\n"); This is not needed for the x3310 and may be related to, that the AQR105 is connected to phy address 1 on my card. Strange enough the manufacturer's offline driver seems not to need this (but there the sequence of initialization is also different, so it is likely due to this reason). Note: having set the speed to 1MHz here does NOT allow to skip the similar call in tn40_priv_init, because BIT(3) seems to be lost on the way, but is needed for the AQR phy to finish probing, but not for the x3310 (don't really understand this). On the other hand, the x3310 requires to add XAUI to host_interfaces. Otherwise it will not switch to this interface on its own. So, adding this to tn40_phy.c. --- a/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 06:43:40.865474664 +0200 +++ b/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 18:57:01.978776712 +0200 @@ -54,6 +54,8 @@ int tn40_phy_register(struct tn40_priv * return -1; } + __set_bit(PHY_INTERFACE_MODE_XAUI, phydev->host_interfaces); + config = &priv->phylink_config; config->dev = &priv->ndev->dev; config->type = PHYLINK_NETDEV; I have to mention, though, that the phy-drivers for x3310 and aqr105 are not yet ready and will also need some changes related to the firmware loading, because for most (all?) of the Tehuti-based cards the phy firmware has to be uploaded via MDIO. For AQR phys there is already support for firmware loading in the driver. However, this is so far only working for systems using device-tree. A few rather trivial changes (sorry, I know I should have posted them already) will sort this. Further to this I have "switched-on" some features coming from AQR107. I need to investigate whether they are essential for operation. For x3310 the patch provided by Tobias Waldekranz (or a similar patch posted some year or two ago) will be needed for firmware-uploading. I'll prepare a patch for the AQR105. Then we have at least one BASE-T card fully supported.
On Sun, Jun 09, 2024 at 02:40:03PM +0200, Hans-Frieder Vogt wrote: > --- a/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 > 06:43:40.865474664 +0200 > +++ b/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 > 18:57:01.978776712 +0200 > @@ -54,6 +54,8 @@ int tn40_phy_register(struct tn40_priv * > return -1; > } > > + __set_bit(PHY_INTERFACE_MODE_XAUI, phydev->host_interfaces); > + > config = &priv->phylink_config; > config->dev = &priv->ndev->dev; > config->type = PHYLINK_NETDEV; This shouldn't be done - host_interfaces is really only for SFPs, and it suggests that the 88x3310 isn't properly configured with pinstrapping for the correct MAC type (which determines the interface mode to be used to communicate with the MAC.) I'm not sure what to suggest here, other than further debug (e.g. what interface mode is the 88x3310 trying to use without this?) > I have to mention, though, that the phy-drivers for x3310 and aqr105 are > not yet ready and will also need some changes related to the firmware > loading, because for most (all?) of the Tehuti-based cards the phy > firmware has to be uploaded via MDIO. That's problematical - as I understand it, the 88x3310 firmware at least is not freely redistributable (we've run into this with other platforms that do not program the SPI flash attached to the 88x3310 and been told my Marvell that the firmware can't be made available as part of e.g. linux-firmware.) So quite what we do with the 88x3310 based boards that don't have firmware, I'm not sure, but it seems given the non-distributable firmware issue, that's going to be hard.
On 09.06.2024 15.48, Russell King (Oracle) wrote: > On Sun, Jun 09, 2024 at 02:40:03PM +0200, Hans-Frieder Vogt wrote: >> --- a/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 >> 06:43:40.865474664 +0200 >> +++ b/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 >> 18:57:01.978776712 +0200 >> @@ -54,6 +54,8 @@ int tn40_phy_register(struct tn40_priv * >> return -1; >> } >> >> + __set_bit(PHY_INTERFACE_MODE_XAUI, phydev->host_interfaces); >> + >> config = &priv->phylink_config; >> config->dev = &priv->ndev->dev; >> config->type = PHYLINK_NETDEV; > This shouldn't be done - host_interfaces is really only for SFPs, and > it suggests that the 88x3310 isn't properly configured with pinstrapping > for the correct MAC type (which determines the interface mode to be used > to communicate with the MAC.) I already wondered why host_interfaces was only used in the 88x3310, but not in the aqr105 phy driver. Makes sense because the 88x3310 supports both directly BASE-T and an SFP+ cage. > > I'm not sure what to suggest here, other than further debug (e.g. what > interface mode is the 88x3310 trying to use without this?) The message is: tn40xx 0000:10:00.0 enp16s0: PHY has no common interfaces tn40xx 0000:10:00.0 enp16s0: validation of xaui with support 00,00000000,00018000,0000706f and advertisement 00,00000000,00018000,0000706f failed: -22 You are probably right that the interfaceis not properly pinstrapped. bits 2:0 in 1f.f001 are initially 0, which means RXAUI, and the vendor driver just forces the bits to 1 (XAUI with rate matching). Maybe it is a quirk (or a design flaw of the Tehuti reference card). Just a thought: if it cannot be autodetected then maybe the simplest solution is adding a module parameter. >> I have to mention, though, that the phy-drivers for x3310 and aqr105 are >> not yet ready and will also need some changes related to the firmware >> loading, because for most (all?) of the Tehuti-based cards the phy >> firmware has to be uploaded via MDIO. > That's problematical - as I understand it, the 88x3310 firmware at least > is not freely redistributable (we've run into this with other > platforms that do not program the SPI flash attached to the 88x3310 > and been told my Marvell that the firmware can't be made available as > part of e.g. linux-firmware.) > > So quite what we do with the 88x3310 based boards that don't have > firmware, I'm not sure, but it seems given the non-distributable > firmware issue, that's going to be hard. > I followed this discussion, but was hoping that the situation would change. I think I will give the card with AQR105 phy priority, assuming that the firmware topic is easier to solve for the Aquantia line of Marvell products. Thanks, Russel! BR, Hans
On Sun, Jun 09, 2024 at 08:36:42PM +0200, Hans-Frieder Vogt wrote: > On 09.06.2024 15.48, Russell King (Oracle) wrote: > > > On Sun, Jun 09, 2024 at 02:40:03PM +0200, Hans-Frieder Vogt wrote: > > > --- a/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 > > > 06:43:40.865474664 +0200 > > > +++ b/drivers/net/ethernet/tehuti/tn40_phy.c 2024-06-06 > > > 18:57:01.978776712 +0200 > > > @@ -54,6 +54,8 @@ int tn40_phy_register(struct tn40_priv * > > > return -1; > > > } > > > > > > + __set_bit(PHY_INTERFACE_MODE_XAUI, phydev->host_interfaces); > > > + > > > config = &priv->phylink_config; > > > config->dev = &priv->ndev->dev; > > > config->type = PHYLINK_NETDEV; > > This shouldn't be done - host_interfaces is really only for SFPs, and > > it suggests that the 88x3310 isn't properly configured with pinstrapping > > for the correct MAC type (which determines the interface mode to be used > > to communicate with the MAC.) > I already wondered why host_interfaces was used in the 88x3310, but not > in the aqr105 phy driver. Makes sense because the 88x3310 supports both > directly BASE-T and an SFP+ cage. > > I'm not sure what to suggest here, other than further debug (e.g. what > > interface mode is the 88x3310 trying to use without this?) > > The message is: > tn40xx 0000:10:00.0 enp16s0: PHY has no common interfaces So... the 88x3310 supports SGMII, 2500BASE-X, 5GBASE-R, XAUI, RXAUI, 10GBASE-R and USXGMII on its host side interface. When the phy is attached, the config_init method in the PHY driver will be called, and it will fill in phydev->possible_interfaces to reflect the interface modes that the PHY will _actually_ be using. Phylink will notice phydev->possible_interfaces being non-empty and check whether the union of the set of PHY possible_interfaces and the set of MAC supported_interfaces is non-empty. If it's empty, then the above message will be issued. This suggests, as mentioned earlier, that the operating mode of the 88x3310 PHY doesn't match what this MAC can support, which makes me wonder about the pinstrapping options for the 88x3310 on this hardware. My guess is someone found a hardware design error and decided "software can sort this out for us!" > tn40xx 0000:10:00.0 enp16s0: validation of xaui with support > 00,00000000,00018000,0000706f and advertisement > 00,00000000,00018000,0000706f failed: -22 Basically, the PHY isn't operating in XAUI mode. > You are probably right that the interfaceis not properly pinstrapped. > bits 2:0 in 1f.f001 are initially 0, which means RXAUI, and the vendor > driver just forces the bits to 1 (XAUI with rate matching). Maybe it is > a quirk (or a design flaw of the Tehuti reference card). Just a thought: > if it cannot be autodetected then maybe the simplest solution is adding > a module parameter. It may be that the best option is to set phydev->host_interfaces, but I would like to see a comment giving details about why this is necessary - essentially covering the information above please. It's making use of that outside its original purpose, and I would like such uses well documented so if stuff needs to change, we know what and why these drivers are making use of it. > I followed this discussion, but was hoping that the situation would change. > I think I will give the card with AQR105 phy priority, assuming that the > firmware topic is easier to solve for the Aquantia line of Marvell products. Fingers crossed! Thanks.
> then the AQR105 seems to need the MDIO bus set at 1MHz to allow for > detection: > > @@ -1681,6 +1681,8 @@ static int tn40_probe(struct pci_dev *pd > goto err_unset_drvdata; > } > > + tn40_mdio_set_speed(priv, TN40_MDIO_SPEED_1MHZ); > + > ret = tn40_mdiobus_init(priv); > if (ret) { > dev_err(&pdev->dev, "failed to initialize mdio bus.\n"); > > This is not needed for the x3310 and may be related to, that the AQR105 > is connected to phy address 1 on my card. 802.3 says the PHY should operate with a clock up to 2.5Mhz, and many PHYs operate at faster speeds than 2.5Mhz. So this is likely a symptom, not a cause. Is the PHY getting reset? Is it being given enough time to get itself together after the reset? Maybe a delay needs to be added. Andrew
On Sun, 9 Jun 2024 11:10:54 +0200 Hans-Frieder Vogt <hfdevel@gmx.net> wrote: >> FUJITA Tomonori (6): >> net: tn40xx: add pci driver for Tehuti Networks TN40xx chips >> net: tn40xx: add register defines >> net: tn40xx: add basic Tx handling >> net: tn40xx: add basic Rx handling >> net: tn40xx: add mdio bus support >> net: tn40xx: add phylink support >> >> MAINTAINERS | 8 +- >> drivers/net/ethernet/tehuti/Kconfig | 15 + >> drivers/net/ethernet/tehuti/Makefile | 3 + >> drivers/net/ethernet/tehuti/tn40.c | 1771 +++++++++++++++++++++++ >> drivers/net/ethernet/tehuti/tn40.h | 233 +++ >> drivers/net/ethernet/tehuti/tn40_mdio.c | 143 ++ >> drivers/net/ethernet/tehuti/tn40_phy.c | 76 + >> drivers/net/ethernet/tehuti/tn40_regs.h | 245 ++++ >> 8 files changed, 2493 insertions(+), 1 deletion(-) >> create mode 100644 drivers/net/ethernet/tehuti/tn40.c >> create mode 100644 drivers/net/ethernet/tehuti/tn40.h >> create mode 100644 drivers/net/ethernet/tehuti/tn40_mdio.c >> create mode 100644 drivers/net/ethernet/tehuti/tn40_phy.c >> create mode 100644 drivers/net/ethernet/tehuti/tn40_regs.h >> >> >> base-commit: c790275b5edf5d8280ae520bda7c1f37da460c00 > > Hi Tomonori, > > feel free to add my > > Reviewed-by: Hans-Frieder Vogt <hfdevel@gmx.net> I added your Reviewed-by except for patches that you've reviewed. Please have look at the remaining patches in v10. I'll add PHY specific data for initialization to the PCI id_table after merged.
On Tue, 11 Jun 2024 13:54:42 +0900 (JST) FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: >> feel free to add my >> >> Reviewed-by: Hans-Frieder Vogt <hfdevel@gmx.net> > > I added your Reviewed-by except for patches that you've > reviewed. Please have look at the remaining patches in v10. Sorry, I meant, except for patches that you gave review comments on.