RE: [RFC/Patch 0/12] ACPI based root bridge hot-add

RE: [RFC/Patch 0/12] ACPI based root bridge hot-add

[Dely Sy]

On Monday, March 21, 2005 5:07 PM, Dely Sy wrote:
> On Monday, March 21, 2005 10:05 AM, Rajesh Shah wrote:
> > On Fri, Mar 18, 2005 at 09:13:32PM -0800, Greg KH wrote:
> > > 	- Does this break the i386 acpiphp functionality?

> > Dely Sy had tested hotplug with an earlier version of my patches
> > (with minor differences from the current series) on i386 and it
> > worked fine. She probably hasn't tested the latest one. Dely,
> > could you check that please? 

> I tested an earlier version of this patch on my i386 system with
> PCI Express hot-plug slots.  The i386 acpiphp functionality worked
> fine - i.e. I was able to do hot-plug of single- & multi-function 
> cards.  

> I'll check this new patch on my system.

Earlier I reported that Matthew's acpiphp rewrite had problem in 
powering down slot on my i386 system.  The following patch is 
needed to get the acpiphp rewrite properly powering down the slot.
A similar patch was sent out to Matthew for comment and Rajesh 
had tested the patch on Tiger4.

I just did a test of Rajesh's latest patch on 2.6.11.5 with
Wilcox's acpiphp rewrite and the following patch.  Hot-plug of 
PCI Express card worked fine on my i386 system
 
Thanks,
Dely

Signed-off-by: Dely Sy <dely.l.sy@intel.com>

