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

[Benjamin Herrenschmidt]

On Fri, 2019-06-28 at 14:41 +0800, Jeremy Kerr 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>
> ---

You want to take maintainership of drivers/fsi ? :-)

> 
> 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
>  }
>  
>  /* FSI master support */