[PATCH v2] fsi/core: Fix error paths on CFAM init
qianlihu
wangzhiqiang8906 at gmail.com
Fri Jun 28 19:11:55 AEST 2019
Tested-by: John Wang <wangzqbj at inspur.com>
On Fri, Jun 28, 2019 at 4:09 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>
> ---
> v2:
> fix dropped semicolon
> ---
> drivers/fsi/fsi-core.c | 32 ++++++++++++++++++++------------
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 1d83f3ba478b..1f76740f33b6 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,10 +1072,13 @@ 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);
> +err_free_ida:
> + fsi_free_minor(slave->dev.devt);
> +err_free:
> + of_node_put(slave->dev.of_node);
> + kfree(slave);
> return rc;
> }
>
> --
> 2.20.1
>
More information about the openbmc
mailing list