<html><head><meta name="color-scheme" content="light dark"></head><body><pre style="word-wrap: break-word; white-space: pre-wrap;">ChangeSet 1.1517, 2004/01/29 15:34:56-08:00, t-kochi@bq.jp.nec.com

[PATCH] PCI Hotplug: add address file and fix acpiphp bugs

This is the pending patch that adds 'address' file to show
PCI-address and a few other minor fixes.
As 2.6.0 is out, I'm resending the patch.
Would you mind taking this?

&gt; &gt; &gt; Thanks.  I had a little time to try your patch today.  Sorry
&gt; &gt; &gt; to report that it isn't working for me.
&gt; &gt; &gt;
&gt; &gt; &gt; I first powered off (successfully the 1st time) a populated slot
&gt; &gt; &gt; and removed and reinserted the card into the same slot.  The slot
&gt; &gt; &gt; powered back up but I was then unable to power it off.  I believe
&gt; &gt; &gt; the following instruction that still exists in power_off_slot()
&gt; &gt; &gt; may be preventing the slot from being powered off more than once.
&gt; &gt; &gt;     func-&gt;flags &amp;= (~FUNC_EXISTS);
&gt; &gt; &gt;
&gt; &gt; &gt; I then tried to insert an adapter in an un-populated slot.  For
&gt; &gt; &gt; some reason (which I don't understand yet) there was an enabling
&gt; &gt; &gt; error which I believe caused enable_device() to exit via a path
&gt; &gt; &gt; that bypassed the instruction that sets the FUNC_EXISTS flag.
&gt; &gt; &gt; I was then unable to power off the slot which I believe was due
&gt; &gt; &gt; to the FUNC_EXISTS flag not being set.
&gt; &gt; &gt;
&gt; &gt; &gt; I didn't have time to definitely confirmed the above theories.
&gt; &gt; &gt; I'll take a closer look at this tomorrow unless you are able
&gt; &gt; &gt; to diagnose using my vague clues :)
&gt; &gt;
&gt; &gt; It turns out that both of the above mentioned problems happened
&gt; &gt; because the call to acpiphp_configure_slot() from enable_device()
&gt; &gt; failed after inserting the card.  When this happens enable_device()
&gt; &gt; exits without setting the FUNC_EXISTS flag for any of the slot
&gt; &gt; functions.  Subsequent attempts to power off the same slot fail
&gt; &gt; when power_off_slot() is unable to locate a function with both
&gt; &gt; FUNC_HAS_EJ0 and FUNC_EXISTS flags set.
&gt; &gt;
&gt; &gt; The patch works okay when using a card that allows
&gt; &gt; acpiphp_configure_slot() to succeed but I believe it should
&gt; &gt; be improved to allow the slot to be powered off following
&gt; &gt; device enablement errors.
&gt;
&gt; Thanks for testing and comments.
&gt; I really appreciate it.
&gt;
&gt; This problem turned out to be somewhat fragile state
&gt; transition:
&gt;
&gt; a lifecycle of a slot is (if there's no error)
&gt;
&gt;   function             state
&gt; ----------------------------------------------------
&gt; 0                      nothing
&gt; 1  power_on_slot()  -&gt; SLOT_POWERDON
&gt; 2  enable_device()  -&gt; SLOT_POWEREDON + SLOT_ENABLED
&gt; 3  disable_device() -&gt; SLOT_POWEREDON
&gt; 4  power_off_slot() -&gt; nothing
&gt;
&gt; but if any error occur during enable_device(), slot will remain
&gt; SLOT_POWERDON, but some functions on the card may not have
&gt; FUNC_EXISTS flags, which will eventually prevents powering
&gt; off in power_off_slot(), state transition from 1 to 4 directly.
&gt; I.e, the FUNC_EXISTS flag introduced more states to
&gt; complicate things.
&gt;
&gt; The FUNC_EXISTS flag was introduced after some discussion
&gt; between me and Irene Zubarev, but it has no more meaning
&gt; than that the function has corresponding 'pci_dev' structure.
&gt; So I eliminated the usage of FUNC_EXISTS and the result is
&gt; the patches attached to this mail (for both 2.4 and 2.6.
&gt; I think Greg already applied the 2.4 'cleanup' patch to his tree,
&gt; but it's not in Marcelo's release so I'm re-attaching to
&gt; this mail for anyone interested in this topic.  It's identical
&gt; to the one I posted earlier).
&gt; These patches don't include Gary's patch in his post last week,
&gt; so please apply separately.
&gt;
&gt; Please note that current acpiphp driver cannot handle a
&gt; PCI card that has a PCI-to-PCI bridge on it (support
&gt; for such cards is incomplete).  But if it's treated as
&gt; an error, it should be recoverable anyway.


 drivers/pci/hotplug/acpiphp.h          |    5 +--
 drivers/pci/hotplug/acpiphp_core.c     |   30 +++++++++++++++++++++--
 drivers/pci/hotplug/acpiphp_glue.c     |   32 +++++++++++++++++--------
 drivers/pci/hotplug/acpiphp_pci.c      |    6 ++--
 drivers/pci/hotplug/acpiphp_res.c      |    3 --
 drivers/pci/hotplug/pci_hotplug.h      |    6 ++++
 drivers/pci/hotplug/pci_hotplug_core.c |   42 +++++++++++++++++++++++++++++++++
 7 files changed, 103 insertions(+), 21 deletions(-)


diff -Nru a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
--- a/drivers/pci/hotplug/acpiphp.h	Thu Jan 29 17:24:14 2004
+++ b/drivers/pci/hotplug/acpiphp.h	Thu Jan 29 17:24:14 2004
@@ -201,7 +201,7 @@
 
 #define SLOT_POWEREDON		(0x00000001)
 #define SLOT_ENABLED		(0x00000002)
-#define SLOT_MULTIFUNCTION	(x000000004)
+#define SLOT_MULTIFUNCTION	(0x00000004)
 
 /* function flags */
 
@@ -212,8 +212,6 @@
 #define FUNC_HAS_PS2		(0x00000040)
 #define FUNC_HAS_PS3		(0x00000080)
 
-#define FUNC_EXISTS		(0x10000000) /* to make sure we call _EJ0 only for existing funcs */
-
 /* function prototypes */
 
 /* acpiphp_glue.c */
@@ -231,6 +229,7 @@
 extern u8 acpiphp_get_attention_status (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_latch_status (struct acpiphp_slot *slot);
 extern u8 acpiphp_get_adapter_status (struct acpiphp_slot *slot);
+extern u32 acpiphp_get_address (struct acpiphp_slot *slot);
 
 /* acpiphp_pci.c */
 extern struct pci_dev *acpiphp_allocate_pcidev (struct pci_bus *pbus, int dev, int fn);
diff -Nru a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
--- a/drivers/pci/hotplug/acpiphp_core.c	Thu Jan 29 17:24:14 2004
+++ b/drivers/pci/hotplug/acpiphp_core.c	Thu Jan 29 17:24:14 2004
@@ -30,14 +30,14 @@
  *
  */
 
-#include &lt;linux/config.h&gt;
-#include &lt;linux/kernel.h&gt;
+#include &lt;linux/init.h&gt;
 #include &lt;linux/module.h&gt;
+
+#include &lt;linux/kernel.h&gt;
 #include &lt;linux/pci.h&gt;
 #include &lt;linux/slab.h&gt;
 #include &lt;linux/smp.h&gt;
 #include &lt;linux/smp_lock.h&gt;
-#include &lt;linux/init.h&gt;
 #include "pci_hotplug.h"
 #include "acpiphp.h"
 
@@ -71,6 +71,7 @@
 static int hardware_test	(struct hotplug_slot *slot, u32 value);
 static int get_power_status	(struct hotplug_slot *slot, u8 *value);
 static int get_attention_status	(struct hotplug_slot *slot, u8 *value);
+static int get_address		(struct hotplug_slot *slot, u32 *value);
 static int get_latch_status	(struct hotplug_slot *slot, u8 *value);
 static int get_adapter_status	(struct hotplug_slot *slot, u8 *value);
 static int get_max_bus_speed	(struct hotplug_slot *hotplug_slot, enum pci_bus_speed *value);
@@ -86,6 +87,7 @@
 	.get_attention_status	= get_attention_status,
 	.get_latch_status	= get_latch_status,
 	.get_adapter_status	= get_adapter_status,
+	.get_address		= get_address,
 	.get_max_bus_speed	= get_max_bus_speed,
 	.get_cur_bus_speed	= get_cur_bus_speed,
 };
@@ -317,6 +319,28 @@
 	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot-&gt;name);
 
 	*value = acpiphp_get_adapter_status(slot-&gt;acpi_slot);
