MIME-Version: 1.0
References: <20230307200502.2263655-1-grundler@chromium.org> <20230307200502.2263655-2-grundler@chromium.org>
In-Reply-To: <20230307200502.2263655-2-grundler@chromium.org>
From:   Grant Grundler <grundler@chromium.org>
Date:   Tue, 7 Mar 2023 12:13:33 -0800
Message-ID: <CANEJEGt4RCZX8uPBgGcRPmb=Ws6jHFC8h98phfA4sz-8rdtkkA@mail.gmail.com>
Subject: Re: [PATCHv2 2/2] net: asix: init mdiobus from one function
To:     Grant Grundler <grundler@chromium.org>
Cc:     Oleksij Rempel <linux@rempel-privat.de>,
        Pavel Skripkin <paskripkin@gmail.com>,
        Lukas Wunner <lukas@wunner.de>,
        Eizan Miyamoto <eizan@chromium.org>,
        Jakub Kicinski <kuba@kernel.org>,
        netdev <netdev@vger.kernel.org>,
        "David S . Miller" <davem@davemloft.net>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Mar 7, 2023 at 12:05=E2=80=AFPM Grant Grundler <grundler@chromium.o=
rg> wrote:
>
> Make asix driver consistent with other drivers (e.g. tg3 and r8169) which
> use mdiobus calls: setup and tear down be handled in one function each.
>
> Signed-off-by: Grant Grundler <grundler@chromium.org>
> ---
>  drivers/net/usb/asix_devices.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_device=
s.c
> index 21845b88a64b9..d7caab4493d15 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -637,7 +637,7 @@ static int asix_resume(struct usb_interface *intf)
>         return usbnet_resume(intf);
>  }
>
> -static int ax88772_init_mdio(struct usbnet *dev)
> +static int ax88772_mdio_register(struct usbnet *dev)
>  {
>         struct asix_common_private *priv =3D dev->driver_priv;
>         int ret;
> @@ -657,10 +657,22 @@ static int ax88772_init_mdio(struct usbnet *dev)
>         ret =3D mdiobus_register(priv->mdio);
>         if (ret) {
>                 netdev_err(dev->net, "Could not register MDIO bus (err %d=
)\n", ret);
> -               mdiobus_free(priv->mdio);
> -               priv->mdio =3D NULL;
> +               goto mdio_register_err;
>         }
>
> +       priv->phydev =3D mdiobus_get_phy(priv->mdio, priv->phy_addr);
> +       if (!priv->phydev) {
> +               netdev_err(dev->net, "Could not find PHY\n");
> +               ret=3D-ENODEV;

My apologies: checkpatch wants spaces around "=3D" and especially
because it should be "ret =3D -ENODEV".

I can resend - but is this trivial enough to fix up by maintainer?

cheers,
grant

> +               goto mdio_phy_err;
> +       }
> +
> +       return 0;
> +
> +mdio_phy_err:
> +       mdiobus_unregister(priv->mdio);
> +mdio_register_err:
> +       mdiobus_free(priv->mdio);
>         return ret;
>  }
>
> @@ -675,13 +687,6 @@ static int ax88772_init_phy(struct usbnet *dev)
>         struct asix_common_private *priv =3D dev->driver_priv;
>         int ret;
>
> -       priv->phydev =3D mdiobus_get_phy(priv->mdio, priv->phy_addr);
> -       if (!priv->phydev) {
> -               netdev_err(dev->net, "Could not find PHY\n");
> -               ax88772_mdio_unregister(priv);
> -               return -ENODEV;
> -       }
> -
>         ret =3D phy_connect_direct(dev->net, priv->phydev, &asix_adjust_l=
ink,
>                                  PHY_INTERFACE_MODE_INTERNAL);
>         if (ret) {
> @@ -799,7 +804,7 @@ static int ax88772_bind(struct usbnet *dev, struct us=
b_interface *intf)
>         priv->presvd_phy_bmcr =3D 0;
>         priv->presvd_phy_advertise =3D 0;
>
> -       ret =3D ax88772_init_mdio(dev);
> +       ret =3D ax88772_mdio_register(dev);
>         if (ret)
>                 return ret;
>
> --
> 2.39.2
>