[PATCH] fsi/core: Fix error paths on CFAM init

[John Wang]

On Fri, Jun 28, 2019 at 2:44 PM Jeremy Kerr <jk at ozlabs.org> wrote:
>
> Change d1dcd67825 re-worked the struct fsi_slave initialisation in
> fsi_slave_init, but introduced a few inconsitencies: the slave->dev is
> now registered through cdev_device_add, but we may kfree() the device
> out from underneath the cdev registration. We may also leave an IDA
> allocated.
>
> This change fixes the error paths, so that we kfree() only before the
> device is registered with the core code. We also move the smode write to
> before we start creating proper devices, as it's the most likely to
> fail. We also remove the IDA-allocated minor on error, and properly
> clean up the of_node.
>
> Fixes: d1dcd678257603e71cf3f3d84c70e2b6f0f14bb8
> Reported-by: Lei YU <mine260309 at gmail.com>
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
> ---
>
> Lei: Can you test this on your hardware? Thanks!
>
> ---
>  drivers/fsi/fsi-core.c | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 1d83f3ba478b..9ba5e19d1c42 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -1029,6 +1029,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
>
>         }
>
> +       rc = fsi_slave_set_smode(slave);
> +       if (rc) {
> +               dev_warn(&master->dev,
> +                               "can't set smode on slave:%02x:%02x %d\n",
> +                               link, id, rc);
> +               goto err_free;
> +       }
> +
>         /* Allocate a minor in the FSI space */
>         rc = __fsi_get_new_minor(slave, fsi_dev_cfam, &slave->dev.devt,
>                                  &slave->cdev_idx);
> @@ -1040,17 +1048,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
>         rc = cdev_device_add(&slave->cdev, &slave->dev);
>         if (rc) {
>                 dev_err(&slave->dev, "Error %d creating slave device\n", rc);
> -               goto err_free;
> +               goto err_free_ida;
>         }
>
> -       rc = fsi_slave_set_smode(slave);
> -       if (rc) {
> -               dev_warn(&master->dev,
> -                               "can't set smode on slave:%02x:%02x %d\n",
> -                               link, id, rc);
> -               kfree(slave);
> -               return -ENODEV;
> -       }
> +       /* Now that we have the cdev registered with the core, any fatal
> +        * failures beyond this point will need to clean up through
> +        * cdev_device_del(). Fortunately though, nothing past here is fatal.
> +        */
> +
>         if (master->link_config)
>                 master->link_config(master, link,
>                                     slave->t_send_delay,
> @@ -1067,11 +1072,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
>                 dev_dbg(&master->dev, "failed during slave scan with: %d\n",
>                                 rc);
>
> -       return rc;
> +       return 0;
>
> - err_free:
> -       put_device(&slave->dev);
> -       return rc;
> +err_free_ida:
> +       fsi_free_minor(slave->dev.devt);
> +err_free:
> +       of_node_put(slave->dev.of_node);
> +       kfree(slave);
> +       return rc
semicolon missed :)
>  }
>
>  /* FSI master support */
> --
> 2.20.1
>