lkml.org 
[lkml]   [2020]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH] Revert "net: ethernet: ixgbe: check the return value of ixgbe_mii_bus_init()"
    Date
    > -----Original Message-----
    > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
    > Sent: Friday, September 25, 2020 16:15
    > To: Liu, Yongxin <Yongxin.Liu@windriver.com>
    > Cc: David S . Miller <davem@davemloft.net>; netdev
    > <netdev@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>
    > Subject: Re: [PATCH] Revert "net: ethernet: ixgbe: check the return value
    > of ixgbe_mii_bus_init()"
    >
    > On Fri, Sep 25, 2020 at 4:45 AM Yongxin Liu <yongxin.liu@windriver.com>
    > wrote:
    > >
    > > This reverts commit 09ef193fef7efb0175a04634853862d717adbb95.
    > >
    > > For C3000 family of SoCs, they have four ixgbe devices sharing a single
    > MDIO bus.
    > > ixgbe_mii_bus_init() returns -ENODEV for other three devices. The
    > > propagation of the error code makes other three ixgbe devices
    > unregistered.
    > >
    > > Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com>
    > > ---
    > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 +-----
    > > 1 file changed, 1 insertion(+), 5 deletions(-)
    > >
    > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
    > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
    > > index 2f8a4cfc5fa1..5e5223becf86 100644
    > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
    > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
    > > @@ -11031,14 +11031,10 @@ static int ixgbe_probe(struct pci_dev *pdev,
    > const struct pci_device_id *ent)
    > > IXGBE_LINK_SPEED_10GB_FULL |
    > IXGBE_LINK_SPEED_1GB_FULL,
    > > true);
    > >
    > > - err = ixgbe_mii_bus_init(hw);
    > > - if (err)
    > > - goto err_netdev;
    > > + ixgbe_mii_bus_init(hw);
    > >
    > > return 0;
    > >
    > > -err_netdev:
    > > - unregister_netdev(netdev);
    > > err_register:
    > > ixgbe_release_hw_control(adapter);
    > > ixgbe_clear_interrupt_scheme(adapter);
    > > --
    > > 2.14.4
    > >
    >
    > Then we should check if err == -ENODEV, not outright ignore all potential
    > errors, right?
    >

    Hm, it is weird to take -ENODEV as a no error.
    How about just return 0 instead of -ENODEV in the following function?

    <drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c>
    902 s32 ixgbe_mii_bus_init(struct ixgbe_hw *hw)
    903 {
    .
    .
    .
    913 switch (hw->device_id) {
    914 /* C3000 SoCs */
    915 case IXGBE_DEV_ID_X550EM_A_KR:
    916 case IXGBE_DEV_ID_X550EM_A_KR_L:
    917 case IXGBE_DEV_ID_X550EM_A_SFP_N:
    918 case IXGBE_DEV_ID_X550EM_A_SGMII:
    919 case IXGBE_DEV_ID_X550EM_A_SGMII_L:
    920 case IXGBE_DEV_ID_X550EM_A_10G_T:
    921 case IXGBE_DEV_ID_X550EM_A_SFP:
    922 case IXGBE_DEV_ID_X550EM_A_1G_T:
    923 case IXGBE_DEV_ID_X550EM_A_1G_T_L:
    924 if (!ixgbe_x550em_a_has_mii(hw))
    925 return -ENODEV;


    Thanks,
    Yongxin


    > Bartosz
    \
     
     \ /
      Last update: 2020-09-25 10:52    [W:3.949 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site