public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
@ 2007-10-01 17:08 Kristen Carlson Accardi
  2007-10-01 17:19 ` Greg KH
  2007-10-01 17:43 ` Gary Hade
  0 siblings, 2 replies; 9+ messages in thread
From: Kristen Carlson Accardi @ 2007-10-01 17:08 UTC (permalink / raw)
  To: pcihpd-discuss; +Cc: linux-acpi, gregkh

Hi,
I notice in acpiphp that the code path for powering down the slot
via sysfs does not execute the _EJ0 method, but instead simply
looks for _PS3 and of couse disables all the bridges and devices.  
I suppose this could be valid depending on your definition of what 
/sys/bus/pci/slots/<slot_no>/power should do.

Is it intended to just power down the adapter that's in the slot, 
or is it intended to make the adapter in the slot able to be removed?
If it's intended to make the adapter able to be removed, shouldn't
we be calling _EJ0?

As a comparison, in pciehp when the sysfs power file is written,
we do actually go out and send the commands to the hotplug controller
to physically power off the slot.

Kristen

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 17:08 what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do? Kristen Carlson Accardi
@ 2007-10-01 17:19 ` Greg KH
  2007-10-01 17:35   ` Kristen Carlson Accardi
  2007-10-01 17:43 ` Gary Hade
  1 sibling, 1 reply; 9+ messages in thread
From: Greg KH @ 2007-10-01 17:19 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: pcihpd-discuss, linux-acpi

On Mon, Oct 01, 2007 at 10:08:58AM -0700, Kristen Carlson Accardi wrote:
> Hi,
> I notice in acpiphp that the code path for powering down the slot
> via sysfs does not execute the _EJ0 method, but instead simply
> looks for _PS3 and of couse disables all the bridges and devices.  
> I suppose this could be valid depending on your definition of what 
> /sys/bus/pci/slots/<slot_no>/power should do.
> 
> Is it intended to just power down the adapter that's in the slot, 
> or is it intended to make the adapter in the slot able to be removed?
> If it's intended to make the adapter able to be removed, shouldn't
> we be calling _EJ0?

If that is required to power down the device, yes, it should be called.

> As a comparison, in pciehp when the sysfs power file is written,
> we do actually go out and send the commands to the hotplug controller
> to physically power off the slot.

Yes, that is the intention of writing that value to that file.  The
power should be gone and it should be safe for the user to physically
remove the device after that write has succeeded.

Perhaps this is not happening due to the age of when the acpiphp driver
was first written?  If I recall, there was no ACPI pci hotplug spec at
the time and so it just used one specific bios implementation of what
was needed.  Things might have changed in the 5 or so years since then
:)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 17:19 ` Greg KH
@ 2007-10-01 17:35   ` Kristen Carlson Accardi
  2007-10-01 17:46     ` [Pcihpd-discuss] " Matthew Wilcox
  0 siblings, 1 reply; 9+ messages in thread
From: Kristen Carlson Accardi @ 2007-10-01 17:35 UTC (permalink / raw)
  To: Greg KH; +Cc: pcihpd-discuss, linux-acpi

On Mon, 1 Oct 2007 10:19:32 -0700
Greg KH <gregkh@suse.de> wrote:

> On Mon, Oct 01, 2007 at 10:08:58AM -0700, Kristen Carlson Accardi wrote:
> > Hi,
> > I notice in acpiphp that the code path for powering down the slot
> > via sysfs does not execute the _EJ0 method, but instead simply
> > looks for _PS3 and of couse disables all the bridges and devices.  
> > I suppose this could be valid depending on your definition of what 
> > /sys/bus/pci/slots/<slot_no>/power should do.
> > 
> > Is it intended to just power down the adapter that's in the slot, 
> > or is it intended to make the adapter in the slot able to be removed?
> > If it's intended to make the adapter able to be removed, shouldn't
> > we be calling _EJ0?
> 
> If that is required to power down the device, yes, it should be called.