+
+	return retval;
+}
+
+
+/**
+ * get_address - get pci address of a slot
+ * @hotplug_slot: slot to get status
+ * @busdev: pointer to struct pci_busdev (seg, bus, dev)
+ *
+ */
+static int get_address (struct hotplug_slot *hotplug_slot, u32 *value)
+{
+	struct slot *slot = get_slot(hotplug_slot, __FUNCTION__);
+	int retval = 0;
+
+	if (slot == NULL)
+		return -ENODEV;
+
+	dbg("%s - physical_slot = %s\n", __FUNCTION__, hotplug_slot-&gt;name);
+
+	*value = acpiphp_get_address(slot-&gt;acpi_slot);
 
 	return retval;
 }
diff -Nru a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
--- a/drivers/pci/hotplug/acpiphp_glue.c	Thu Jan 29 17:24:14 2004
+++ b/drivers/pci/hotplug/acpiphp_glue.c	Thu Jan 29 17:24:14 2004
@@ -26,12 +26,12 @@
  *
  */
 
-#include &lt;linux/config.h&gt;
-#include &lt;linux/kernel.h&gt;
+#include &lt;linux/init.h&gt;
 #include &lt;linux/module.h&gt;
+
+#include &lt;linux/kernel.h&gt;
 #include &lt;linux/pci.h&gt;
 #include &lt;linux/smp_lock.h&gt;