diff -urpN linux-2.6.11.5rbha/drivers/pci/hotplug/acpiphp_glue.c linux-2.6.11.5rbhatst/drivers/pci/hotplug/acpiphp_glue.c
--- linux-2.6.11.5rbha/drivers/pci/hotplug/acpiphp_glue.c	2005-03-22 00:56:04.000000000 -0800
+++ linux-2.6.11.5rbhatst/drivers/pci/hotplug/acpiphp_glue.c	2005-03-22 23:21:23.000000000 -0800
@@ -586,7 +586,7 @@ static int power_off_slot(struct acpiphp
 	list_for_each (l, &slot->funcs) {
 		func = list_entry(l, struct acpiphp_func, sibling);
 
-		if (func->pci_dev && (func->flags & FUNC_HAS_PS3)) {
+		if (func->flags & FUNC_HAS_PS3) {
 			status = acpi_evaluate_object(func->handle, "_PS3", NULL, NULL);
 			if (ACPI_FAILURE(status)) {
 				warn("%s: _PS3 failed\n", __FUNCTION__);
@@ -601,7 +601,7 @@ static int power_off_slot(struct acpiphp
 		func = list_entry(l, struct acpiphp_func, sibling);
 
 		/* We don't want to call _EJ0 on non-existing functions. */
-		if (func->pci_dev && (func->flags & FUNC_HAS_EJ0)) {
+		if (func->flags & FUNC_HAS_EJ0) {
 			/* _EJ0 method take one argument */
 			arg_list.count = 1;
 			arg_list.pointer = &arg;

Re: [Pcihpd-discuss] RE: [RFC/Patch 0/12] ACPI based root bridge hot-add

[Tom Duffy]

[-- Attachment #1: Type: text/plain, Size: 2209 bytes --]

On Tue, 2005-03-22 at 19:13 -0800, Dely Sy wrote:
> I just did a test of Rajesh's latest patch on 2.6.11.5 with
> Wilcox's acpiphp rewrite and the following patch.  Hot-plug of 
> PCI Express card worked fine on my i386 system

I have updated to Wilcox's rewrite, Rajesh's stuff, and Dely's latest
patch.  Still seeing these:

[root@intlhotp-1 4]# uname -a
Linux intlhotp-1 2.6.11andro #5 SMP Wed Mar 23 12:00:56 PST 2005 x86_64
x86_64 x86_64 GNU/Linux

[root@intlhotp-1 ~]# modprobe acpiphp
acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5
acpiphp: Slot [4] registered
acpiphp: Slot [3] registered

[root@intlhotp-1 ~]# cd /sys/bus/pci/slots/4

[root@intlhotp-1 4]# echo 0 > power

This *does* take this slot off line.  Before, I see in lscpi
08:00.0 Ethernet controller: Intel Corp.: Unknown device 105f (rev 03)
08:00.1 Ethernet controller: Intel Corp.: Unknown device 105f (rev 03)
After, these are gone.

[root@intlhotp-1 4]# cat power
1

Hrm, this should be 0.

[root@intlhotp-1 4]# echo 1 > power
acpiphp_glue: No new device found

There is a card plugged into that slot.

Here is some info from the BIOS:

[root@intlhotp-1 4]# dmidecode
# dmidecode 2.2
SMBIOS 2.3 present.
91 structures occupying 3496 bytes.
Table at 0x000FCA80.
Handle 0x0000
        DMI type 0, 20 bytes.
        BIOS Information
                Vendor: American Megatrends Inc.
                Version: SE7520AF20.86B.P.03.00.0085.092420041113
                Release Date: 09/24/2004

<snip>

Handle 0x0002
        DMI type 2, 15 bytes.
        Base Board Information
                Manufacturer: Intel
                Product Name: SE7520AF2HP
                Version: FRU Ver 0.3

<snip>

Handle 0x0034
        DMI type 9, 13 bytes.
        System Slot Information
                Designation: Slot 4 (PCI_Express x8)
                Type: 64-bit PCI
                Current Usage: Available
                Length: Short
                ID: 3
                Characteristics:
                        3.3 V is provided
                        Opening is shared
                        PME signal is supported

Any ideas?

Thanks,

-tduffy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Pcihpd-discuss] RE: [RFC/Patch 0/12] ACPI based root bridge hot-add

[Rajesh Shah]

On Wed, Mar 23, 2005 at 03:03:16PM -0800, Tom Duffy wrote:
> 
> I have updated to Wilcox's rewrite, Rajesh's stuff, and Dely's latest
> patch.  Still seeing these:
> 
> [root@intlhotp-1 ~]# modprobe acpiphp
> acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5
> acpiphp: Slot [4] registered
> acpiphp: Slot [3] registered
> 
> [root@intlhotp-1 ~]# cd /sys/bus/pci/slots/4
> 
> [root@intlhotp-1 4]# echo 0 > power
> 
> This *does* take this slot off line.  Before, I see in lscpi

Note that the current code also attempts to physically eject the
card when you power it off like this. 

> After, these are gone.
> 
> [root@intlhotp-1 4]# cat power
> 1
> 
> Hrm, this should be 0.
> 
The code invokes an ACPI _STAtus method to report the power state.
It can get confused if that reports that the device is still
present. The code does keep internal info about the power state
of each slot, but it doesn't use it to report power state.

> [root@intlhotp-1 4]# echo 1 > power
> acpiphp_glue: No new device found
> 
If the firmware had a working _EJ0 method for this slot, the
card has been physically ejected.

Does this patch help? It decouples power changes from physical
ejection. With this, you can use the sysfs power control file
to repeatedly power a device on and off. You have to use an
acpi button press to actually physically eject it. Patch 
applies on 2.6.12-rc1-mm4 on top of the acpiphp re-write
plus all the other new patches.

-----------

Current acpiphp code does not distinguish between the physical
presence and power state of a device/slot. That is, if a device
has to be disabled, it also tries to physically ejects the device.
This patch decouples power state from physical presence. You can
now echo to the corresponding sysfs power control file to 
repeatedly enable and disable a device without having to
physically re-insert it. 

Signed-off-by: Rajesh Shah <rajesh.shah@intel.com>

---

 linux-2.6.12-rc1-mm4-rshah1/drivers/pci/hotplug/acpiphp_glue.c |   69 +++++-----
 1 files changed, 38 insertions(+), 31 deletions(-)

diff -puN drivers/pci/hotplug/acpiphp_glue.c~acpiphp-decouple-power-eject drivers/pci/hotplug/acpiphp_glue.c
--- linux-2.6.12-rc1-mm4/drivers/pci/hotplug/acpiphp_glue.c~acpiphp-decouple-power-eject	2005-03-31 13:10:24.846754032 -0800
+++ linux-2.6.12-rc1-mm4-rshah1/drivers/pci/hotplug/acpiphp_glue.c	2005-03-31 13:12:20.483471366 -0800
@@ -592,8 +592,6 @@ static int power_off_slot(struct acpiphp
 	acpi_status status;
 	struct acpiphp_func *func;
 	struct list_head *l;
-	struct acpi_object_list arg_list;
-	union acpi_object arg;
 
 	int retval = 0;
 
@@ -615,27 +613,6 @@ static int power_off_slot(struct acpiphp
 		}
 	}
 
-	list_for_each (l, &slot->funcs) {
-		func = list_entry(l, struct acpiphp_func, sibling);
-
-		/* We don't want to call _EJ0 on non-existing functions. */
-		if (func->flags & FUNC_HAS_EJ0) {
-			/* _EJ0 method take one argument */
-			arg_list.count = 1;
-			arg_list.pointer = &arg;
-			arg.type = ACPI_TYPE_INTEGER;
-			arg.integer.value = 1;
-
-			status = acpi_evaluate_object(func->handle, "_EJ0", &arg_list, NULL);
-			if (ACPI_FAILURE(status)) {
-				warn("%s: _EJ0 failed\n", __FUNCTION__);
-				retval = -1;
-				goto err_exit;
-			} else
-				break;
-		}
-	}
-
 	/* TBD: evaluate _STA to check if the slot is disabled */
 
 	slot->flags &= (~SLOT_POWEREDON);
@@ -782,6 +759,39 @@ static unsigned int get_slot_status(stru
 }
 
 /**
+ * acpiphp_eject_slot - physically eject the slot
+ */
+static int acpiphp_eject_slot(struct acpiphp_slot *slot)
+{
+	acpi_status status;
+	struct acpiphp_func *func;
+	struct list_head *l;
+	struct acpi_object_list arg_list;
+	union acpi_object arg;
+
+	list_for_each (l, &slot->funcs) {
+		func = list_entry(l, struct acpiphp_func, sibling);
+
+		/* We don't want to call _EJ0 on non-existing functions. */
+		if ((func->flags & FUNC_HAS_EJ0)) {
+			/* _EJ0 method take one argument */
+			arg_list.count = 1;
+			arg_list.pointer = &arg;
+			arg.type = ACPI_TYPE_INTEGER;
+			arg.integer.value = 1;
+
+			status = acpi_evaluate_object(func->handle, "_EJ0", &arg_list, NULL);
+			if (ACPI_FAILURE(status)) {
+				warn("%s: _EJ0 failed\n", __FUNCTION__);
+				return -1;
+			} else
+				break;
+		}
+	}
+	return 0;
+}
+
+/**
  * acpiphp_check_bridge - re-enumerate devices
  *
  * Iterate over all slots under this bridge and make sure that if a
@@ -804,6 +814,8 @@ static int acpiphp_check_bridge(struct a
 			if (retval) {
 				err("Error occurred in disabling\n");
 				goto err_exit;
+			} else {
+				acpiphp_eject_slot(slot);
 			}
 			disabled++;
 		} else {
@@ -1041,7 +1053,6 @@ static void handle_hotplug_event_bridge(
 	}
 }
 
-
 /**
  * handle_hotplug_event_func - handle ACPI event on functions (i.e. slots)
  *
@@ -1084,7 +1095,8 @@ static void handle_hotplug_event_func(ac
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		/* request device eject */
 		dbg("%s: Device eject notify on %s\n", __FUNCTION__, objname);
-		acpiphp_disable_slot(func->slot);
+		if (!(acpiphp_disable_slot(func->slot)))
+			acpiphp_eject_slot(func->slot);
 		break;
 
 	default:
@@ -1268,7 +1280,6 @@ int acpiphp_enable_slot(struct acpiphp_s
 	return retval;
 }
 
-
 /**
  * acpiphp_disable_slot - power off slot
  */
@@ -1300,11 +1311,7 @@ int acpiphp_disable_slot(struct acpiphp_
  */
 u8 acpiphp_get_power_status(struct acpiphp_slot *slot)
 {
-	unsigned int sta;
-
-	sta = get_slot_status(slot);
-
-	return (sta & ACPI_STA_ENABLED) ? 1 : 0;
+	return (slot->flags & SLOT_POWEREDON);
 }
 
 
_

Re: [Pcihpd-discuss] RE: [RFC/Patch 0/12] ACPI based root bridge hot-add

[Tom Duffy]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

On Thu, 2005-03-31 at 14:03 -0800, Rajesh Shah wrote:
> Does this patch help?

YES!  I can now power down the slot, see it gone from pci list, reenable
it, etc.  Awesome.  Thank you.

[root@intlhotp-1 ~]# lspci -s 08:00
08:00.0 Ethernet controller: Intel Corp.: Unknown device 105f (rev 03)
08:00.1 Ethernet controller: Intel Corp.: Unknown device 105f (rev 03)
[root@intlhotp-1 ~]# cd /sys/bus/pci/slots/4/
[root@intlhotp-1 4]# cat power
1
[root@intlhotp-1 4]# echo 0 > power
[root@intlhotp-1 4]# lspci -s 08:00
[root@intlhotp-1 4]# cat power
0
[root@intlhotp-1 4]# echo 1 > power
[root@intlhotp-1 4]# lspci -s 08:00
08:00.0 Ethernet controller: Intel Corp.: Unknown device 105f (rev 03)
08:00.1 Ethernet controller: Intel Corp.: Unknown device 105f (rev 03)


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]