So - here's where we get to interpret.  The spec says that _EJ0 is
used to "eject" the card.  From the spec:
6.3.3 _EJx (Eject)
These control methods are optional and are supplied for devices that 
support a software-controlled VCR-style ejection mechanism or that 
require an action be performed such as isolation of power/data lines 
before the device can be removed from the system. 
<snip>
For hot removal, the device must be immediately ejected when OSPM calls 
the _EJ0 control method. The _EJ0 control method does not return until 
ejection is complete. After calling _EJ0, OSPM verifies the
device no longer exists to determine if the eject succeeded. 
For _HID devices, OSPM evaluates the _STA method. For _ADR devices, 
OSPM checks with the bus driver for that device.

So, when I read this, it seems like if a vendor followed the spec,
executing _EJ0 would hang until we physically pulled the card out
of the system, which seems bad.  So, maybe this is why we don't call
_EJ0?  

_PS3 apparently is used to put a device into D3.  By "device" here
I'm not sure if we are talking about the adapter in the slot, or the
slot itself.


> 
> > As a comparison, in pciehp when the sysfs power file is written,
> > we do actually go out and send the commands to the hotplug controller
> > to physically power off the slot.
> 
> Yes, that is the intention of writing that value to that file.  The
> power should be gone and it should be safe for the user to physically
> remove the device after that write has succeeded.
> 
> Perhaps this is not happening due to the age of when the acpiphp driver
> was first written?  If I recall, there was no ACPI pci hotplug spec at
> the time and so it just used one specific bios implementation of what
> was needed.  Things might have changed in the 5 or so years since then
> :)
> 
> thanks,
> 
> greg k-h
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Pcihpd-discuss] what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 17:08 what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do? Kristen Carlson Accardi
  2007-10-01 17:19 ` Greg KH
@ 2007-10-01 17:43 ` Gary Hade
  2007-10-01 17:50   ` Kristen Carlson Accardi
  1 sibling, 1 reply; 9+ messages in thread
From: Gary Hade @ 2007-10-01 17:43 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: pcihpd-discuss, linux-acpi, gregkh

On Mon, Oct 01, 2007 at 10:08:58AM -0700, Kristen Carlson Accardi wrote:
> Hi,
> I notice in acpiphp that the code path for powering down the slot
> via sysfs does not execute the _EJ0 method, but instead simply
> looks for _PS3 and of couse disables all the bridges and devices.  
> I suppose this could be valid depending on your definition of what 
> /sys/bus/pci/slots/<slot_no>/power should do.
> 
> Is it intended to just power down the adapter that's in the slot, 
> or is it intended to make the adapter in the slot able to be removed?
> If it's intended to make the adapter able to be removed, shouldn't
> we be calling _EJ0?
> 
> As a comparison, in pciehp when the sysfs power file is written,
> we do actually go out and send the commands to the hotplug controller
> to physically power off the slot.

Kristen, This sounds a lot like the problem I was trying to address 
with the "fix slot poweroff problem on systems without _PS3" patch 
that I posted in June.
http://sourceforge.net/mailarchive/forum.php?thread_name=20070604232736.GA3239%40us.ibm.com&forum_name=pcihpd-discuss
It seems to be working fine on our systems.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Pcihpd-discuss] what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 17:35   ` Kristen Carlson Accardi
@ 2007-10-01 17:46     ` Matthew Wilcox
  0 siblings, 0 replies; 9+ messages in thread
From: Matthew Wilcox @ 2007-10-01 17:46 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: Greg KH, linux-acpi, pcihpd-discuss

On Mon, Oct 01, 2007 at 10:35:57AM -0700, Kristen Carlson Accardi wrote:
> So - here's where we get to interpret.  The spec says that _EJ0 is
> used to "eject" the card.  From the spec:
> 6.3.3 _EJx (Eject)
> These control methods are optional and are supplied for devices that 
> support a software-controlled VCR-style ejection mechanism or that 
> require an action be performed such as isolation of power/data lines 
> before the device can be removed from the system. 
> <snip>
> For hot removal, the device must be immediately ejected when OSPM calls 
> the _EJ0 control method. The _EJ0 control method does not return until 
> ejection is complete. After calling _EJ0, OSPM verifies the
> device no longer exists to determine if the eject succeeded. 
> For _HID devices, OSPM evaluates the _STA method. For _ADR devices, 
> OSPM checks with the bus driver for that device.
> 
> So, when I read this, it seems like if a vendor followed the spec,
> executing _EJ0 would hang until we physically pulled the card out
> of the system, which seems bad.  So, maybe this is why we don't call
> _EJ0?  

I think you've misread it.  Follow the other arm of the 'or':
	"require an action be performed such as isolation of power/data
	lines before the device can be removed from the system."
So I interpret "the eject succeeded" as "power/data lines are isolated"
for PCI cards.

> _PS3 apparently is used to put a device into D3.  By "device" here
> I'm not sure if we are talking about the adapter in the slot, or the
> slot itself.

Slots don't have a D3.  If you're talking about the bus, I guess there's
a B3.  But D is for devices.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Pcihpd-discuss] what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 17:43 ` Gary Hade
@ 2007-10-01 17:50   ` Kristen Carlson Accardi
  2007-10-01 18:33     ` Gary Hade
  0 siblings, 1 reply; 9+ messages in thread