-#include &lt;linux/init.h&gt;
 #include &lt;asm/semaphore.h&gt;
 
 #include "../pci.h"
@@ -686,7 +686,7 @@
 	struct list_head *l;
 	int retval = 0;
 
-	/* is this already enabled? */
+	/* if already enabled, just skip */
 	if (slot-&gt;flags &amp; SLOT_POWEREDON)
 		goto err_exit;
 
@@ -724,14 +724,14 @@
 
 	int retval = 0;
 
-	/* is this already enabled? */
+	/* if already disabled, just skip */
 	if ((slot-&gt;flags &amp; SLOT_POWEREDON) == 0)
 		goto err_exit;
 
 	list_for_each (l, &amp;slot-&gt;funcs) {
 		func = list_entry(l, struct acpiphp_func, sibling);
 
-		if (func-&gt;flags &amp; (FUNC_HAS_PS3 | FUNC_EXISTS)) {
+		if (func-&gt;pci_dev &amp;&amp; (func-&gt;flags &amp; FUNC_HAS_PS3)) {
 			status = acpi_evaluate_object(func-&gt;handle, "_PS3", NULL, NULL);
 			if (ACPI_FAILURE(status)) {
 				warn("%s: _PS3 failed\n", __FUNCTION__);
@@ -745,7 +745,7 @@
 		func = list_entry(l, struct acpiphp_func, sibling);
 
 		/* We don't want to call _EJ0 on non-existing functions. */
-		if (func-&gt;flags &amp; (FUNC_HAS_EJ0 | FUNC_EXISTS)) {
+		if (func-&gt;pci_dev &amp;&amp; (func-&gt;flags &amp; FUNC_HAS_EJ0)) {
 			/* _EJ0 method take one argument */
 			arg_list.count = 1;
 			arg_list.pointer = &amp;arg;
@@ -758,7 +758,6 @@
 				retval = -1;
 				goto err_exit;
 			}
-			func-&gt;flags &amp;= (~FUNC_EXISTS);
 		}
 	}
 
@@ -838,8 +837,6 @@
 		retval = acpiphp_configure_function(func);
 		if (retval)
 			goto err_exit;
-
-		func-&gt;flags |= FUNC_EXISTS;
 	}
 
 	slot-&gt;flags |= SLOT_ENABLED;
@@ -1348,4 +1345,19 @@
 	sta = get_slot_status(slot);
 
 	return (sta == 0) ? 0 : 1;
+}
+
+
+/*
+ * pci address (seg/bus/dev)
+ */
+u32 acpiphp_get_address (struct acpiphp_slot *slot)
+{
+	u32 address;
+
+	address = ((slot-&gt;bridge-&gt;seg) &lt;&lt; 16) |
+		  ((slot-&gt;bridge-&gt;bus) &lt;&lt; 8) |
+		  slot-&gt;device;
+
+	return address;
 }
diff -Nru a/drivers/pci/hotplug/acpiphp_pci.c b/drivers/pci/hotplug/acpiphp_pci.c
--- a/drivers/pci/hotplug/acpiphp_pci.c	Thu Jan 29 17:24:14 2004
+++ b/drivers/pci/hotplug/acpiphp_pci.c	Thu Jan 29 17:24:14 2004
@@ -29,11 +29,11 @@
  *
  */
 
-#include &lt;linux/config.h&gt;
-#include &lt;linux/kernel.h&gt;
+#include &lt;linux/init.h&gt;
 #include &lt;linux/module.h&gt;
+
+#include &lt;linux/kernel.h&gt;
 #include &lt;linux/pci.h&gt;
-#include &lt;linux/init.h&gt;
 #include &lt;linux/acpi.h&gt;
 #include "../pci.h"
 #include "pci_hotplug.h"
diff -Nru a/drivers/pci/hotplug/acpiphp_res.c b/drivers/pci/hotplug/acpiphp_res.c
--- a/drivers/pci/hotplug/acpiphp_res.c	Thu Jan 29 17:24:14 2004
+++ b/drivers/pci/hotplug/acpiphp_res.c	Thu Jan 29 17:24:14 2004
@@ -29,7 +29,7 @@
  *
  */
 
-#include &lt;linux/config.h&gt;
+#include &lt;linux/init.h&gt;
 #include &lt;linux/module.h&gt;
 
 #include &lt;linux/kernel.h&gt;
@@ -39,7 +39,6 @@
 #include &lt;linux/pci.h&gt;
 #include &lt;linux/smp.h&gt;
 #include &lt;linux/smp_lock.h&gt;
-#include &lt;linux/init.h&gt;
 
 #include &lt;linux/string.h&gt;
 #include &lt;linux/mm.h&gt;
diff -Nru a/drivers/pci/hotplug/pci_hotplug.h b/drivers/pci/hotplug/pci_hotplug.h
--- a/drivers/pci/hotplug/pci_hotplug.h	Thu Jan 29 17:24:14 2004
+++ b/drivers/pci/hotplug/pci_hotplug.h	Thu Jan 29 17:24:14 2004
@@ -74,6 +74,9 @@
  * @get_adapter_status: Called to get see if an adapter is present in the slot or not.
  *	If this field is NULL, the value passed in the struct hotplug_slot_info
  *	will be used when this value is requested by a user.
+ * @get_address: Called to get pci address of a slot.
+ *	If this field is NULL, the value passed in the struct hotplug_slot_info
+ *	will be used when this value is requested by a user.
  * @get_max_bus_speed: Called to get the max bus speed for a slot.
  *	If this field is NULL, the value passed in the struct hotplug_slot_info
  *	will be used when this value is requested by a user.
@@ -96,6 +99,7 @@
 	int (*get_attention_status)	(struct hotplug_slot *slot, u8 *value);
 	int (*get_latch_status)		(struct hotplug_slot *slot, u8 *value);
 	int (*get_adapter_status)	(struct hotplug_slot *slot, u8 *value);
+	int (*get_address)		(struct hotplug_slot *slot, u32 *value);
 	int (*get_max_bus_speed)	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 	int (*get_cur_bus_speed)	(struct hotplug_slot *slot, enum pci_bus_speed *value);
 };
