pcmcia: move unbind/rebind into dev_pm_ops.complete

PATCH RESEND Superseded Christian Lamparter 3 participants 6 messages Jul 6, 2012 – Jul 9, 2012 Pcmcia

Share & tools

Christian Lamparter

author

2012-07-06 21:24 UTC #0 lore raw

PATCH RESEND 1/1 pcmcia: move unbind/rebind into dev_pm_ops.complete

The idea of moving rebind procedure into pm.complete was taken from the usb-subsystem, which has similar problems with reattaching devices during/after resume.

Signed-off-by Christian Lamparter <[email protected]>

---
To Greg:

I have submitted this patch back in March and again in May. As far as I can tell it was neither rejected nor was it accepted into linux-pcmcia.git since. So I'm asking you, if you could take the patch instead... please.

Regards,
Christian

PS: I'm not on the pcmcia/kernel list, so please keep the 'CC'.

---
 drivers/pcmcia/cs.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

drivers/pcmcia/cs.c +18 -4

index d9ea192..503596f 100644
@@ -512,6 +512,13 @@ static int socket_late_resume(struct pcmcia_socket *skt)
 		return socket_insert(skt);
 	}
 
+	if (!(skt->state & SOCKET_CARDBUS) && (skt->callback))
+		skt->callback->early_resume(skt);
+	return 0;
+}
+
+static int socket_complete_resume(struct pcmcia_socket *skt)
+{
 #ifdef CONFIG_CARDBUS
 	if (skt->state & SOCKET_CARDBUS) {
 		/* We can't be sure the CardBus card is the same

@@ -519,11 +526,8 @@ static int socket_late_resume(struct pcmcia_socket *skt)
 		 * and re-add... */
 		cb_free(skt);
 		cb_alloc(skt);
-		return 0;
 	}
 #endif
-	if (!(skt->state & SOCKET_CARDBUS) && (skt->callback))
-		skt->callback->early_resume(skt);
 	return 0;
 }

@@ -534,11 +538,15 @@ static int socket_late_resume(struct pcmcia_socket *skt)
  */
 static int socket_resume(struct pcmcia_socket *skt)
 {
+	int err;
 	if (!(skt->state & SOCKET_SUSPEND))
 		return -EBUSY;
 
 	socket_early_resume(skt);
-	return socket_late_resume(skt);
+	err = socket_late_resume(skt);
+	if (!err)
+		socket_complete_resume(skt);
+	return err;
 }
 
 static void socket_remove(struct pcmcia_socket *skt)

@@ -849,6 +857,11 @@ static int __used pcmcia_socket_dev_resume(struct device *dev)
 	return __pcmcia_pm_op(dev, socket_late_resume);
 }
 
