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


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/