spi/spi: don't release the spi device twice

While rmmoding pxa2xx_spi I hit:

|BUG: unable to handle kernel paging request at 6b6b6b9b
|IP: [<c115bf61>] device_del+0x11/0x140
|Call Trace:
| [<c115c09b>] ? device_unregister+0xb/0x20
| [<c118fe33>] ? spi_unregister_master+0x93/0xc0
| [<f806c554>] ? pxa2xx_spi_remove+0x84/0xc0 [pxa2xx_spi]

According to my debug printks, the loop

| dummy = device_for_each_child(master->dev.parent, &master->dev,
| __unregister);

calls __unregister for all childs of spi devicee (spidev in my case) and the spi device itself. So calling device_unregister() for the device itself leads to trouble.
This seems to be comming from 3486008 aka ("spi: free children in spi_unregister_master, not siblings") so therefore I cc stable for v36.

---
This is on v2.6.37-rc1. Unless this got fixed somewhere else in the meantime it is still there.

 drivers/spi/spi.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

index 709c836..3c8ff6f 100644
@@ -586,7 +586,6 @@ void spi_unregister_master(struct spi_master *master)
 
 	dummy = device_for_each_child(master->dev.parent, &master->dev,
 					__unregister);
-	device_unregister(&master->dev);
 }
 EXPORT_SYMBOL_GPL(spi_unregister_master);
 

1.7.3.2


------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today
http://p.sf.net/sfu/msIE9-sfdev2dev

This code is the old code, before patch 3486008 which you're citing. 3486008 does:

- dummy = device_for_each_child(master->dev.parent, &master->dev,
- __unregister);
+ dummy = device_for_each_child(&master->dev, NULL, __unregister);

Considering that this patch is in 2.6.36 (and 36.1), you seem to have mixed up your sources. Please make sure your checkout is current and unbroken...

"Mu."

Hmmm.

# git describe --long
v2.6.37-rc3-0-g3561d43

After looking at spi_unregister_master() in drivers/spi/spi.c, I see:

      dummy = device_for_each_child(master->dev.parent, &master->dev,
                      __unregister);
      device_unregister(&master->dev);

}

This change got back in by:

commit 2b9603a0d7e395fb844af90fba71448bc8019077

Author: Feng Tang <[email protected]>
Date:   Mon Aug 2 15:52:15 2010 +0800

spi: enable spi_board_info to be registered after spi_master

which is v2.6.37-rc1~2^2~4. So I probably mixed up you with Feng.

This thread starts at
http://www.mail-archive.com/spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org/msg05437.html

Sebastian

------------------------------------------------------------------------------
Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev

Hi Sebastian,

I checked my original patch which didn't touch the logic of spi_unregister_master() as
-----------------------------------------
@@ -568,6 +592,10 @@ void spi_unregister_master(struct spi_master *master)
{
int dummy;

+ mutex_lock(&board_lock);
+ list_del(&master->list);
+ mutex_unlock(&board_lock);
+
dummy = device_for_each_child(master->dev.parent, &master->dev,
__unregister);
device_unregister(&master->dev);
-----------------------------------------

So this should be a merge problem, which corrupt the commit from David's commit 3486008 "spi: free children in spi_unregister_master, not siblings"

Thanks,
Feng

------------------------------------------------------------------------------
Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev

This was fixed by David Lamparter in v2.6.36-rc5 3486008 ("spi: free children in spi_unregister_master, not siblings") and broken again in v2.6.37-rc1~2^2~4 during the merge of 2b9603a0 ("spi: enable spi_board_info to be registered after spi_master").

---
Okay, Feng. So here is the merge fixup.

 drivers/spi/spi.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

index 709c836..b02d0cb 100644
@@ -584,8 +584,7 @@ void spi_unregister_master(struct spi_master *master)
 	list_del(&master->list);
 	mutex_unlock(&board_lock);
 
-	dummy = device_for_each_child(master->dev.parent, &master->dev,
-					__unregister);
+	dummy = device_for_each_child(&master->dev, NULL, __unregister);
 	device_unregister(&master->dev);
 }
 EXPORT_SYMBOL_GPL(spi_unregister_master);

1.7.3.2

------------------------------------------------------------------------------
Increase Visibility of Your 3D Game App & Earn a Chance To Win $500!
Tap into the largest installed PC base & get more eyes on your game by
optimizing for Intel(R) Graphics Technology. Get started today with the
Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev

I just checked, all other parts should be ok.

Thanks,
Feng

------------------------------------------------------------------------------
Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs.
http://p.sf.net/sfu/intelisp-dev2dev

ping