From: Kristen Carlson Accardi @ 2007-10-01 17:50 UTC (permalink / raw)
  To: Gary Hade; +Cc: pcihpd-discuss, linux-acpi, gregkh

On Mon, 1 Oct 2007 10:43:41 -0700
Gary Hade <garyhade@us.ibm.com> wrote:

> On Mon, Oct 01, 2007 at 10:08:58AM -0700, Kristen Carlson Accardi wrote:
> > Hi,
> > I notice in acpiphp that the code path for powering down the slot
> > via sysfs does not execute the _EJ0 method, but instead simply
> > looks for _PS3 and of couse disables all the bridges and devices.  
> > I suppose this could be valid depending on your definition of what 
> > /sys/bus/pci/slots/<slot_no>/power should do.
> > 
> > Is it intended to just power down the adapter that's in the slot, 
> > or is it intended to make the adapter in the slot able to be removed?
> > If it's intended to make the adapter able to be removed, shouldn't
> > we be calling _EJ0?
> > 
> > As a comparison, in pciehp when the sysfs power file is written,
> > we do actually go out and send the commands to the hotplug controller
> > to physically power off the slot.
> 
> Kristen, This sounds a lot like the problem I was trying to address 
> with the "fix slot poweroff problem on systems without _PS3" patch 
> that I posted in June.
> http://sourceforge.net/mailarchive/forum.php?thread_name=20070604232736.GA3239%40us.ibm.com&forum_name=pcihpd-discuss
> It seems to be working fine on our systems.
> 
> Gary

ok - hum well, I seem to have not merged that patch for some reason, and
it also isn't anywhere in my inbox.  Can you resend it?  Seems like we
should get it in there.

BTW - people should copy me directly when they send their patches otherwise
they may get overlooked. (not that I'm saying this is what happened here,
it also happens sometimes that patches get buried in my inbox despite my
best efforts).

Thanks,
Kristen

> 
> -- 
> Gary Hade
> System x Enablement
> IBM Linux Technology Center
> 503-578-4503  IBM T/L: 775-4503
> garyhade@us.ibm.com
> http://www.ibm.com/linux/ltc
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Pcihpd-discuss] what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 17:50   ` Kristen Carlson Accardi
@ 2007-10-01 18:33     ` Gary Hade
  2007-10-01 18:39       ` Gary Hade
  2007-10-01 18:43       ` Kristen Carlson Accardi
  0 siblings, 2 replies; 9+ messages in thread
From: Gary Hade @ 2007-10-01 18:33 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: Gary Hade, linux-acpi, gregkh, pcihpd-discuss

On Mon, Oct 01, 2007 at 10:50:07AM -0700, Kristen Carlson Accardi wrote:
> On Mon, 1 Oct 2007 10:43:41 -0700
> Gary Hade <garyhade@us.ibm.com> wrote:
> 
> > On Mon, Oct 01, 2007 at 10:08:58AM -0700, Kristen Carlson Accardi wrote:
> > > Hi,
> > > I notice in acpiphp that the code path for powering down the slot
> > > via sysfs does not execute the _EJ0 method, but instead simply
> > > looks for _PS3 and of couse disables all the bridges and devices.  
> > > I suppose this could be valid depending on your definition of what 
> > > /sys/bus/pci/slots/<slot_no>/power should do.
> > > 
> > > Is it intended to just power down the adapter that's in the slot, 
> > > or is it intended to make the adapter in the slot able to be removed?
> > > If it's intended to make the adapter able to be removed, shouldn't
> > > we be calling _EJ0?
> > > 
> > > As a comparison, in pciehp when the sysfs power file is written,
> > > we do actually go out and send the commands to the hotplug controller
> > > to physically power off the slot.
> > 
> > Kristen, This sounds a lot like the problem I was trying to address 
> > with the "fix slot poweroff problem on systems without _PS3" patch 
> > that I posted in June.
> > http://sourceforge.net/mailarchive/forum.php?thread_name=20070604232736.GA3239%40us.ibm.com&forum_name=pcihpd-discuss
> > It seems to be working fine on our systems.
> > 
> > Gary
> 
> ok - hum well, I seem to have not merged that patch for some reason, and
> it also isn't anywhere in my inbox.  Can you resend it?  