+static void __used pcmcia_socket_dev_complete(struct device *dev)
+{
+	__pcmcia_pm_op(dev, socket_complete_resume);
+}
+
 static const struct dev_pm_ops pcmcia_socket_pm_ops = {
 	/* dev_resume may be called with IRQs enabled */
 	SET_SYSTEM_SLEEP_PM_OPS(NULL,

@@ -863,6 +876,7 @@ static const struct dev_pm_ops pcmcia_socket_pm_ops = {
 	.resume_noirq = pcmcia_socket_dev_resume_noirq,
 	.thaw_noirq = pcmcia_socket_dev_resume_noirq,
 	.restore_noirq = pcmcia_socket_dev_resume_noirq,
+	.complete = pcmcia_socket_dev_complete,
 };
 
 #define PCMCIA_SOCKET_CLASS_PM_OPS (&pcmcia_socket_pm_ops)

Signature · 1 line

1.7.9.1

Greg KH

2012-07-06 21:30 UTC #1 lore raw

Quoted 5 lines · Christian Lamparter

The idea of moving rebind procedure into pm.complete was taken from the usb-subsystem, which has similar problems with reattaching devices during/after resume.

Quoted 8 lines

Signed-off-by Christian Lamparter <[email protected]>
---
To Greg:
I have submitted this patch back in March and again in May. As far as I can tell it was neither rejected nor was it accepted into linux-pcmcia.git since. So I'm asking you, if you could take the patch instead... please.

There is a PCMCIA "team" who should be taking these types of patches. Why are they not doing so?

confused,

greg k-h

Andrew Morton

2012-07-09 21:59 UTC #2 lore raw

Quoted 10 lines · On Fri, 6 Jul 2012 14:30:16 -0700 Greg KH <[email protected]> wrote:

Quoted 9 lines · Christian Lamparter
The idea of moving rebind procedure into pm.complete was taken from the usb-subsystem, which has similar problems with reattaching devices during/after resume.
Signed-off-by Christian Lamparter <[email protected]>
---
To Greg:

Quoted 8 lines

Quoted 4 lines
I have submitted this patch back in March and again in May. As far as I can tell it was neither rejected nor was it accepted into linux-pcmcia.git since. So I'm asking you, if you could take the patch instead... please.
There is a PCMCIA "team" who should be taking these types of patches. Why are they not doing so?

Things are pretty quiet in pcmcia world, but Dominik does appear to still be doing stuff.

I sometimes queue PCMCIA patches for people, but not this one. The changelog is just junk. What does the patch do? Why does it do it? What problems does it solve? What are these mysterious "problems with reattaching devices" to which it refers? Useless...

Christian Lamparter

author

2012-07-09 22:55 UTC #3 lore raw

Quoted 22 lines · Andrew Morton

Quoted 10 lines · On Fri, 6 Jul 2012 14:30:16 -0700 Greg KH <[email protected]> wrote:
Quoted 9 lines · Christian Lamparter
The idea of moving rebind procedure into pm.complete was taken from the usb-subsystem, which has similar problems with reattaching devices during/after resume.
Signed-off-by Christian Lamparter <[email protected]>
---
To Greg:
Quoted 8 lines
Quoted 4 lines
I have submitted this patch back in March and again in May. As far as I can tell it was neither rejected nor was it accepted into linux-pcmcia.git since. So I'm asking you, if you could take the patch instead... please.
There is a PCMCIA "team" who should be taking these types of patches. Why are they not doing so?

Quoted 8 lines

Things are pretty quiet in pcmcia world, but Dominik does appear to still be doing stuff.
I sometimes queue PCMCIA patches for people, but not this one. The changelog is just junk. What does the patch do? Why does it do it? What problems does it solve? What are these mysterious "problems with reattaching devices" to which it refers? Useless...

Well, that can be improved, but it is a bit tricky. AFAICT the usb subsystem dealt with pm in this commit:

"commit 5096aedcd2eb70fbea83f09281f97f9ec973d9de

Author: Alan Stern <[email protected]>
Date:   Tue Aug 12 14:34:14 2008 -0400

USB: Don't rebind before "complete" callback

[...] We are not allowed to call drivers' probe routines during a system sleep transition between the "prepare" and "complete" callbacks, but that's exactly what we do when a driver doesn't have full suspend/resume support. [...]"

And on the pcmcia subsystem we have this:

"commit 88b060d6c03fcb9e4d2018b4349954c4242a5c7f

Author: Dominik Brodowski <[email protected]>
Date:   Sat Jan 2 14:14:23 2010 +0100

pcmcia: improve check for same card in slot after resume

During a suspend/resume cycle, an user may change the card in the PCMCIA/CardBus slot. [...]

For CardBus devices, the PCI hotplug interface doesn't offer a "rescan" facility which also _removes_ devices no longer to be found behind a bridge. Therefore, remove and re-add all devices unconditionally."

Unfortunately, the "re-add" is currently done in the *pm resume* callback (socket_late_resume), but according to "USB: Don't rebind..." is not allowed to have it there, so the patch moves it into the *pm complete* callback. The Documentation/power/* contains mostly informations for drivers developers, but AFAICT it doesn't say much about the subsystem to which the device is connected should behave, so there's a bit of a "citing-gap".

So, my question now: Would you accept the pcmcia patch if I add the "USB: Don't rebind..." as a reference to why the re-add needs to be done in complete? Or do you think that I should bug the pm people (and Alan - since he wrote that it is "not allowed") in this case so I can link their official answer to this patch?

Regards,
Chr

Andrew Morton

2012-07-09 23:01 UTC #4 lore raw

Quoted 60 lines · On Tue, 10 Jul 2012 00:54:54 +0200 Christian Lamparter…

Quoted 22 lines · Andrew Morton
Quoted 10 lines · On Fri, 6 Jul 2012 14:30:16 -0700 Greg KH <[email protected]> wrote:
Quoted 9 lines · Christian Lamparter
The idea of moving rebind procedure into pm.complete was taken from the usb-subsystem, which has similar problems with reattaching devices during/after resume.
Signed-off-by Christian Lamparter <[email protected]>
---
To Greg:
Quoted 8 lines
Quoted 4 lines
I have submitted this patch back in March and again in May. As far as I can tell it was neither rejected nor was it accepted into linux-pcmcia.git since. So I'm asking you, if you could take the patch instead... please.
There is a PCMCIA "team" who should be taking these types of patches. Why are they not doing so?
Quoted 8 lines
Things are pretty quiet in pcmcia world, but Dominik does appear to still be doing stuff.
I sometimes queue PCMCIA patches for people, but not this one. The changelog is just junk. What does the patch do? Why does it do it? What problems does it solve? What are these mysterious "problems with reattaching devices" to which it refers? Useless...
Well, that can be improved, but it is a bit tricky. AFAICT the usb subsystem dealt with pm in this commit:
"commit 5096aedcd2eb70fbea83f09281f97f9ec973d9de
Author: Alan Stern <[email protected]>
Date:   Tue Aug 12 14:34:14 2008 -0400
USB: Don't rebind before "complete" callback
[...] We are not allowed to call drivers' probe routines during a system sleep transition between the "prepare" and "complete" callbacks, but that's exactly what we do when a driver doesn't have full suspend/resume support. [...]"
And on the pcmcia subsystem we have this:
"commit 88b060d6c03fcb9e4d2018b4349954c4242a5c7f
Author: Dominik Brodowski <[email protected]>
Date:   Sat Jan 2 14:14:23 2010 +0100
pcmcia: improve check for same card in slot after resume
During a suspend/resume cycle, an user may change the card in the PCMCIA/CardBus slot. [...]
For CardBus devices, the PCI hotplug interface doesn't offer a "rescan" facility which also _removes_ devices no longer to be found behind a bridge. Therefore, remove and re-add all devices unconditionally."

Quoted 7 lines

Unfortunately, the "re-add" is currently done in the *pm resume* callback (socket_late_resume), but according to "USB: Don't rebind..." is not allowed to have it there, so the patch moves it into the *pm complete* callback. The Documentation/power/* contains mostly informations for drivers developers, but AFAICT it doesn't say much about the subsystem to which the device is connected should behave, so there's a bit of a "citing-gap".

hm, it does seem a bit of a screwup.

What's unclear to me is whether your patch fixes any observed runtime problems. Or adds any runtime problems, which looks to be a distinct possibility.

Quoted 5 lines

So, my question now: Would you accept the pcmcia patch if I add the "USB: Don't rebind..." as a reference to why the re-add needs to be done in complete? Or do you think that I should bug the pm people (and Alan - since he wrote that it is "not allowed") in this case so I can link their official answer to this patch?

Well I could grab it and give it a little bit of testing in linux-next. But I'd be super-reluctant to send such a patch upstream without detailed input from Alan/Greg/Rafael/Dominik/etc.

Christian Lamparter

author

2012-07-09 23:07 UTC #5 lore raw

Quoted 71 lines · Andrew Morton

Quoted 60 lines · On Tue, 10 Jul 2012 00:54:54 +0200 Christian Lamparter…
Quoted 22 lines · Andrew Morton
Quoted 10 lines · On Fri, 6 Jul 2012 14:30:16 -0700 Greg KH <[email protected]> wrote:
Quoted 9 lines · Christian Lamparter
The idea of moving rebind procedure into pm.complete was taken from the usb-subsystem, which has similar problems with reattaching devices during/after resume.
Signed-off-by Christian Lamparter <[email protected]>
---
To Greg:
Quoted 8 lines
Quoted 4 lines
I have submitted this patch back in March and again in May. As far as I can tell it was neither rejected nor was it accepted into linux-pcmcia.git since. So I'm asking you, if you could take the patch instead... please.
There is a PCMCIA "team" who should be taking these types of patches. Why are they not doing so?
Quoted 8 lines
Things are pretty quiet in pcmcia world, but Dominik does appear to still be doing stuff.
I sometimes queue PCMCIA patches for people, but not this one. The changelog is just junk. What does the patch do? Why does it do it? What problems does it solve? What are these mysterious "problems with reattaching devices" to which it refers? Useless...
Well, that can be improved, but it is a bit tricky. AFAICT the usb subsystem dealt with pm in this commit:
"commit 5096aedcd2eb70fbea83f09281f97f9ec973d9de
Author: Alan Stern <[email protected]>
Date:   Tue Aug 12 14:34:14 2008 -0400
USB: Don't rebind before "complete" callback
[...] We are not allowed to call drivers' probe routines during a system sleep transition between the "prepare" and "complete" callbacks, but that's exactly what we do when a driver doesn't have full suspend/resume support. [...]"
And on the pcmcia subsystem we have this:
"commit 88b060d6c03fcb9e4d2018b4349954c4242a5c7f
Author: Dominik Brodowski <[email protected]>
Date:   Sat Jan 2 14:14:23 2010 +0100
pcmcia: improve check for same card in slot after resume
During a suspend/resume cycle, an user may change the card in the PCMCIA/CardBus slot. [...]
For CardBus devices, the PCI hotplug interface doesn't offer a "rescan" facility which also _removes_ devices no longer to be found behind a bridge. Therefore, remove and re-add all devices unconditionally."
Quoted 7 lines
Unfortunately, the "re-add" is currently done in the *pm resume* callback (socket_late_resume), but according to "USB: Don't rebind..." is not allowed to have it there, so the patch moves it into the *pm complete* callback. The Documentation/power/* contains mostly informations for drivers developers, but AFAICT it doesn't say much about the subsystem to which the device is connected should behave, so there's a bit of a "citing-gap".

Quoted 5 lines

hm, it does seem a bit of a screwup.
What's unclear to me is whether your patch fixes any observed runtime problems. Or adds any runtime problems, which looks to be a distinct possibility.

well, there is this bug

Quoted 12 lines

Quoted 6 lines
So, my question now: Would you accept the pcmcia patch if I add the "USB: Don't rebind..." as a reference to why the re-add needs to be done in complete? Or do you think that I should bug the pm people (and Alan - since he wrote that it is "not allowed") in this case so I can link their official answer to this patch?
Well I could grab it and give it a little bit of testing in linux-next. But I'd be super-reluctant to send such a patch upstream without detailed input from Alan/Greg/Rafael/Dominik/etc.