[PATCH] PCI IRQ resource deallocation support [2/3]

[PATCH] PCI IRQ resource deallocation support [2/3]

[Kenji Kaneshige]

This patch is ACPI portion of IRQ deallocation. This patch defines the
following new interface. The implementation of this interface depends
on each platform.

    o void acpi_unregister_gsi(int irq)

        This is a opposite portion of acpi_register_gsi(). This has a
        responsibility for deallocating IRQ resources associated with
        the specified linux IRQ number.

        We need to consider the case of shared interrupt. In the case
        of shared interrupt, acpi_register_gsi() is called multiple
        times for one gsi. That is, registrations and unregistrations
        can be nested.

        This function undoes the effect of one call to
        acpi_register_gsi(). If this matches the last registration,
        IRQ resources associated with the specified linux IRQ number
        are freed.

This patch also adds the following new function.

    o void acpi_pci_irq_disable (struct pci_dev *dev)

        This function is a opposite portion of
        acpi_pci_enable_irq(). It clears the device's linux IRQ number
        and calls acpi_unregister_gsi() to deallocate IRQ resources.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>


---

 linux-2.6.9-rc2-mm1-kanesige/arch/i386/kernel/acpi/boot.c |   11 +++++
 linux-2.6.9-rc2-mm1-kanesige/arch/ia64/kernel/acpi.c      |   11 +++++
 linux-2.6.9-rc2-mm1-kanesige/drivers/acpi/pci_irq.c       |   26 ++++++++++++++
 linux-2.6.9-rc2-mm1-kanesige/include/linux/acpi.h         |    2 +
 4 files changed, 50 insertions(+)

diff -puN arch/i386/kernel/acpi/boot.c~IRQ_deallocation_acpi arch/i386/kernel/acpi/boot.c
--- linux-2.6.9-rc2-mm1/arch/i386/kernel/acpi/boot.c~IRQ_deallocation_acpi	2004-09-21 14:06:20.000000000 +0900
+++ linux-2.6.9-rc2-mm1-kanesige/arch/i386/kernel/acpi/boot.c	2004-09-21 15:23:48.012354914 +0900
@@ -480,6 +480,17 @@ unsigned int acpi_register_gsi(u32 gsi, 
 }
 EXPORT_SYMBOL(acpi_register_gsi);
 
