Skip to content

Commit

Permalink
fsi/core: Fix error paths on CFAM init
Browse files Browse the repository at this point in the history
Change d1dcd67 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: d1dcd67 ("fsi: Add cfam char devices")
Reported-by: Lei YU <[email protected]>
Tested-by: John Wang <[email protected]>
Signed-off-by: Jeremy Kerr <[email protected]>
Signed-off-by: Joel Stanley <[email protected]>
  • Loading branch information
jk-ozlabs authored and shenki committed Jul 3, 2019
1 parent afd2611 commit 371975b
Showing 1 changed file with 20 additions and 12 deletions.
32 changes: 20 additions & 12 deletions drivers/fsi/fsi-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,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);
Expand All @@ -1048,17 +1056,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,
Expand All @@ -1075,10 +1080,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;
}

Expand Down

0 comments on commit 371975b

Please sign in to comment.