| Message ID | 20201016232006.3352947-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ixgbe: fix probing of multi-port devices with one MDIO | expand |
On Sat, Oct 17, 2020 at 1:20 AM Jakub Kicinski <kuba@kernel.org> wrote: > > Ian reports that after upgrade from v5.8.14 to v5.9 only one > of his 4 ixgbe netdevs appear in the system. > > Quoting the comment on ixgbe_x550em_a_has_mii(): > * Returns true if hw points to lowest numbered PCI B:D.F x550_em_a device in > * the SoC. There are up to 4 MACs sharing a single MDIO bus on the x550em_a, > * but we only want to register one MDIO bus. > > This matches the symptoms, since the return value from > ixgbe_mii_bus_init() is no longer ignored we need to handle > the higher ports of x550em without an error. Nice, that fixes it! You can add a: Tested-by: Ian Kumlien <ian.kumlien@gmail.com> ;) > Fixes: 09ef193fef7e ("net: ethernet: ixgbe: check the return value of ixgbe_mii_bus_init()") > Reported-by: Ian Kumlien <ian.kumlien@gmail.com> > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 23 ++++++++++++-------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c > index f77fa3e4fdd1..fc389eecdd2b 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c > @@ -901,15 +901,13 @@ static bool ixgbe_x550em_a_has_mii(struct ixgbe_hw *hw) > **/ > s32 ixgbe_mii_bus_init(struct ixgbe_hw *hw) > { > + s32 (*write)(struct mii_bus *bus, int addr, int regnum, u16 val); > + s32 (*read)(struct mii_bus *bus, int addr, int regnum); > struct ixgbe_adapter *adapter = hw->back; > struct pci_dev *pdev = adapter->pdev; > struct device *dev = &adapter->netdev->dev; > struct mii_bus *bus; > > - bus = devm_mdiobus_alloc(dev); > - if (!bus) > - return -ENOMEM; > - > switch (hw->device_id) { > /* C3000 SoCs */ > case IXGBE_DEV_ID_X550EM_A_KR: > @@ -922,16 +920,23 @@ s32 ixgbe_mii_bus_init(struct ixgbe_hw *hw) > case IXGBE_DEV_ID_X550EM_A_1G_T: > case IXGBE_DEV_ID_X550EM_A_1G_T_L: > if (!ixgbe_x550em_a_has_mii(hw)) > - return -ENODEV; > - bus->read = &ixgbe_x550em_a_mii_bus_read; > - bus->write = &ixgbe_x550em_a_mii_bus_write; > + return 0; > + read = &ixgbe_x550em_a_mii_bus_read; > + write = &ixgbe_x550em_a_mii_bus_write; > break; > default: > - bus->read = &ixgbe_mii_bus_read; > - bus->write = &ixgbe_mii_bus_write; > + read = &ixgbe_mii_bus_read; > + write = &ixgbe_mii_bus_write; > break; > } > > + bus = devm_mdiobus_alloc(dev); > + if (!bus) > + return -ENOMEM; > + > + bus->read = read; > + bus->write = write; > + > /* Use the position of the device in the PCI hierarchy as the id */ > snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%s", ixgbe_driver_name, > pci_name(pdev)); > -- > 2.26.2 >
On Sat, 17 Oct 2020 01:50:59 +0200 Ian Kumlien wrote: > On Sat, Oct 17, 2020 at 1:20 AM Jakub Kicinski <kuba@kernel.org> wrote: > > Ian reports that after upgrade from v5.8.14 to v5.9 only one > > of his 4 ixgbe netdevs appear in the system. > > > > Quoting the comment on ixgbe_x550em_a_has_mii(): > > * Returns true if hw points to lowest numbered PCI B:D.F x550_em_a device in > > * the SoC. There are up to 4 MACs sharing a single MDIO bus on the x550em_a, > > * but we only want to register one MDIO bus. > > > > This matches the symptoms, since the return value from > > ixgbe_mii_bus_init() is no longer ignored we need to handle > > the higher ports of x550em without an error. > > Nice, that fixes it! > > You can add a: > Tested-by: Ian Kumlien <ian.kumlien@gmail.com> Will do, thanks! Tony, should I apply directly to net?
Jakub Kicinski wrote: > On Sat, 17 Oct 2020 01:50:59 +0200 Ian Kumlien wrote: > > On Sat, Oct 17, 2020 at 1:20 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > Ian reports that after upgrade from v5.8.14 to v5.9 only one > > > of his 4 ixgbe netdevs appear in the system. > > > > > > Quoting the comment on ixgbe_x550em_a_has_mii(): > > > * Returns true if hw points to lowest numbered PCI B:D.F x550_em_a device in > > > * the SoC. There are up to 4 MACs sharing a single MDIO bus on the x550em_a, > > > * but we only want to register one MDIO bus. > > > > > > This matches the symptoms, since the return value from > > > ixgbe_mii_bus_init() is no longer ignored we need to handle > > > the higher ports of x550em without an error. > > > > Nice, that fixes it! > > > > You can add a: > > Tested-by: Ian Kumlien <ian.kumlien@gmail.com> > > Will do, thanks! > > Tony, should I apply directly to net? Thank you Kuba! Seems like a pretty straight forward bug-fix. I recommend you apply it directly. Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
On Fri, 16 Oct 2020 17:48:07 -0700 Jesse Brandeburg wrote: > Jakub Kicinski wrote: > > > On Sat, 17 Oct 2020 01:50:59 +0200 Ian Kumlien wrote: > > > On Sat, Oct 17, 2020 at 1:20 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > > Ian reports that after upgrade from v5.8.14 to v5.9 only one > > > > of his 4 ixgbe netdevs appear in the system. > > > > > > > > Quoting the comment on ixgbe_x550em_a_has_mii(): > > > > * Returns true if hw points to lowest numbered PCI B:D.F x550_em_a device in > > > > * the SoC. There are up to 4 MACs sharing a single MDIO bus on the x550em_a, > > > > * but we only want to register one MDIO bus. > > > > > > > > This matches the symptoms, since the return value from > > > > ixgbe_mii_bus_init() is no longer ignored we need to handle > > > > the higher ports of x550em without an error. > > > > > > Nice, that fixes it! > > > > > > You can add a: > > > Tested-by: Ian Kumlien <ian.kumlien@gmail.com> > > > > Will do, thanks! > > > > Tony, should I apply directly to net? > > Thank you Kuba! > > Seems like a pretty straight forward bug-fix. I recommend > you apply it directly. Done. > Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com> Thank you!
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c index f77fa3e4fdd1..fc389eecdd2b 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c @@ -901,15 +901,13 @@ static bool ixgbe_x550em_a_has_mii(struct ixgbe_hw *hw) **/ s32 ixgbe_mii_bus_init(struct ixgbe_hw *hw) { + s32 (*write)(struct mii_bus *bus, int addr, int regnum, u16 val); + s32 (*read)(struct mii_bus *bus, int addr, int regnum); struct ixgbe_adapter *adapter = hw->back; struct pci_dev *pdev = adapter->pdev; struct device *dev = &adapter->netdev->dev; struct mii_bus *bus; - bus = devm_mdiobus_alloc(dev); - if (!bus) - return -ENOMEM; - switch (hw->device_id) { /* C3000 SoCs */ case IXGBE_DEV_ID_X550EM_A_KR: @@ -922,16 +920,23 @@ s32 ixgbe_mii_bus_init(struct ixgbe_hw *hw) case IXGBE_DEV_ID_X550EM_A_1G_T: case IXGBE_DEV_ID_X550EM_A_1G_T_L: if (!ixgbe_x550em_a_has_mii(hw)) - return -ENODEV; - bus->read = &ixgbe_x550em_a_mii_bus_read; - bus->write = &ixgbe_x550em_a_mii_bus_write; + return 0; + read = &ixgbe_x550em_a_mii_bus_read; + write = &ixgbe_x550em_a_mii_bus_write; break; default: - bus->read = &ixgbe_mii_bus_read; - bus->write = &ixgbe_mii_bus_write; + read = &ixgbe_mii_bus_read; + write = &ixgbe_mii_bus_write; break; } + bus = devm_mdiobus_alloc(dev); + if (!bus) + return -ENOMEM; + + bus->read = read; + bus->write = write; + /* Use the position of the device in the PCI hierarchy as the id */ snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%s", ixgbe_driver_name, pci_name(pdev));
Ian reports that after upgrade from v5.8.14 to v5.9 only one of his 4 ixgbe netdevs appear in the system. Quoting the comment on ixgbe_x550em_a_has_mii(): * Returns true if hw points to lowest numbered PCI B:D.F x550_em_a device in * the SoC. There are up to 4 MACs sharing a single MDIO bus on the x550em_a, * but we only want to register one MDIO bus. This matches the symptoms, since the return value from ixgbe_mii_bus_init() is no longer ignored we need to handle the higher ports of x550em without an error. Fixes: 09ef193fef7e ("net: ethernet: ixgbe: check the return value of ixgbe_mii_bus_init()") Reported-by: Ian Kumlien <ian.kumlien@gmail.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 23 ++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-)