RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Li, Shaohua]

Hi,
>
>If an adapter is surprise removed, the interrupt pin must be guessed,
as
>any attempts to read it would obviously be invalid.  cycle through all
>possible interrupt pin values until we can either lookup or derive the
>right irq to disable.
>
>Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
>
>diff -uprN -X linux-2.6.14-rc2/Documentation/dontdiff linux-2.6.14-
>rc2/drivers/acpi/pci_irq.c linux-2.6.14-rc2-kca1/drivers/acpi/pci_irq.c
>--- linux-2.6.14-rc2/drivers/acpi/pci_irq.c	2005-09-27
>09:01:28.000000000 -0700
>+++ linux-2.6.14-rc2-kca1/drivers/acpi/pci_irq.c	2005-09-28
>10:40:57.000000000 -0700
>@@ -491,6 +491,79 @@ void __attribute__ ((weak)) acpi_unregis
> {
> }
>
>+
>+
>+/*
>+ * This function will be called only in the case of
>+ * a "surprise" hot plug removal.  For surprise removals,
>+ * the card has either already be yanked out of the slot, or
>+ * the slot's been powered off, so we have to brute force
>+ * our way through all the possible interrupt pins to derive
>+ * the GSI, then we double check with the value stored in the
>+ * pci_dev structure to make sure we have the GSI that belongs
>+ * to this IRQ.
>+ */
>+void acpi_pci_irq_disable_nodev(struct pci_dev *dev)
>+{
>+	int gsi = 0;
>+	u8  pin = 0;
>+	int edge_level = ACPI_LEVEL_SENSITIVE;
>+	int active_high_low = ACPI_ACTIVE_LOW;
>+	int irq;
>+
>+	/*
>+	 * since our device is not present, we
>+	 * can't just read the interrupt pin
>+	 * and use the value to derive the irq.
>+	 * in this case, we are going to check
>+	 * each returned irq value to make
>+	 * sure it matches our already assigned
>+	 * irq before we use it.
>+	 */
>+	for (pin = 0; pin < 4; pin++) {
>+		/*
>+	 	 * First we check the PCI IRQ routing table (PRT) for an
IRQ.
>+	 	 */
>+		gsi = acpi_pci_irq_lookup(dev->bus,
PCI_SLOT(dev->devfn), pin,
>+				  &edge_level, &active_high_low, NULL,
>+				  acpi_pci_free_irq);
acpi_pci_free_irq has side effect. In the link device case, it
deferences a count. The blind guess will mass the reference count. Could
you introduce something like 'acpi_pci_find_irq'?

Thanks,
Shaohua 

RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Kristen Accardi]

On Fri, 2005-10-07 at 17:56 -0700, Li, Shaohua wrote:
> Hi,
> >
> >If an adapter is surprise removed, the interrupt pin must be guessed,
> as
> >any attempts to read it would obviously be invalid.  cycle through all
> >possible interrupt pin values until we can either lookup or derive the
> >right irq to disable.
> >
> >Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> >diff -uprN -X linux-2.6.14-rc2/Documentation/dontdiff linux-2.6.14-
> >rc2/drivers/acpi/pci_irq.c linux-2.6.14-rc2-kca1/drivers/acpi/pci_irq.c
> >--- linux-2.6.14-rc2/drivers/acpi/pci_irq.c	2005-09-27
> >09:01:28.000000000 -0700
> >+++ linux-2.6.14-rc2-kca1/drivers/acpi/pci_irq.c	2005-09-28
> >10:40:57.000000000 -0700
> >@@ -491,6 +491,79 @@ void __attribute__ ((weak)) acpi_unregis
> > {
> > }
> >
> >+
> >+
> >+/*
> >+ * This function will be called only in the case of
> >+ * a "surprise" hot plug removal.  For surprise removals,
> >+ * the card has either already be yanked out of the slot, or
> >+ * the slot's been powered off, so we have to brute force
> >+ * our way through all the possible interrupt pins to derive
> >+ * the GSI, then we double check with the value stored in the
> >+ * pci_dev structure to make sure we have the GSI that belongs
> >+ * to this IRQ.
> >+ */
> >+void acpi_pci_irq_disable_nodev(struct pci_dev *dev)
> >+{
> >+	int gsi = 0;
> >+	u8  pin = 0;
> >+	int edge_level = ACPI_LEVEL_SENSITIVE;
> >+	int active_high_low = ACPI_ACTIVE_LOW;
> >+	int irq;
> >+
> >+	/*
> >+	 * since our device is not present, we
> >+	 * can't just read the interrupt pin
> >+	 * and use the value to derive the irq.
> >+	 * in this case, we are going to check
> >+	 * each returned irq value to make
> >+	 * sure it matches our already assigned
> >+	 * irq before we use it.
> >+	 */
> >+	for (pin = 0; pin < 4; pin++) {
> >+		/*
> >+	 	 * First we check the PCI IRQ routing table (PRT) for an
> IRQ.
> >+	 	 */
> >+		gsi = acpi_pci_irq_lookup(dev->bus,
> PCI_SLOT(dev->devfn), pin,
> >+				  &edge_level, &active_high_low, NULL,
> >+				  acpi_pci_free_irq);
> acpi_pci_free_irq has side effect. In the link device case, it
> deferences a count. The blind guess will mass the reference count. Could
> you introduce something like 'acpi_pci_find_irq'?
> 
> Thanks,
> Shaohua 

Is the ref count decrement in pci-link.c in this section of code:
#ifdef  FUTURE_USE
        /*
         * The Link reference count allows us to _DISable an unused link
         * and suspend time, and set it again  on resume.
         * However, 2.6.12 still has irq_router.resume
         * which blindly restores the link state.
         * So we disable the reference count method
         * to prevent duplicate acpi_pci_link_set()
         * which would harm some systems
         */
        link->refcnt--;
#endif


Or is it somewhere else?  Just want to make sure I know where I need to
avoid calling into.




-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl

RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Kristen Accardi]

For surprise hotplug removal, the interrupt pin must be guessed, as any
attempts to read it would obviously be invalid.  This patch adds a new
function to cycle through all possible pin values, and tries to either
lookup or derive the right irq to disable.  It also adds a new function
to acpi which can be used to just find the irq, without freeing it, just
in case we guess the wrong pin.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

diff -uprN -X linux-2.6.14-rc3.orig/Documentation/dontdiff linux-2.6.14-rc3.orig/drivers/acpi/pci_irq.c linux-2.6.14-rc3/drivers/acpi/pci_irq.c
--- linux-2.6.14-rc3.orig/drivers/acpi/pci_irq.c	2005-10-07 13:37:50.000000000 -0700
+++ linux-2.6.14-rc3/drivers/acpi/pci_irq.c	2005-10-11 10:02:42.000000000 -0700
@@ -298,6 +298,28 @@ acpi_pci_free_irq(struct acpi_prt_entry 
 	return_VALUE(irq);
 }
 
+
+
+static int 
+acpi_pci_find_irq(struct acpi_prt_entry *entry,
+		  int *edge_level, int *active_high_low, char **link)
+{
+	int irq;
+
+	ACPI_FUNCTION_TRACE("acpi_pci_find_irq");
+	if (entry->link.handle) {
+		irq = acpi_pci_link_find_irq(entry->link.handle);
+	} else {
+		irq = entry->link.index;
+	}
+
+	*link = entry;
+
+	return_VALUE(irq);
+}
+
+
+	
 /*
  * acpi_pci_irq_lookup
  * success: return IRQ >= 0
@@ -491,6 +513,83 @@ void __attribute__ ((weak)) acpi_unregis
 {
 }
 
+
+
+/*
+ * This function will be called only in the case of
+ * a "surprise" hot plug removal.  For surprise removals,
+ * the card has either already be yanked out of the slot, or
+ * the slot's been powered off, so we have to brute force 
+ * our way through all the possible interrupt pins to derive
+ * the GSI, then we double check with the value stored in the
+ * pci_dev structure to make sure we have the GSI that belongs
+ * to this IRQ.
+ */
+void acpi_pci_irq_disable_nodev(struct pci_dev *dev)
+{
+	int gsi = 0;
+	u8  pin = 0;
+	int edge_level = ACPI_LEVEL_SENSITIVE;
+	int active_high_low = ACPI_ACTIVE_LOW;
+	int irq;
+	struct acpi_prt_entry *entry;
+
+	ACPI_FUNCTION_TRACE("acpi_pci_irq_disable_nodev");
+
+	/* 
+	 * since our device is not present, we 
+	 * can't just read the interrupt pin
+	 * and use the value to derive the irq.
+	 * in this case, we are going to check
+	 * each returned irq value to make
+	 * sure it matches our already assigned
+	 * irq before we use it.
+	 */
+	for (pin = 0; pin < 4; pin++) {
+		/*
+	 	 * First we check the PCI IRQ routing table (PRT) for an IRQ.
+	 	 */
+		gsi = acpi_pci_irq_lookup(dev->bus, PCI_SLOT(dev->devfn), pin,
+				  &edge_level, &active_high_low, (char **)&entry,
+				  acpi_pci_find_irq);
+
+		/*
+	 	 * If no PRT entry was found, we'll try to derive an IRQ from the
+	 	 * device's parent bridge.
+	 	 */
+		if (gsi < 0)
+ 			gsi = acpi_pci_irq_derive(dev, pin,
+				&edge_level, &active_high_low, &entry, acpi_pci_find_irq);
+
+		/* 
+		 * If we could not derive the IRQ, give up on this pin number
+		 * and try a different one.
+		 */
+		if (gsi < 0)
+		 	continue;	
+		
+		if (acpi_gsi_to_irq(gsi, &irq) < 0)
+			continue;
+
+		/* 
+		 * make sure we got the right irq 
+		 */
+		if (irq == dev->irq) {
+			acpi_pci_free_irq(entry, &edge_level, &active_high_low, NULL);
+			printk(KERN_INFO PREFIX 
+				"PCI interrupt for device %s disabled\n",
+	       			pci_name(dev));
+
+			acpi_unregister_gsi(gsi);
+			return_VOID;
+		}
+	}
+	return_VOID;
+}
+
+
+
+
 void acpi_pci_irq_disable(struct pci_dev *dev)
 {
 	int gsi = 0;
@@ -506,6 +605,14 @@ void acpi_pci_irq_disable(struct pci_dev
 	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
 	if (!pin)
 		return_VOID;
+
+	/* 
+	 * Check to see if the device was present 
+	 */
+	if (pin == 0xff) {
+		acpi_pci_irq_disable_nodev(dev);
+		return_VOID;
+	}
 	pin--;
 
 	/*
diff -uprN -X linux-2.6.14-rc3.orig/Documentation/dontdiff linux-2.6.14-rc3.orig/drivers/acpi/pci_link.c linux-2.6.14-rc3/drivers/acpi/pci_link.c
--- linux-2.6.14-rc3.orig/drivers/acpi/pci_link.c	2005-10-07 13:37:50.000000000 -0700
+++ linux-2.6.14-rc3/drivers/acpi/pci_link.c	2005-10-11 09:51:27.000000000 -0700
@@ -665,6 +665,41 @@ acpi_pci_link_allocate_irq(acpi_handle h
 	return_VALUE(link->irq.active);
 }
 
+
+
+int acpi_pci_link_find_irq(acpi_handle handle)
+{
+	struct acpi_device *device = NULL;
+	struct acpi_pci_link *link = NULL;
+	acpi_status result;
+
+	ACPI_FUNCTION_TRACE("acpi_pci_link_find_irq");
+
+	result = acpi_bus_get_device(handle, &device);
+	if (result) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid link device\n"));
+		return_VALUE(-1);
+	}
+
+	link = (struct acpi_pci_link *)acpi_driver_data(device);
+	if (!link) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid link context\n"));
+		return_VALUE(-1);
+	}
+
+	down(&acpi_link_lock);
+	if (!link->irq.initialized) {
+		up(&acpi_link_lock);
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Link isn't initialized\n"));
+		return_VALUE(-1);
+	}
+
+	up(&acpi_link_lock);
+	return_VALUE(link->irq.active);
+}
+
+
+
 /*
  * We don't change link's irq information here.  After it is reenabled, we
  * continue use the info

Re: [Pcihpd-discuss] RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Kristen Accardi]

For surprise hotplug removal, the interrupt pin must be guessed, as any
attempts to read it would obviously be invalid.  This patch adds a new
function to cycle through all possible pin values, and tries to either
lookup or derive the right irq to disable.  It also adds a new function
to acpi which can be used to just find the irq, without freeing it, just
in case we guess the wrong pin.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
---
I fixed this patch so that it compiles without warnings now.

diff -uprN -X linux-2.6.14-rc3/Documentation/dontdiff linux-2.6.14-rc3.orig/drivers/acpi/pci_irq.c linux-2.6.14-rc3/drivers/acpi/pci_irq.c
--- linux-2.6.14-rc3.orig/drivers/acpi/pci_irq.c	2005-10-07 13:37:50.000000000 -0700
+++ linux-2.6.14-rc3/drivers/acpi/pci_irq.c	2005-10-18 16:26:44.000000000 -0700
@@ -298,6 +298,28 @@ acpi_pci_free_irq(struct acpi_prt_entry 
 	return_VALUE(irq);
 }
 
+
+
+static int
+acpi_pci_find_irq(struct acpi_prt_entry *entry,
+		  int *edge_level, int *active_high_low, char **link)
+{
+	int irq;
+
+	ACPI_FUNCTION_TRACE("acpi_pci_find_irq");
+	if (entry->link.handle) {
+		irq = acpi_pci_link_find_irq(entry->link.handle);
+	} else {
+		irq = entry->link.index;
+	}
+
+	*link = (char *)entry;
+
+	return_VALUE(irq);
+}
+
+
+
 /*
  * acpi_pci_irq_lookup
  * success: return IRQ >= 0
@@ -491,6 +513,86 @@ void __attribute__ ((weak)) acpi_unregis
 {
 }
 
+
+
+/*
+ * This function will be called only in the case of
+ * a "surprise" hot plug removal.  For surprise removals,
+ * the card has either already be yanked out of the slot, or
+ * the slot's been powered off, so we have to brute force 
+ * our way through all the possible interrupt pins to derive
+ * the GSI, then we double check with the value stored in the
+ * pci_dev structure to make sure we have the GSI that belongs
+ * to this IRQ.
+ */
+void acpi_pci_irq_disable_nodev(struct pci_dev *dev)
+{
+	int gsi = 0;
+	u8  pin = 0;
+	int edge_level = ACPI_LEVEL_SENSITIVE;
+	int active_high_low = ACPI_ACTIVE_LOW;
+	int irq;
+	struct acpi_prt_entry *entry;
+
+	ACPI_FUNCTION_TRACE("acpi_pci_irq_disable_nodev");
+
+	/*
+	 * since our device is not present, we
+	 * can't just read the interrupt pin
+	 * and use the value to derive the irq.
+	 * in this case, we are going to check
+	 * each returned irq value to make
+	 * sure it matches our already assigned
+	 * irq before we use it.
+	 */
+	for (pin = 0; pin < 4; pin++) {
+		/*
+	 	 * First we check the PCI IRQ routing table (PRT) for an IRQ.
+	 	 */
+		gsi = acpi_pci_irq_lookup(dev->bus, PCI_SLOT(dev->devfn), pin,
+				&edge_level, &active_high_low,
+				(char **)&entry,
+				acpi_pci_find_irq);
+
+		/*
+	 	 * If no PRT entry was found, we'll try to derive an IRQ from the
+	 	 * device's parent bridge.
+	 	 */
+		if (gsi < 0)
+ 			gsi = acpi_pci_irq_derive(dev, pin,
+				&edge_level, &active_high_low,
+				(char **)&entry, acpi_pci_find_irq);
+
+		/*
+		 * If we could not derive the IRQ, give up on this pin number
+		 * and try a different one.
+		 */
+		if (gsi < 0)
+		 	continue;
+		
+		if (acpi_gsi_to_irq(gsi, &irq) < 0)
+			continue;
+
+		/*
+		 * make sure we got the right irq
+		 */
+		if (irq == dev->irq) {
+			acpi_pci_free_irq(entry, &edge_level,
+					&active_high_low, NULL);
+			printk(KERN_INFO PREFIX
+				"PCI interrupt for device %s disabled\n",
+	       			pci_name(dev));
+
+			acpi_unregister_gsi(gsi);
+			return_VOID;
+		}
+	}
+	return_VOID;
+}
+
+
+
+
 void acpi_pci_irq_disable(struct pci_dev *dev)
 {
 	int gsi = 0;
@@ -506,6 +608,14 @@ void acpi_pci_irq_disable(struct pci_dev
 	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
 	if (!pin)
 		return_VOID;
+
+	/*
+	 * Check to see if the device was present
+	 */
+	if (pin == 0xff) {
+		acpi_pci_irq_disable_nodev(dev);
+		return_VOID;
+	}
 	pin--;
 
 	/*
diff -uprN -X linux-2.6.14-rc3/Documentation/dontdiff linux-2.6.14-rc3.orig/drivers/acpi/pci_link.c linux-2.6.14-rc3/drivers/acpi/pci_link.c
--- linux-2.6.14-rc3.orig/drivers/acpi/pci_link.c	2005-10-07 13:37:50.000000000 -0700
+++ linux-2.6.14-rc3/drivers/acpi/pci_link.c	2005-10-18 16:17:30.000000000 -0700
@@ -665,6 +665,41 @@ acpi_pci_link_allocate_irq(acpi_handle h
 	return_VALUE(link->irq.active);
 }
 
+
+
+int acpi_pci_link_find_irq(acpi_handle handle)
+{
+	struct acpi_device *device = NULL;
+	struct acpi_pci_link *link = NULL;
+	acpi_status result;
+
+	ACPI_FUNCTION_TRACE("acpi_pci_link_find_irq");
+
+	result = acpi_bus_get_device(handle, &device);
+	if (result) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid link device\n"));
+		return_VALUE(-1);
+	}
+
+	link = (struct acpi_pci_link *)acpi_driver_data(device);
+	if (!link) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid link context\n"));
+		return_VALUE(-1);
+	}
+
+	down(&acpi_link_lock);
+	if (!link->irq.initialized) {
+		up(&acpi_link_lock);
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Link isn't initialized\n"));
+		return_VALUE(-1);
+	}
+
+	up(&acpi_link_lock);
+	return_VALUE(link->irq.active);
+}
+
+
+
 /*
  * We don't change link's irq information here.  After it is reenabled, we
  * continue use the info
diff -uprN -X linux-2.6.14-rc3/Documentation/dontdiff linux-2.6.14-rc3.orig/include/acpi/acpi_drivers.h linux-2.6.14-rc3/include/acpi/acpi_drivers.h
--- linux-2.6.14-rc3.orig/include/acpi/acpi_drivers.h	2005-10-07 13:37:52.000000000 -0700
+++ linux-2.6.14-rc3/include/acpi/acpi_drivers.h	2005-10-18 16:20:08.000000000 -0700
@@ -55,6 +55,7 @@ int acpi_irq_penalty_init(void);
 int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *edge_level,
 			       int *active_high_low, char **name);
 int acpi_pci_link_free_irq(acpi_handle handle);
+int acpi_pci_link_find_irq(acpi_handle handle);
 
 /* ACPI PCI Interrupt Routing (pci_irq.c) */
 

Re: [ACPI] Re: [Pcihpd-discuss] RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Bjorn Helgaas]

On Tuesday 18 October 2005 5:57 pm, Kristen Accardi wrote:
> For surprise hotplug removal, the interrupt pin must be guessed, as any
> attempts to read it would obviously be invalid.  This patch adds a new
> function to cycle through all possible pin values, and tries to either
> lookup or derive the right irq to disable.

I don't really like this because it adds a new path that's only
used for "surprise" removals.  So we have acpi_pci_irq_disable(),
which is used for normal removals, and acpi_pci_irq_disable_nodev()
for the surprise path.  That feels like a maintenance problem.

Other, non-ACPI, IRQ routing schemes should have the same problem
(needing to know the interrupt pin after the device has been removed),
so maybe the pin needs to be cached in the pci_dev?

Re: [ACPI] Re: [Pcihpd-discuss] RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Kristen Accardi]

On Wed, 2005-10-19 at 09:29 -0600, Bjorn Helgaas wrote:
> On Tuesday 18 October 2005 5:57 pm, Kristen Accardi wrote:
> > For surprise hotplug removal, the interrupt pin must be guessed, as any
> > attempts to read it would obviously be invalid.  This patch adds a new
> > function to cycle through all possible pin values, and tries to either
> > lookup or derive the right irq to disable.
> 
> I don't really like this because it adds a new path that's only
> used for "surprise" removals.  So we have acpi_pci_irq_disable(),
> which is used for normal removals, and acpi_pci_irq_disable_nodev()
> for the surprise path.  That feels like a maintenance problem.
> 
> Other, non-ACPI, IRQ routing schemes should have the same problem
> (needing to know the interrupt pin after the device has been removed),
> so maybe the pin needs to be cached in the pci_dev?

This seems like a good idea to me, if nobody objects to adding another
field to pci_dev, I can change the patch to do this and resubmit. 

Re: [ACPI] Re: [Pcihpd-discuss] RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Greg KH]

On Wed, Oct 19, 2005 at 09:51:51AM -0700, Kristen Accardi wrote:
> On Wed, 2005-10-19 at 09:29 -0600, Bjorn Helgaas wrote:
> > On Tuesday 18 October 2005 5:57 pm, Kristen Accardi wrote:
> > > For surprise hotplug removal, the interrupt pin must be guessed, as any
> > > attempts to read it would obviously be invalid.  This patch adds a new
> > > function to cycle through all possible pin values, and tries to either
> > > lookup or derive the right irq to disable.
> > 
> > I don't really like this because it adds a new path that's only
> > used for "surprise" removals.  So we have acpi_pci_irq_disable(),
> > which is used for normal removals, and acpi_pci_irq_disable_nodev()
> > for the surprise path.  That feels like a maintenance problem.
> > 
> > Other, non-ACPI, IRQ routing schemes should have the same problem
> > (needing to know the interrupt pin after the device has been removed),
> > so maybe the pin needs to be cached in the pci_dev?
> 
> This seems like a good idea to me, if nobody objects to adding another
> field to pci_dev, I can change the patch to do this and resubmit. 

No objection from me.

thanks,

greg k-h

Re: [ACPI] Re: [Pcihpd-discuss] RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Kristen Accardi]

This patch will allow the acpi code to correctly disable the irq when an
adapter has been "surprise" removed.  The INTERRUPT_PIN value is stored
in the pci_dev structure at probe time, and if the acpi code attempts to
read the INTERRUPT_PIN from config space and detects that the adapter is
not present, it will use the stored value instead.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

---
I've re-worked this patch as you suggested.  the INTERRUPT_PIN was read
in many different places in the code, so I hope that this is the right
place to save off the value.  Thanks for taking a look.

 drivers/acpi/pci_irq.c |   10 ++++++++++
 drivers/pci/probe.c    |    1 +
 include/linux/pci.h    |    1 +
 3 files changed, 12 insertions(+)

Index: linux-2.6.13/drivers/acpi/pci_irq.c
===================================================================
--- linux-2.6.13.orig/drivers/acpi/pci_irq.c
+++ linux-2.6.13/drivers/acpi/pci_irq.c
@@ -504,6 +504,16 @@ void acpi_pci_irq_disable(struct pci_dev
 		return_VOID;
 
 	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
+
+	/*
+	 * If a device has been "surprise" removed via
+	 * hotplug, the pin value will be invalid
+	 * In this case, we should use the stored
+	 * pin value from the pci_dev structure
+	 */
+	if (pin == 0xff)
+		pin = dev->pin;
+
 	if (!pin)
 		return_VOID;
 	pin--;
Index: linux-2.6.13/drivers/pci/probe.c
===================================================================
--- linux-2.6.13.orig/drivers/pci/probe.c
+++ linux-2.6.13/drivers/pci/probe.c
@@ -571,6 +571,7 @@ static void pci_read_irq(struct pci_dev 
 	unsigned char irq;
 
 	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
+	dev->pin = irq;
 	if (irq)
 		pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
 	dev->irq = irq;
Index: linux-2.6.13/include/linux/pci.h
===================================================================
--- linux-2.6.13.orig/include/linux/pci.h
+++ linux-2.6.13/include/linux/pci.h
@@ -98,6 +98,7 @@ struct pci_dev {
 	unsigned int	class;		/* 3 bytes: (base,sub,prog-if) */
 	u8		hdr_type;	/* PCI header type (`multi' flag masked out) */
 	u8		rom_base_reg;	/* which config register controls the ROM */
+	u8		pin;  		/* which interrupt pin this device uses */
 
 	struct pci_driver *driver;	/* which driver has allocated this device */
 	u64		dma_mask;	/* Mask of the bits of bus address this

Re: [ACPI] Re: [Pcihpd-discuss] RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Bjorn Helgaas]

>  	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
> +
> +	/*
> +	 * If a device has been "surprise" removed via
> +	 * hotplug, the pin value will be invalid
> +	 * In this case, we should use the stored
> +	 * pin value from the pci_dev structure
> +	 */
> +	if (pin == 0xff)
> +		pin = dev->pin;

I think you should just always use dev->pin, and don't even
bother trying the pci_read_config_byte().  Fewer code paths
to worry about that way.

Bjorn

Re: Re: [Pcihpd-discuss] RE: [patch 2/2] acpi: add ability to derive irq when doing a surpriseremoval of an adapter

[Matthew Wilcox]

On Wed, Oct 19, 2005 at 09:51:51AM -0700, Kristen Accardi wrote:
> This seems like a good idea to me, if nobody objects to adding another
> field to pci_dev, I can change the patch to do this and resubmit. 

If you squeeze it in after rom_base_reg, it won't even take any more
space.  Assuming you'll be using a u8 for it, that is. (We only really
need three bits, right?  0-4)


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl