2004-09-21 08:53:35

by Kenji Kaneshige

[permalink] [raw]
Subject: [PATCH] PCI IRQ resource deallocation support [2/3]


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 <[email protected]>


---

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;

_


2004-09-21 14:58:23

by Bjorn Helgaas

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

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().

2004-09-22 01:22:58

by Kenji Kaneshige

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

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


2004-09-24 05:57:36

by Takayoshi Kochi

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

From: Kenji Kaneshige <[email protected]>
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 <[email protected]>

2004-09-24 06:34:55

by Kenji Kaneshige

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

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

2004-09-24 21:58:33

by Guennadi Liakhovetski

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

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

2004-09-27 04:59:58

by Kenji Kaneshige

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

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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/acpi-devel
>