Re: [PATCH] PCMCIA: prevent auto insert during resume.

Previous thread: Re: kernel 2.6.23 CFS problem? by Wang, Baojun on Friday, October 26, 2007 - 3:04 am. (1 message)

Next thread: [PATCH] dz.c: Resource management by Maciej W. Rozycki on Friday, October 26, 2007 - 4:17 am. (1 message)
From: Rodolfo Giometti
Date: Friday, October 26, 2007 - 3:51 am

If a socket has been ejected before sleeping, at resume time it
shouldn't be awaked.

Signed-off-by: Rodolfo Giometti <giometti@linux.it>
---
 drivers/pcmcia/cs.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pcmcia/cs.c b/drivers/pcmcia/cs.c
index 729e37d..d69de74 100644
--- a/drivers/pcmcia/cs.c
+++ b/drivers/pcmcia/cs.c
@@ -562,10 +562,8 @@ static int socket_resume(struct pcmcia_socket *skt)
 	skt->ops->init(skt);
 	skt->ops->set_socket(skt, &skt->socket);
 
-	if (!(skt->state & SOCKET_PRESENT)) {
-		skt->state &= ~SOCKET_SUSPEND;
-		return socket_insert(skt);
-	}
+	if (!(skt->state & SOCKET_PRESENT))
+		goto resume_exit;
 
 	ret = socket_setup(skt, SS_COMA, resume_delay);
 	if (ret == CS_SUCCESS) {
@@ -599,6 +597,7 @@ static int socket_resume(struct pcmcia_socket *skt)
 		socket_shutdown(skt);
 	}
 
+resume_exit:
 	skt->state &= ~SOCKET_SUSPEND;
 
 	return CS_SUCCESS;
-- 
1.5.2.5

-

From: Russell King
Date: Friday, October 26, 2007 - 8:11 am

Conversely, if a card has been inserted into an empty slot prior to
resuming, it should be detected.  Removing this check prevents that
happening, which sucks for at least embedded devices.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-

From: Rodolfo Giometti
Date: Friday, October 26, 2007 - 8:47 am

You can use "pccardctl insert" to do it. :)

On battery powered device I should prevent power lost, so if I power
down a device I'd like it should remain off even after resume.

Why the system should power it on automagically? Just for detection?

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127
-

From: Russell King
Date: Friday, October 26, 2007 - 9:02 am

In which case you need to remember why it was powered down and act

When you bring the battery device out of resume, and you've inserted a
card, you want it to be detected.  Your change means you have to wait
until the system has finished resuming before you plug the card in,
which practically is a pain in the butt and actually leads to user
errors.  IOW:

"I plugged my wireless card in after I pressed the power button, why
wasn't it detected?"

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-

From: Rodolfo Giometti
Date: Friday, October 26, 2007 - 9:27 am

Yes. This have some reasons... for example, my custom board has a WiFi
connected to the PCMCIA interface (which consumes a lot of power) and
if the user switch off the WiFi I think he/she doesn't wish the WiFi
is automagic powered on after resume... this behaviour can cause power

My patch doesn't affect the power on sequence, just the resume
one. Also if you didn't eject the socket, at resume the device will be
powered up again, my patch just prevents that a pre-powered off device
to be turned on at resume time.

However you should consider that some embedded systems have fixed
PCMCIA devices that can't be removed so there are no reasons to detect
them after resume, nobody can change them. :)

Also battery powered devices can go very frequently to sleep and the
current behavior force the user to switch off the unused device each
time the system resumes from sleep.

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127
-

From: Russell King
Date: Friday, October 26, 2007 - 9:36 am

On a lot of devices, the "power button" is the resume button.  When you

I realise that.  I do work on embedded devices, and this behaviour is
explicitly there to support embedded devices.

I've suggested a workable solution to you which allows both of us to
have the behaviour we both desire from the system.  That sounds like
a negotiated solution to me...

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-

From: Rodolfo Giometti
Date: Friday, October 26, 2007 - 9:54 am

I see. My device has two buttons.

However even in this case, if you never eject the device, then switch
off (sleep), change the device into the slot and then power on (wake
up), with my patch everything continues to work as before.


Do you mean to switch off the socket from userland? It could be a
solution but in this case the device is powered on each time even if
for a short delay...

Maybe we can add an entry into sysfs? Or just a module parameter? So
developers can choose their preferred behaviour. :)

Thanks for your time,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127
-

From: Russell King
Date: Friday, October 26, 2007 - 10:00 am

Yes and it's extremely irritating that a plugged in card doesn't get

If it's a permanent device, and you've powered it down via pccardctl,
then you've powered it down from userland.  So record that it's been
powered down from userland.  Then, on resume, if it's been powered down
from userland, don't try to re-power it on resume.


Or that - probably a sysfs attribute on the pcmcia socket would be
better.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-

From: Rodolfo Giometti
Date: Friday, October 26, 2007 - 10:18 am

But the userland doesn't re-power it on resume... it's the kernel
itself whos re-powers the device on resume. So the userland can only
power down the device again.


Ok, but how can I do it? Can you please suggest to me the name of such
attribute and where should I add it into the PCMCIA code? Also, which
should be the default dehaviour? ;)

Thanks in advance,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127
-

From: Russell King
Date: Friday, October 26, 2007 - 11:37 am

My consulting rates are...  Seriously, if you want me to write a patch
for you, you have to pay for that.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-

From: Rodolfo Giometti
Date: Friday, October 26, 2007 - 1:52 pm