Included below.

> Seems like we should get it in there.
> 
> BTW - people should copy me directly when they send their patches otherwise
> they may get overlooked. (not that I'm saying this is what happened here,
> it also happens sometimes that patches get buried in my inbox despite my
> best efforts).

In this case, I did send it directly to you with 
pcihpd-discuss@lists.sourceforge.net on gregkh@suse.de the
CC line.  You and Greg subsequently signed off on it and it 
entered mainline at 2.6.23-rc1.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc


On systems where the optional _PS3 ACPI object is not implemented 
acpiphp fails to power off the slot.  This is happening because 
the current code does not attempt to remove power using the 
_EJ0 ACPI object.  This patch restores the _EJ0 evaluation attempt 
which was apparently inadvertently removed from the power-off
sequence when the _EJ0 evaluation code was relocated from 
power_off_slot() to acpiphp_eject_slot().

Signed-off-by: Gary Hade <garyhade@us.ibm.com>
---

--- linux-2.6.22-rc3/drivers/pci/hotplug/acpiphp.h.orig	2007-06-04 20:17:25.000000000 -0700
+++ linux-2.6.22-rc3/drivers/pci/hotplug/acpiphp.h	2007-06-04 20:39:55.000000000 -0700
@@ -211,6 +211,7 @@ typedef int (*acpiphp_callback)(struct a

 extern int acpiphp_enable_slot (struct acpiphp_slot *slot);
 extern int acpiphp_disable_slot (struct acpiphp_slot *slot);
+extern int acpiphp_eject_slot (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_power_status (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_attention_status (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_latch_status (struct acpiphp_slot *slot);

--- linux-2.6.22-rc3/drivers/pci/hotplug/acpiphp_glue.c.orig	2007-05-25 19:55:14.000000000 -0700
+++ linux-2.6.22-rc3/drivers/pci/hotplug/acpiphp_glue.c	2007-06-04 21:10:25.000000000 -0700
@@ -1282,7 +1282,7 @@ static unsigned int get_slot_status(stru
 /**
  * acpiphp_eject_slot - physically eject the slot
  */
-static int acpiphp_eject_slot(struct acpiphp_slot *slot)
+int acpiphp_eject_slot(struct acpiphp_slot *slot)
 {
 	acpi_status status;
 	struct acpiphp_func *func;

--- linux-2.6.22-rc3/drivers/pci/hotplug/acpiphp_core.c.orig	2007-06-04 20:05:27.000000000 -0700
+++ linux-2.6.22-rc3/drivers/pci/hotplug/acpiphp_core.c	2007-06-04 20:39:46.000000000 -0700
@@ -156,11 +156,15 @@ static int enable_slot(struct hotplug_sl
 static int disable_slot(struct hotplug_slot *hotplug_slot)
 {
 	struct slot *slot = hotplug_slot->private;
+	int retval;

 	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot->name);

 	/* disable the specified slot */
-	return acpiphp_disable_slot(slot->acpi_slot);
+	retval = acpiphp_disable_slot(slot->acpi_slot);
+	if (!retval)
+		retval = acpiphp_eject_slot(slot->acpi_slot);
+	return retval;
 }



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Pcihpd-discuss] what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 18:33     ` Gary Hade
@ 2007-10-01 18:39       ` Gary Hade
  2007-10-01 18:43       ` Kristen Carlson Accardi
  1 sibling, 0 replies; 9+ messages in thread
From: Gary Hade @ 2007-10-01 18:39 UTC (permalink / raw)
  To: Gary Hade; +Cc: Kristen Carlson Accardi, linux-acpi, gregkh, pcihpd-discuss

On Mon, Oct 01, 2007 at 11:33:54AM -0700, Gary Hade wrote:
> On Mon, Oct 01, 2007 at 10:50:07AM -0700, Kristen Carlson Accardi wrote:
> > On Mon, 1 Oct 2007 10:43:41 -0700
> > Gary Hade <garyhade@us.ibm.com> wrote:
> > 
> > > On Mon, Oct 01, 2007 at 10:08:58AM -0700, Kristen Carlson Accardi wrote:
> > > > Hi,
> > > > I notice in acpiphp that the code path for powering down the slot
> > > > via sysfs does not execute the _EJ0 method, but instead simply
> > > > looks for _PS3 and of couse disables all the bridges and devices.  
> > > > I suppose this could be valid depending on your definition of what 
> > > > /sys/bus/pci/slots/<slot_no>/power should do.
> > > > 
> > > > Is it intended to just power down the adapter that's in the slot, 
> > > > or is it intended to make the adapter in the slot able to be removed?
> > > > If it's intended to make the adapter able to be removed, shouldn't
> > > > we be calling _EJ0?
> > > > 
> > > > As a comparison, in pciehp when the sysfs power file is written,
> > > > we do actually go out and send the commands to the hotplug controller
> > > > to physically power off the slot.
> > > 
> > > Kristen, This sounds a lot like the problem I was trying to address 
> > > with the "fix slot poweroff problem on systems without _PS3" patch 
> > > that I posted in June.
> > > http://sourceforge.net/mailarchive/forum.php?thread_name=20070604232736.GA3239%40us.ibm.com&forum_name=pcihpd-discuss
> > > It seems to be working fine on our systems.
> > > 
> > > Gary
> > 
> > ok - hum well, I seem to have not merged that patch for some reason, and
> > it also isn't anywhere in my inbox.  Can you resend it?  
> 
> Included below.
> 
> > Seems like we should get it in there.
> > 
> > BTW - people should copy me directly when they send their patches otherwise
> > they may get overlooked. (not that I'm saying this is what happened here,
> > it also happens sometimes that patches get buried in my inbox despite my
> > best efforts).
> 
> In this case, I did send it directly to you with 
> pcihpd-discuss@lists.sourceforge.net on gregkh@suse.de the

Oops, this line should read:
pcihpd-discuss@lists.sourceforge.net and gregkh@suse.de on the

> CC line.  You and Greg subsequently signed off on it and it 
> entered mainline at 2.6.23-rc1.
            ...

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Pcihpd-discuss] what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do?
  2007-10-01 18:33     ` Gary Hade
  2007-10-01 18:39       ` Gary Hade
@ 2007-10-01 18:43       ` Kristen Carlson Accardi
  1 sibling, 0 replies; 9+ messages in thread
From: Kristen Carlson Accardi @ 2007-10-01 18:43 UTC (permalink / raw)
  To: Gary Hade; +Cc: linux-acpi, gregkh, pcihpd-discuss

On Mon, 1 Oct 2007 11:33:54 -0700
Gary Hade <garyhade@us.ibm.com> wrote:
> 
> In this case, I did send it directly to you with 
> pcihpd-discuss@lists.sourceforge.net on gregkh@suse.de the
> CC line.  You and Greg subsequently signed off on it and it 
> entered mainline at 2.6.23-rc1.
> 
> Gary

Yay!  Problem solved, that was easy, thanks!

Kristen

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2007-10-01 18:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-01 17:08 what is writing to /sys/bus/pci/slot/<slot_no>/power really supposed to do? Kristen Carlson Accardi
2007-10-01 17:19 ` Greg KH
2007-10-01 17:35   ` Kristen Carlson Accardi
2007-10-01 17:46     ` [Pcihpd-discuss] " Matthew Wilcox
2007-10-01 17:43 ` Gary Hade
2007-10-01 17:50   ` Kristen Carlson Accardi
2007-10-01 18:33     ` Gary Hade
2007-10-01 18:39       ` Gary Hade
2007-10-01 18:43       ` Kristen Carlson Accardi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox