Re: [PATCH v6 4/5] PCI: add PCI controller for keystone PCIe h/w

Murali Karicheri
2014-07-22T19:02:01Z

On 07/22/2014 11:37 AM, Rob Herring wrote:
> On Fri, Jul 18, 2014 at 2:50 PM, Arnd Bergmann<arnd@arndb.de> wrote:
>> On Friday 18 July 2014 14:31:39 Rob Herring wrote:
>>>> +
>>>> + Example:
>>>> + pcie_msi_intc: msi-interrupt-controller {
>>>> + interrupt-controller;
>>>> + #interrupt-cells =<1>;
>>>> + interrupt-parent =<&gic>;
>>>> + interrupts =<GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 31 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 32 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 33 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 34 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 35 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 36 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 37 IRQ_TYPE_EDGE_RISING>;
>>>> + };
>>>> +
>>>> +pcie_intc: Interrupt controller device node for Legacy irq chip
>>>> + interrupt-cells: should be set to 1
>>>> + interrupt-parent: Parent interrupt controller phandle
>>>> + interrupts: GIC interrupt lines connected to PCI Legacy interrupt lines
>>>> +
>>>> + Example:
>>>> + pcie_intc: legacy-interrupt-controller {
>>>> + interrupt-controller;
>>>> + #interrupt-cells =<1>;
>>>> + interrupt-parent =<&gic>;
>>>> + interrupts =<GIC_SPI 26 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 27 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 28 IRQ_TYPE_EDGE_RISING>,
>>>> +<GIC_SPI 29 IRQ_TYPE_EDGE_RISING>;
>>>> + };
>>> This seems wrong. Legacy interrupts should be described with
>>> interrupt-map and then PCI child devices have a standard interrupt
>>> specifier.
>>>
>>> I'm not sure about MSIs, but I would think they would have a standard
>>> format too.
>>>
>> IIRC, it's actually the correct way to do this here: the problem is that
>> the PCI IRQs are not directly connected to the GIC, but instead there is
>> a nested irqchip that has each PCI IRQ routed to it and that requires
>> an extra EOI for each interrupt.
>>
>> The interrupt-map in the PCI host points to this special irqchip rather
>> than to the GIC.
> Okay, if there is still an interrupt-map property, then I agree. This
> wasn't clear in the example.
Rob,

There is and is described in designware DT documentation. This documentation
refers to that and also describe DT bindings specific to Keystone. Could you
Ack this based on this?

Murali
>
> Rob