linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa@the-dreams.de>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Jean Delvare <jdelvare@suse.com>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	gregkh@linuxfoundation.org, Johan Hovold <johan@kernel.org>,
	Alex Elder <elder@linaro.org>
Subject: Re: [PATCH 2/2] i2c-dev: Don't block the adapter from unregistering
Date: Wed, 6 Jul 2016 15:55:24 +0900	[thread overview]
Message-ID: <20160706065523.GA1439@tetsubishi> (raw)
In-Reply-To: <021486be2f5425ce2379219a7ac163ee14ba2aba.1467772840.git.viresh.kumar@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

On Tue, Jul 05, 2016 at 07:57:07PM -0700, 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@linaro.org>

I'd think Jean has more experience with I2C hotplugging approaches and
difficulties, so I'd be interested in his high level review.

However:

> @@ -234,6 +234,7 @@ struct i2c_client {
>  	struct i2c_adapter *adapter;	/* the adapter we sit on	*/
>  	struct device dev;		/* the device structure		*/
>  	int irq;			/* irq issued by device		*/
> +	int adapter_nr;
>  	struct list_head detected;

Adding something to *every* i2c_client for this corner case sounds
pretty expensive to me.

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2016-07-06  6:55 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-06  2:57 [PATCH 0/2] i2c-dev: Don't let userspace block adapter Viresh Kumar
2016-07-06  2:57 ` [PATCH 1/2] i2c-dev: don't get i2c adapter via i2c_dev Viresh Kumar
2016-07-06 17:04   ` Jean Delvare
2016-07-06 17:07     ` Viresh Kumar
2016-07-07 13:16   ` Jean Delvare
2016-07-07 15:35     ` Viresh Kumar
2016-07-08  1:31   ` Wolfram Sang
2016-07-06  2:57 ` [PATCH 2/2] i2c-dev: Don't block the adapter from unregistering Viresh Kumar
2016-07-06  4:32   ` kbuild test robot
2016-07-06  6:55   ` Wolfram Sang [this message]
2016-07-06 13:50     ` Viresh Kumar
2016-07-06 17:12     ` Jean Delvare
2016-07-06 20:55       ` Viresh Kumar
2016-07-11 12:22         ` Jean Delvare
2016-07-11 21:50           ` Greg KH
2016-07-18 20:20             ` Viresh Kumar
2016-07-25  9:39             ` Jean Delvare
2016-07-25 22:31               ` Viresh Kumar
2016-07-26  7:41                 ` Jean Delvare
2016-07-26 15:18                   ` Dmitry Torokhov
2016-07-06  8:22   ` Peter Rosin
2016-07-06 14:33     ` Viresh Kumar
2016-07-06 14:43       ` Lars-Peter Clausen
2016-07-06 15:04         ` Peter Rosin
2016-07-06 15:37           ` Viresh Kumar
2016-07-06 15:35         ` Viresh Kumar
2016-07-06 14:41 ` [PATCH 0/2] i2c-dev: Don't let userspace block adapter Lars-Peter Clausen
2016-07-06 15:34   ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160706065523.GA1439@tetsubishi \
    --to=wsa@the-dreams.de \
    --cc=elder@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jdelvare@suse.com \
    --cc=johan@kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).