Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220705132032.702972-1-yangyingliang@huawei.com> <20220705132032.702972-2-yangyingliang@huawei.com>
In-Reply-To: <20220705132032.702972-2-yangyingliang@huawei.com>
From:   Max Filippov <jcmvbkbc@gmail.com>
Date:   Tue, 5 Jul 2022 08:59:43 -0700
Message-ID: <CAMo8BfJvXaDnsXrA1ELKJz1QZmW+8NcQpkhkjKbkp1zxMTePCg@mail.gmail.com>
Subject: Re: [PATCH -next 2/2] xtensa: iss: change the return type of
 iss_net_configure() to void
To:     Yang Yingliang <yangyingliang@huawei.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        "open list:TENSILICA XTENSA PORT (xtensa)" 
        <linux-xtensa@linux-xtensa.org>, Chris Zankel <chris@zankel.net>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Yang,

On Tue, Jul 5, 2022 at 6:10 AM Yang Yingliang <yangyingliang@huawei.com> wrote:
>
> Change the return type of iss_net_configure() to void, because it's not used.
>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  arch/xtensa/platforms/iss/network.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)

This change removes a lot of code that was added in the previous change.
Maybe fold both patches into one to avoid that pattern of adding and then
immediately removing code? Or, if you feel that changing the return type
of the iss_net_configure is an important separate step, maybe do it first?

> diff --git a/arch/xtensa/platforms/iss/network.c b/arch/xtensa/platforms/iss/network.c
> index 5e475f7472e4..e38ff51ce833 100644
> --- a/arch/xtensa/platforms/iss/network.c
> +++ b/arch/xtensa/platforms/iss/network.c
> @@ -472,16 +472,15 @@ static const struct net_device_ops iss_netdev_ops = {
>         .ndo_set_rx_mode        = iss_net_set_multicast_list,
>  };
>
> -static int iss_net_configure(int index, char *init)
> +static void iss_net_configure(int index, char *init)
>  {
>         struct net_device *dev;
>         struct iss_net_private *lp;
> -       int err;
>
>         dev = alloc_etherdev(sizeof(*lp));
>         if (dev == NULL) {
>                 pr_err("eth_configure: failed to allocate device\n");
> -               return -ENOMEM;
> +               return;
>         }
>
>         /* Initialize private element. */
> @@ -509,7 +508,6 @@ static int iss_net_configure(int index, char *init)
>         if (!tuntap_probe(lp, index, init)) {
>                 pr_err("%s: invalid arguments. Skipping device!\n",
>                        dev->name);
> -               err = -EINVAL;
>                 goto err_free_netdev;
>         }
>
> @@ -518,8 +516,7 @@ static int iss_net_configure(int index, char *init)
>         /* sysfs register */
>
>         if (!driver_registered) {
> -               err = platform_driver_register(&iss_net_driver);
> -               if (err)
> +               if (platform_driver_register(&iss_net_driver))
>                         goto err_free_netdev;
>                 driver_registered = 1;
>         }
> @@ -530,8 +527,7 @@ static int iss_net_configure(int index, char *init)
>
>         lp->pdev.id = index;
>         lp->pdev.name = DRIVER_NAME;
> -       err = platform_device_register(&lp->pdev);
> -       if (err)
> +       if (platform_device_register(&lp->pdev))
>                 goto err_free_netdev;
>         SET_NETDEV_DEV(dev, &lp->pdev.dev);
>
> @@ -541,23 +537,22 @@ static int iss_net_configure(int index, char *init)
>         dev->irq = -1;
>
>         rtnl_lock();
> -       err = register_netdevice(dev);
> -       rtnl_unlock();
> -
> -       if (err) {
> +       if (register_netdevice(dev)) {
> +               rtnl_unlock();
>                 pr_err("%s: error registering net device!\n", dev->name);
>                 goto err_unregister_device;
>         }
> +       rtnl_unlock();
>
>         timer_setup(&lp->tl, iss_net_user_timer_expire, 0);
>
> -       return 0;
> +       return;
>
>  err_unregister_device:
>         platform_device_unregister(&lp->pdev);
>  err_free_netdev:
>         free_netdev(dev);
> -       return err;
> +       return;

No need for 'return' at the end of the void function.

-- 
Thanks.
-- Max