@@ -106,6 +110,7 @@
  * @attention_status: if the attention light is enabled or not (1/0)
  * @latch_status: if the latch (if any) is open or closed (1/0)
  * @adapter_present: if there is a pci board present in the slot or not (1/0)
+ * @address: (domain &lt;&lt; 16 | bus &lt;&lt; 8 | dev)
  *
  * Used to notify the hotplug pci core of the status of a specific slot.
  */
@@ -114,6 +119,7 @@
 	u8	attention_status;
 	u8	latch_status;
 	u8	adapter_status;
+	u32	address;
 	enum pci_bus_speed	max_bus_speed;
 	enum pci_bus_speed	cur_bus_speed;
 };
diff -Nru a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
--- a/drivers/pci/hotplug/pci_hotplug_core.c	Thu Jan 29 17:24:14 2004
+++ b/drivers/pci/hotplug/pci_hotplug_core.c	Thu Jan 29 17:24:14 2004
@@ -159,6 +159,7 @@
 GET_STATUS(attention_status, u8)
 GET_STATUS(latch_status, u8)
 GET_STATUS(adapter_status, u8)
+GET_STATUS(address, u32)
 GET_STATUS(max_bus_speed, enum pci_bus_speed)
 GET_STATUS(cur_bus_speed, enum pci_bus_speed)
 
