Re: [PATCH 2/2] i2c-dev: Don't block the adapter from unregistering

From: Peter Rosin
Date: Wed Jul 06 2016 - 04:38:42 EST


On 2016-07-06 04:57, Viresh Kumar wrote:
> The i2c-dev calls i2c_get_adapter() from the .open() callback, which
> doesn't let the adapter device unregister unless the .close() callback
> is called.
>
> On some platforms (like Google ARA), this doesn't let the modules
> (hardware attached to the phone) eject from the phone as the cleanup
> path for the module hasn't finished yet (i2c adapter not removed).
>
> We can't let the userspace block the kernel forever in such cases.
>
> Fix this by calling i2c_get_adapter() from all other file operations,
> i.e. read/write/ioctl, to make sure the adapter doesn't get away while
> we are in the middle of a operation, but not otherwise. In .open() we
> will release the adapter device before returning and so if there is no
> data transfer in progress, then the i2c-dev doesn't block the adapter
> from unregistering.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
> drivers/i2c/i2c-dev.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++-----
> include/linux/i2c.h | 1 +
> 2 files changed, 66 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> index 66f323fd3982..b2562603daa9 100644
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -142,13 +142,25 @@ static ssize_t i2cdev_read(struct file *file, char __user *buf, size_t count,
> int ret;
>
> struct i2c_client *client = file->private_data;
> + struct i2c_adapter *adap;
> +
> + adap = i2c_get_adapter(client->adapter_nr);
> + if (!adap)
> + return -ENODEV;
> +
> + if (adap != client->adapter) {
> + ret = -EINVAL;
> + goto put_adapter;
> + }

I don't see how this can work with the i2c-demux-pinctrl driver.
I also wonder if/how other muxes handle this relaxed adapter
lifetime thingy?

Out of curiosity, why would client->adapter change anyway?
(that is, if not because of a demux-pinctrl op)

Or maybe I'm just paranoid, but this is not obvious...

Cheers,
Peter