On 06-07-16, 10:22, Peter Rosin wrote:
> 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?
I would like to mention here that I am no I2C expert and have limited
knowledge of it :)
I haven't had a look at the muxes implementation earlier, now that I
looked at them, I see that they unregister/register the adapter,
perhaps while switching functionality.
I am not sure though, if this patch will break it or not. And I don't
have a way of testing it out.
> Out of curiosity, why would client->adapter change anyway?
> (that is, if not because of a demux-pinctrl op)
I didn't mean that it will change, and perhaps we can add a
WARN_ON(adap != client->adapter).
But, thinking about it again now, I think it is possible.
What about this sequence:
- i2c-adap-register (address P1)
- .open(), client->adapter = P1;
- .read/write/ioctl()..
- i2c-adap-unregister (adapter freed)
- i2c-adap-register with same adapter_nr (address P2);
- .read/write/ioctl().
Wouldn't the address differ here ?
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