@@ -302,6 +303,28 @@
 	.show = presence_read_file,
 };
 
+static ssize_t address_read_file (struct hotplug_slot *slot, char *buf)
+{
+	int retval;
+	u32 address;
+
+	retval = get_address (slot, &amp;address);
+	if (retval)
+		goto exit;
+	retval = sprintf (buf, "%04x:%02x:%02x\n",
+			  (address &gt;&gt; 16) &amp; 0xffff,
+			  (address &gt;&gt; 8) &amp; 0xff,
+			  address &amp; 0xff);
+
+exit:
+	return retval;
+}
+
+static struct hotplug_slot_attribute hotplug_slot_attr_address = {
+	.attr = {.name = "address", .mode = S_IFREG | S_IRUGO},
+	.show = address_read_file,
+};
+
 static char *unknown_speed = "Unknown bus speed";
 
 static ssize_t max_bus_speed_read_file (struct hotplug_slot *slot, char *buf)
@@ -425,6 +448,15 @@
 	return -ENOENT;
 }
 
+static int has_address_file (struct hotplug_slot *slot)
+{
+	if ((!slot) || (!slot-&gt;ops))
+		return -ENODEV;
+	if (slot-&gt;ops-&gt;get_address)
+		return 0;
+	return -ENOENT;
+}
+
 static int has_max_bus_speed_file (struct hotplug_slot *slot)
 {
 	if ((!slot) || (!slot-&gt;ops))
@@ -466,6 +498,9 @@
 	if (has_adapter_file(slot) == 0)
 		sysfs_create_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_presence.attr);
 
+	if (has_address_file(slot) == 0)
+		sysfs_create_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_address.attr);
+
 	if (has_max_bus_speed_file(slot) == 0)
 		sysfs_create_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_max_bus_speed.attr);
 
@@ -492,6 +527,9 @@
 	if (has_adapter_file(slot) == 0)
 		sysfs_remove_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_presence.attr);
 
+	if (has_address_file(slot) == 0)
+		sysfs_remove_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_address.attr);
+
 	if (has_max_bus_speed_file(slot) == 0)
 		sysfs_remove_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_max_bus_speed.attr);
 
@@ -611,6 +649,10 @@
 	if ((has_adapter_file(slot) == 0) &amp;&amp;
 	    (slot-&gt;info-&gt;adapter_status != info-&gt;adapter_status))
 		sysfs_update_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_presence.attr);
+
+	if ((has_address_file(slot) == 0) &amp;&amp;
+	    (slot-&gt;info-&gt;address != info-&gt;address))
+		sysfs_update_file(&amp;slot-&gt;kobj, &amp;hotplug_slot_attr_address.attr);
 
 	if ((has_max_bus_speed_file(slot) == 0) &amp;&amp;
 	    (slot-&gt;info-&gt;max_bus_speed != info-&gt;max_bus_speed))
</pre></body></html>