+/*
+ * This function undoes the effect of one call to acpi_register_gsi().
+ * If this matches the last regstration, any IRQ resources for gsi
+ * associated with the irq are freed.
+ */
+void
+acpi_unregister_gsi (unsigned int irq)
+{
+}
+EXPORT_SYMBOL(acpi_unregister_gsi);
+
 static unsigned long __init
 acpi_scan_rsdp (
 	unsigned long		start,
diff -puN arch/ia64/kernel/acpi.c~IRQ_deallocation_acpi arch/ia64/kernel/acpi.c
--- linux-2.6.9-rc2-mm1/arch/ia64/kernel/acpi.c~IRQ_deallocation_acpi	2004-09-21 14:06:20.000000000 +0900
+++ linux-2.6.9-rc2-mm1-kanesige/arch/ia64/kernel/acpi.c	2004-09-21 15:23:11.218165006 +0900
@@ -516,6 +516,17 @@ acpi_register_gsi (u32 gsi, int edge_lev
 }
 EXPORT_SYMBOL(acpi_register_gsi);
 
+/*
+ * This function undoes the effect of one call to acpi_register_gsi().
+ * If this matches the last regstration, any IRQ resources for gsi
+ * associated with the irq are freed.
+ */
+void
+acpi_unregister_gsi (unsigned int irq)
+{
+}
+EXPORT_SYMBOL(acpi_unregister_gsi);
+
 static int __init
 acpi_parse_fadt (unsigned long phys_addr, unsigned long size)
 {
diff -puN drivers/acpi/pci_irq.c~IRQ_deallocation_acpi drivers/acpi/pci_irq.c
--- linux-2.6.9-rc2-mm1/drivers/acpi/pci_irq.c~IRQ_deallocation_acpi	2004-09-21 14:06:20.000000000 +0900
+++ linux-2.6.9-rc2-mm1-kanesige/drivers/acpi/pci_irq.c	2004-09-21 14:06:20.000000000 +0900
@@ -390,3 +390,29 @@ acpi_pci_irq_enable (
 
 	return_VALUE(dev->irq);
 }
+
+void
+acpi_pci_irq_disable (
+	struct pci_dev		*dev)
+{
+	unsigned char irq_disabled, irq;
+
+	ACPI_FUNCTION_TRACE("acpi_pci_irq_disable");
+
+	irq_disabled = dev->irq;
+
+	/*
+	 * dev->irq is cleared by BIOS-assigned IRQ set during boot.
+	 */
+	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
+	if (irq)
+		pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
+	dev->irq = irq;
+
+	printk(KERN_INFO PREFIX "PCI interrupt for device %s disabled\n",
+	       pci_name(dev));
+
+	acpi_unregister_gsi(irq_disabled);
+
+	return_VOID;
+}
diff -puN include/linux/acpi.h~IRQ_deallocation_acpi include/linux/acpi.h
--- linux-2.6.9-rc2-mm1/include/linux/acpi.h~IRQ_deallocation_acpi	2004-09-21 14:06:20.000000000 +0900
+++ linux-2.6.9-rc2-mm1-kanesige/include/linux/acpi.h	2004-09-21 14:06:20.000000000 +0900
@@ -414,6 +414,7 @@ static inline int acpi_boot_init(void)
 #endif 	/*!CONFIG_ACPI_BOOT*/
 
 unsigned int acpi_register_gsi (u32 gsi, int edge_level, int active_high_low);
+void acpi_unregister_gsi (unsigned int);
 int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
 
 #ifdef CONFIG_ACPI_PCI
@@ -439,6 +440,7 @@ extern struct acpi_prt_list	acpi_prt;
 struct pci_dev;
 
 int acpi_pci_irq_enable (struct pci_dev *dev);
+void acpi_pci_irq_disable (struct pci_dev *dev);
 
 struct acpi_pci_driver {
 	struct acpi_pci_driver *next;

_

Re: [ACPI] [PATCH] PCI IRQ resource deallocation support [2/3]

[Bjorn Helgaas]

On Tuesday 21 September 2004 2:52 am, Kenji Kaneshige wrote:
> + * This function undoes the effect of one call to acpi_register_gsi().
> + * If this matches the last regstration, any IRQ resources for gsi

s/regstration/registration/ (also other occurrences below).

> +void
> +acpi_pci_irq_disable (
> + struct pci_dev  *dev)
> +{
> + unsigned char irq_disabled, irq;

pci_dev.irq is unsigned int, not unsigned char, so irq_disabled
should be unsigned int as well.

> +  * dev->irq is cleared by BIOS-assigned IRQ set during boot.
> +  */
> + pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
> + if (irq)
> +  pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> + dev->irq = irq;

Why do we need to fiddle with dev->irq?  I think it should
just be undefined after acpi_pci_irq_disable().

Re: [ACPI] [PATCH] PCI IRQ resource deallocation support [2/3]

[Kenji Kaneshige]

Hi Bjorn,

Thank you for your feedbacks.

Bjorn Helgaas wrote:

> On Tuesday 21 September 2004 2:52 am, Kenji Kaneshige wrote:
>> + * This function undoes the effect of one call to acpi_register_gsi().
>> + * If this matches the last regstration, any IRQ resources for gsi
> 
> s/regstration/registration/ (also other occurrences below).

Oops..
I'll fix these.

> 
>> +void
>> +acpi_pci_irq_disable (
>> + struct pci_dev  *dev)
>> +{
>> + unsigned char irq_disabled, irq;
> 
> pci_dev.irq is unsigned int, not unsigned char, so irq_disabled
> should be unsigned int as well.
> 

I'll fix this, thanks.


>> +  * dev->irq is cleared by BIOS-assigned IRQ set during boot.
>> +  */
>> + pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
>> + if (irq)
>> +  pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
>> + dev->irq = irq;
> 
> Why do we need to fiddle with dev->irq?  I think it should
> just be undefined after acpi_pci_irq_disable().

I had been considering what the "undefined dev->irq" was.
In fact, I had other ideas that was clearing it by zero or
-1 (0xffffffff). But I didn't know if we can use these values
as a undefined IRQ number. So I'm clearing it by the value
which was assigned by PCI core code (pci_read_irq()) before
acpi_pci_irq_enable() was called. 

How do you think?

Thanks,
Kenji Kaneshige

Re: [ACPI] [PATCH] PCI IRQ resource deallocation support [2/3]

[Takayoshi Kochi]

From: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Subject: Re: [ACPI] [PATCH] PCI IRQ resource deallocation support [2/3]
Date: Wed, 22 Sep 2004 10:24:40 +0900

> >> +  * dev->irq is cleared by BIOS-assigned IRQ set during boot.
> >> +  */
> >> + pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
> >> + if (irq)
> >> +  pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> >> + dev->irq = irq;
> > 
> > Why do we need to fiddle with dev->irq?  I think it should
> > just be undefined after acpi_pci_irq_disable().
> 
> I had been considering what the "undefined dev->irq" was.
> In fact, I had other ideas that was clearing it by zero or
> -1 (0xffffffff). But I didn't know if we can use these values
> as a undefined IRQ number. So I'm clearing it by the value
> which was assigned by PCI core code (pci_read_irq()) before
> acpi_pci_irq_enable() was called. 

I think it has little sense in restoring value from the configuration
space to dev->irq or clearing it.

If we do preventive programming, it may be worth
trying to define some magic constant (e.g. PCI_UNDEFINED_IRQ) and
panic/BUG when the irq is being enabled.
Otherwise, leaving dev->irq as it is would be ok.

---
Takayoshi Kochi <t-kochi@bq.jp.nec.com>

Re: [ACPI] [PATCH] PCI IRQ resource deallocation support [2/3]

[Kenji Kaneshige]

Hi Takayoshi,

Thank you for feedback.

Takayoshi Kochi wrote:
>> > Why do we need to fiddle with dev->irq?  I think it should
>> > just be undefined after acpi_pci_irq_disable().
>> 
>> I had been considering what the "undefined dev->irq" was.
>> In fact, I had other ideas that was clearing it by zero or
>> -1 (0xffffffff). But I didn't know if we can use these values
>> as a undefined IRQ number. So I'm clearing it by the value
>> which was assigned by PCI core code (pci_read_irq()) before
>> acpi_pci_irq_enable() was called. 
> 
> I think it has little sense in restoring value from the configuration
> space to dev->irq or clearing it.
> 
> If we do preventive programming, it may be worth
> trying to define some magic constant (e.g. PCI_UNDEFINED_IRQ) and
> panic/BUG when the irq is being enabled.
> Otherwise, leaving dev->irq as it is would be ok.

Hmm, I think you are right.
I'll change my patch to leave dev->irq as it is. And then I'll
investigate about defining PCI_UNDEFINED_IRQ.

In fact, I posted updated patches a little before. I'll re-post
it with your feedback :-)

Thanks,
Kenji Kaneshige

Re: [ACPI] [PATCH] PCI IRQ resource deallocation support [2/3]

[Guennadi Liakhovetski]

On Fri, 24 Sep 2004, Kenji Kaneshige wrote:

> Takayoshi Kochi wrote:
> I'll change my patch to leave dev->irq as it is. And then I'll
> investigate about defining PCI_UNDEFINED_IRQ.

Some platforms (arm, arm26, ppc64) define a macro NO_IRQ:

include/asm-arm/irq.h:#define NO_IRQ        ((unsigned int)(-1))
include/asm-arm26/irq.h:#define NO_IRQ      ((unsigned int)(-1))
include/asm-ppc64/irq.h:#define NO_IRQ      (-1)

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [ACPI] [PATCH] PCI IRQ resource deallocation support [2/3]

[Kenji Kaneshige]

Hi Guennadi Liakhovetski,

Thank you for the information.
I refer to arm, arm26 and ppc64 codes.

Thanks,
Kenji Kaneshige


Guennadi Liakhovetski wrote:

> On Fri, 24 Sep 2004, Kenji Kaneshige wrote:
> 
>> Takayoshi Kochi wrote:
>> I'll change my patch to leave dev->irq as it is. And then I'll
>> investigate about defining PCI_UNDEFINED_IRQ.
> 
> 
> Some platforms (arm, arm26, ppc64) define a macro NO_IRQ:
> 
> include/asm-arm/irq.h:#define NO_IRQ        ((unsigned int)(-1))
> include/asm-arm26/irq.h:#define NO_IRQ      ((unsigned int)(-1))
> include/asm-ppc64/irq.h:#define NO_IRQ      (-1)
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski
> 
> 
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
> Project Admins to receive an Apple iPod Mini FREE for your judgement on
> who ports your project to Linux PPC the best. Sponsored by IBM.
> Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
> _______________________________________________
> Acpi-devel mailing list
> Acpi-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/acpi-devel
>