O_o You misunderstand me... I just like some hints about to do it by
myself.

Since (most probably) you are the one which should accept or deny my
patch I simple asked to you how I should do the job in order to avoid
a reject! :)

The name of the attribute, which should be its default behaviour and
an hint where should I put the code (just a file name or a function
one).

Thanks in advance,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127
-

From: Pavel Machek
Date: Monday, October 29, 2007 - 12:24 pm

I think Russell means: at a flag into kernel. If user powers down the
device, set the flag. If flag is set during resume, avoid powering up
the device.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-

From: Rodolfo Giometti
Date: Thursday, November 1, 2007 - 7:53 am

That's exactly what my patch does! :)

If the user does 'eject' the device is not powered on at resume.

Currently, with out the patch, if you do an 'eject' to power down the
device, then you go to sleep and resume, the device is powered up
again and you have to do a new 'eject' to power it down.

My patch fixes this behaviour.

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127
-

From: Russell King
Date: Thursday, November 1, 2007 - 11:37 am

Let's be absolutely clear about this.  The patch in your original post
does *NOT* do that.  It *completely* removes the possibility of powering
up a device inserted into the PCMCIA slot before resuming without
unplugging and replugging it by removing the code which detects an
inserted card on resume.

And let's also be clear about something else.  You _were_ crystal clear
on that aspect of it from your last mail on the subject since you were
asking for names of attributes to set and clear such a flag.  I didn't
respond because I'm not going to hold your hand with such obvious
issues - if you need that level of support, it will be far faster for
me to write the damned patch myself.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-

From: Russell King
Date: Thursday, November 1, 2007 - 11:56 am

Oh, and I'd like to make another thing clear - let's get the roles of
responsibility right.

I'm the ex-PCMCIA maintainer who had a requirement for the current
behaviour on my embedded ARM devices with classical PCMCIA sockets.

Dominik is the current PCMCIA maintainer who gets to say what goes in,
how things should be designed, etc.

You're the guy coming along with a different requirement for a device
using the PCMCIA subsystem in a non-classical way (non-pluggable PCMCIA)
and finding that the subsystem doesn't work in a good way with that
setup, and suggesting we break classical PCMCIA setups to make it
work.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-

From: Rafael J. Wysocki
Date: Thursday, November 1, 2007 - 3:43 pm

OK

Can we CC all future messages regarding this patch to Dominik, please?
-

From: Rodolfo Giometti
Date: Tuesday, November 6, 2007 - 2:09 am

Maybe I'm one of that guys... but _in_ _my_ _humble_ _opinion_ the
currently behaviour of PCMCIA subsystem doesn't fit well low power
requirements for battery powered devices. Powered down devices should
remain off through a suspend/resume. Maybe PCMCIA subsystem needs some
modifications? :)

However I just proposed a patch. If you think it's not useful or
correct or whatever you think about it, please, don't use it.

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127
-

From: Rodolfo Giometti
Date: Tuesday, November 6, 2007 - 1:58 am

Current status without the patch:

   hostname:~# pccardctl status
   Socket 0:
     3.3V 16-bit PC Card
     Subdevice 0 (function 0) [unbound]
   hostname:~# pccardctl eject
   pccard: card ejected from slot 0
   hostname:~# pccardctl status
   Socket 0:
     no card
   hostname:~# apm -s
   Stopping tasks ... done.
   Suspending console(s)
   <<RESUME>>
   hostname:~# usb0: full speed config #1: 100 mA, Ethernet Gadget, using CDC Ethernet Subset
   pccard: PCMCIA card inserted into slot 0
   pcmcia: registering new device pcmcia0.0
   Restarting tasks ... done.
   hostname:~# pccardctl status
   Socket 0:
     3.3V 16-bit PC Card
     Subdevice 0 (function 0) [unbound]

The device into PCMCIA slot 0 is _auto_ powered up again.

(Possible) future status with the patch:

   hostname:~# pccardctl status
   Socket 0:
     3.3V 16-bit PC Card
     Subdevice 0 (function 0) [unbound]
   hostname:~# pccardctl eject 
   pccard: card ejected from slot 0
   hostname:~# pccardctl status
   Socket 0:
     no card
   hostname:~# apm -s
   Stopping tasks ... done.
   Suspending console(s)
   <<RESUME>>
   hostname:~# usb0: full speed config #1: 100 mA, Ethernet Gadget, using CDC Ethernet Subset
   Restarting tasks ... done.
   pccardctl status
   Socket 0:
     no card
   hostname:~# pccardctl insert
   pccard: PCMCIA card inserted into slot 0
   pcmcia: registering new device pcmcia0.0
   hostname:~# pccardctl status
   Socket 0:
     3.3V 16-bit PC Card
     Subdevice 0 (function 0) [unbound]

The device into PCMCIA slot 0 is _not_ powered up and the user should
use "pccardctl insert" to do that (on my battery powered device this
behaviour saves the battery a lot).


In the past I did some "obvious" patches and the maintainer told me:
"plase change this name" or "modify the default behaviour",
etc.. That's why I asked for. I thought it could be less time
consuming ask before then modify again after...

Ciao,

Rodolfo

-- 

GNU/Linux Solutions       ...
Previous thread: Re: kernel 2.6.23 CFS problem? by Wang, Baojun on Friday, October 26, 2007 - 3:04 am. (1 message)

Next thread: [PATCH] dz.c: Resource management by Maciej W. Rozycki on Friday, October 26, 2007 - 4:17 am. (1 message)