Re: wrong driver remove order

Re: wrong driver remove order

[Jean Delvare]

Hi Guennadi,

On Thu, 30 Oct 2008 15:13:14 +0100 (CET), Guennadi Liakhovetski wrote:
> Hi,
> 
> I have two i2c modules, one of which uses the other one: a camera and a 
> GPIO-extender, and the camera uses a GPIO from that extender. I first load 
> the GPIO-extender driver (pca953x.c), then the camera (mt9m001.c), then as 
> I tried to remove the bus driver (i2c-mxc, not yet in the mainline), first 
> the GPIO-extender's .remove() method has been called, which, of course, 
> failed, because mt9m001 was still holding its GPIO... AFAIU, the remove 
> order should be reverse from probe, right?

It would indeed make sense for the removal order to be the reverse of
the bind order. Can you please try the following patch and report if it
solves your problem?

From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Subject: i2c: Remove i2c clients in reverse order

i2c clients should be removed in reverse order compared to the probe
(actually: bind) order. This matters when several clients depend on
each other.

Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
---
 drivers/i2c/i2c-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.28-rc5.orig/drivers/i2c/i2c-core.c	2008-11-17 21:29:59.000000000 +0100
+++ linux-2.6.28-rc5/drivers/i2c/i2c-core.c	2008-11-17 21:44:06.000000000 +0100
@@ -631,7 +631,7 @@ int i2c_del_adapter(struct i2c_adapter *
 
 	/* detach any active clients. This must be done first, because
 	 * it can fail; in which case we give up. */
-	list_for_each_entry_safe(client, _n, &adap->clients, list) {
+	list_for_each_entry_safe_reverse(client, _n, &adap->clients, list) {
 		struct i2c_driver	*driver;
 
 		driver = client->driver;


-- 
Jean Delvare

Re: wrong driver remove order

[Jean Delvare]

On Mon, 17 Nov 2008 22:04:17 +0100, Jean Delvare wrote:
> Hi Guennadi,
> 
> On Thu, 30 Oct 2008 15:13:14 +0100 (CET), Guennadi Liakhovetski wrote:
> > Hi,
> > 
> > I have two i2c modules, one of which uses the other one: a camera and a 
> > GPIO-extender, and the camera uses a GPIO from that extender. I first load 
> > the GPIO-extender driver (pca953x.c), then the camera (mt9m001.c), then as 
> > I tried to remove the bus driver (i2c-mxc, not yet in the mainline), first 
> > the GPIO-extender's .remove() method has been called, which, of course, 
> > failed, because mt9m001 was still holding its GPIO... AFAIU, the remove 
> > order should be reverse from probe, right?
> 
> It would indeed make sense for the removal order to be the reverse of
> the bind order. Can you please try the following patch and report if it
> solves your problem?

Any news from this? I'd like to push this change to Linus this week if
it actually solves your problem.

> From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Subject: i2c: Remove i2c clients in reverse order
> 
> i2c clients should be removed in reverse order compared to the probe
> (actually: bind) order. This matters when several clients depend on
> each other.
> 
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> ---
>  drivers/i2c/i2c-core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-2.6.28-rc5.orig/drivers/i2c/i2c-core.c	2008-11-17 21:29:59.000000000 +0100
> +++ linux-2.6.28-rc5/drivers/i2c/i2c-core.c	2008-11-17 21:44:06.000000000 +0100
> @@ -631,7 +631,7 @@ int i2c_del_adapter(struct i2c_adapter *
>  
>  	/* detach any active clients. This must be done first, because
>  	 * it can fail; in which case we give up. */
> -	list_for_each_entry_safe(client, _n, &adap->clients, list) {
> +	list_for_each_entry_safe_reverse(client, _n, &adap->clients, list) {
>  		struct i2c_driver	*driver;
>  
>  		driver = client->driver;

-- 
Jean Delvare

Re: wrong driver remove order

[Guennadi Liakhovetski]

On Tue, 25 Nov 2008, Jean Delvare wrote:

> On Mon, 17 Nov 2008 22:04:17 +0100, Jean Delvare wrote:
> > Hi Guennadi,
> > 
> > On Thu, 30 Oct 2008 15:13:14 +0100 (CET), Guennadi Liakhovetski wrote:
> > > Hi,
> > > 
> > > I have two i2c modules, one of which uses the other one: a camera and a 
> > > GPIO-extender, and the camera uses a GPIO from that extender. I first load 
> > > the GPIO-extender driver (pca953x.c), then the camera (mt9m001.c), then as 
> > > I tried to remove the bus driver (i2c-mxc, not yet in the mainline), first 
> > > the GPIO-extender's .remove() method has been called, which, of course, 
> > > failed, because mt9m001 was still holding its GPIO... AFAIU, the remove 
> > > order should be reverse from probe, right?
> > 
> > It would indeed make sense for the removal order to be the reverse of
> > the bind order. Can you please try the following patch and report if it
> > solves your problem?
> 
> Any news from this? I'd like to push this change to Linus this week if
> it actually solves your problem.

Sorry, I didn't forget, I just was postponing this test, until I get more 
time... I've tested it now - it does solve the problem, thanks!

Tested-by: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>

Thanks
Guennadi

> 
> > From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Subject: i2c: Remove i2c clients in reverse order
> > 
> > i2c clients should be removed in reverse order compared to the probe
> > (actually: bind) order. This matters when several clients depend on
> > each other.
> > 
> > Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Cc: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> > ---
> >  drivers/i2c/i2c-core.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > --- linux-2.6.28-rc5.orig/drivers/i2c/i2c-core.c	2008-11-17 21:29:59.000000000 +0100
> > +++ linux-2.6.28-rc5/drivers/i2c/i2c-core.c	2008-11-17 21:44:06.000000000 +0100
> > @@ -631,7 +631,7 @@ int i2c_del_adapter(struct i2c_adapter *
> >  
> >  	/* detach any active clients. This must be done first, because
> >  	 * it can fail; in which case we give up. */
> > -	list_for_each_entry_safe(client, _n, &adap->clients, list) {
> > +	list_for_each_entry_safe_reverse(client, _n, &adap->clients, list) {
> >  		struct i2c_driver	*driver;
> >  
> >  		driver = client->driver;
> 
> -- 
> Jean Delvare
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer