Hi Russell,
On 02/02/15 16:33, Russell King - ARM Linux wrote:
> On Wed, Jan 28, 2015 at 02:51:23PM +0000, Marc Zyngier wrote:
>> void __weak pcibios_update_irq(struct pci_dev *dev, int irq)
>> {
>> - dev_dbg(&dev->dev, "assigning IRQ %02d\n", irq);
>> - pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
>> + struct irq_data *d;
>> +
>> + d = irq_get_irq_data(irq);
>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>> + while (d->parent_data)
>> + d = d->parent_data;
>> +#endif
>> + dev_dbg(&dev->dev, "assigning IRQ %02ld\n", d->hwirq);
>> + pci_write_config_byte(dev, PCI_INTERRUPT_LINE, d->hwirq);
>
> I'm really not convinced about this being the correct thing to do.
>
> Let's take an older ARM system, such as a Footbridge based system with a
> PCI southbridge.
>
> Such a system has IRQs 0-15 as the PCI southbridge ISA interrupts. Then
> there are four PCI interrupts provided by the on-board Footbridge.
>
> Right now, PCI devices are programmed with the OS specific interrupt
> number - eg:
>
> 00:06.1 IDE interface: Contaq Microsystems 82c693 (prog-if 80 [Master])
> Flags: medium devsel, IRQ 14
> 30: 00 00 00 00 00 00 00 00 00 00 00 00 0e 01 00 00
>
> 00:06.2 IDE interface: Contaq Microsystems 82c693 (prog-if 00 [])
> Flags: medium devsel, IRQ 15
> 30: 00 00 00 00 00 00 00 00 00 00 00 00 0f 02 00 00
>
> 00:06.3 USB Controller: Contaq Microsystems 82c693 (prog-if 10 [OHCI])
> Flags: medium devsel, IRQ 12
> 30: 00 00 00 00 00 00 00 00 00 00 00 00 0c 01 00 00
>
> 00:07.0 Mass storage controller: Integrated Technology Express, Inc. IT/ITE8212
> Dual channel ATA RAID controller (rev 13)
> Flags: bus master, 66MHz, medium devsel, latency 0, IRQ 24
> 30: 00 00 02 04 80 00 00 00 00 00 00 00 18 01 08 08
>
> 00:08.0 Ethernet controller: 3Com Corporation 3c905C-TX/TX-M [Tornado] (rev 74)
> Flags: bus master, medium devsel, latency 32, IRQ 22
> 30: 00 00 06 04 dc 00 00 00 00 00 00 00 16 01 0a 0a
>
> 00:09.0 VGA compatible controller: S3 Inc. 86c775/86c785 [Trio 64V2/DX or /GX] (rev 16) (prog-if 00 [VGA controller])
> Flags: medium devsel, IRQ 21
> 30: 00 00 00 0c 00 00 00 00 00 00 00 00 15 01 00 00
>
> What your change would mean is that the IRQs currently being programmed
>> = 16 would be programmed into with numbers with 16 removed from them.
> This means that legacy stuff (eg on the Southbridge which really do signal
> via the ISA IRQ controller) end up using the same number range as those
> which take PCI specific IRQs.
>
> This surely is not sane.
I suppose this is ebsa285? I must confess I don't see how to distinguish
the two cases (the GIC case uses a purely virtual number, and the
footbridge case uses something that seems to be physical).
A very easy fix would be to entirely contain this change within #ifdef
CONFIG_IRQ_DOMAIN_HIERARCHY/#endif, but that doesn't fill me with
confidence.
What I don't get is how the hwirq field is set in this case. It probably
isn't very useful (as there is no domain lookup), so I would have hoped
to see irq == hwirq. Obviously, this is not the case. What am I missing?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
