diff mbox

[1/4] PCI: add DT bindings for Cortina Gemini PCI Host Bridge

Message ID 20170128204839.18330-1-linus.walleij@linaro.org (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Linus Walleij Jan. 28, 2017, 8:48 p.m. UTC
This adds device tree bindings for the Cortina Systems Gemini PCI
Host Bridge.

Cc: Janos Laube <janos.dev@gmail.com>
Cc: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Cc: Hans Ulli Kroll <ulli.kroll@googlemail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
This can be merged to the PCI tree whenever it is considered
fine for inclusion.
---
 .../devicetree/bindings/pci/cortina,gemini-pci.txt | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt

Comments

Bjorn Helgaas Jan. 31, 2017, 12:31 a.m. UTC | #1
Hi Linus,

On Sat, Jan 28, 2017 at 09:48:36PM +0100, Linus Walleij wrote:
> This adds device tree bindings for the Cortina Systems Gemini PCI
> Host Bridge.
> 
> Cc: Janos Laube <janos.dev@gmail.com>
> Cc: Paulius Zaleckas <paulius.zaleckas@gmail.com>
> Cc: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I'm hoping one of the DT folks will take a quick look at this.

> ---
> This can be merged to the PCI tree whenever it is considered
> fine for inclusion.
> ---
>  .../devicetree/bindings/pci/cortina,gemini-pci.txt | 64 ++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt b/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
> new file mode 100644
> index 000000000000..e3090d995e1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
> @@ -0,0 +1,64 @@
> +* Cortina Systems Gemini PCI Host Bridge
> +
> +Mandatory properties:
> +
> +- compatible: should be "cortina,gemini-pci"
> +- reg: memory base and size for the host bridge
> +- interrupts: the four PCI interrupts
> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- #interrupt-cells: set to <1>
> +- bus-range: set to <0x00 0x00> (only root bus)

Why is this limited to bus 0?  Is everything completely soldered down
with no possibility of adding or replacing PCI devices?  The interrupt-map
below suggests slots, though.  If there's a slot, we could plug in a card
with a bridge, which would mean more than just bus 0.

> +- device_type, set to "pci"
> +- ranges: see pci.txt
> +- interrupt-map-mask: see pci.txt
> +- interrupt-map: see pci.txt
> +
> +Mandatory subnodes:
> +- One node reprenting the interrupt-controller inside the host bridge

s/reprenting/representing/

> +  with the following mandatory properties:
> +  - interrupt-controller: see interrupt-controller/interrupts.txt
> +  - #address-cells: set to <0>
> +  - #interrupt-cells: set to <1>
> +
> +Example:
> +
> +pci@50000000 {
> +	compatible = "cortina,gemini-pci";
> +	reg = <0x50000000 0x100>;
> +	interrupts = <8 IRQ_TYPE_LEVEL_HIGH>, /* PCI A */
> +			<26 IRQ_TYPE_LEVEL_HIGH>, /* PCI B */
> +			<27 IRQ_TYPE_LEVEL_HIGH>, /* PCI C */
> +			<28 IRQ_TYPE_LEVEL_HIGH>; /* PCI D */
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	#interrupt-cells = <1>;
> +
> +	bus-range = <0x00 0x00>; /* Only root bus */
> +	ranges = /* 1MiB I/O space 0x50000000-0x500fffff */
> +		 <0x01000000 0 0          0x50000000 0 0x00100000>,
> +		 /* 128MiB non-prefetchable memory 0x58000000-0x5fffffff */
> +		 <0x02000000 0 0x58000000 0x58000000 0 0x08000000>;
> +	interrupt-map-mask = <0xff00 0 0 7>;
> +	interrupt-map = <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
> +			<0x4900 0 0 2 &pci_intc 1>,
> +			<0x4a00 0 0 3 &pci_intc 2>,
> +			<0x4b00 0 0 4 &pci_intc 3>,
> +			<0x5000 0 0 1 &pci_intc 0>, /* Slot 10 */
> +			<0x5100 0 0 2 &pci_intc 1>,
> +			<0x5200 0 0 3 &pci_intc 2>,
> +			<0x5300 0 0 4 &pci_intc 3>,
> +			<0x5800 0 0 1 &pci_intc 0>, /* Slot 11 */
> +			<0x5900 0 0 2 &pci_intc 1>,
> +			<0x5a00 0 0 3 &pci_intc 2>,
> +			<0x5b00 0 0 4 &pci_intc 3>,
> +			<0x6000 0 0 1 &pci_intc 0>, /* Slot 12 */
> +			<0x6100 0 0 2 &pci_intc 1>,
> +			<0x6200 0 0 3 &pci_intc 2>,
> +			<0x6300 0 0 4 &pci_intc 3>;
> +	pci_intc: interrupt-controller {
> +		interrupt-controller;
> +		#address-cells = <0>;
> +		#interrupt-cells = <1>;
> +	};
> +};
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Feb. 1, 2017, 11:09 a.m. UTC | #2
On Saturday, January 28, 2017 9:48:36 PM CET Linus Walleij wrote:
> +       pci_intc: interrupt-controller {
> +               interrupt-controller;
> +               #address-cells = <0>;
> +               #interrupt-cells = <1>;
> +       };
> 


I think this one needs at least an interrupt-parent property, otherwise
you would recursively translate the numbers back through the interrupt-map
property of its parent.

	Arnd
Arnd Bergmann Feb. 1, 2017, 11:19 a.m. UTC | #3
On Saturday, January 28, 2017 9:48:36 PM CET Linus Walleij wrote:
> +       interrupt-map-mask = <0xff00 0 0 7>;
> +       interrupt-map = <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
> +                       <0x4900 0 0 2 &pci_intc 1>,
> +                       <0x4a00 0 0 3 &pci_intc 2>,
> +                       <0x4b00 0 0 4 &pci_intc 3>,
> +                       <0x5000 0 0 1 &pci_intc 0>, /* Slot 10 */
> +                       <0x5100 0 0 2 &pci_intc 1>,
> +                       <0x5200 0 0 3 &pci_intc 2>,
> +                       <0x5300 0 0 4 &pci_intc 3>,
> +                       <0x5800 0 0 1 &pci_intc 0>, /* Slot 11 */
> +                       <0x5900 0 0 2 &pci_intc 1>,
> +                       <0x5a00 0 0 3 &pci_intc 2>,
> +                       <0x5b00 0 0 4 &pci_intc 3>,
> +                       <0x6000 0 0 1 &pci_intc 0>, /* Slot 12 */
> +                       <0x6100 0 0 2 &pci_intc 1>,
> +                       <0x6200 0 0 3 &pci_intc 2>,
> +                       <0x6300 0 0 4 &pci_intc 3>;
> 

The mapping looks wrong here, we normally don't list interrupts per function
so the mask should be 0xf800.

Note that the interrupt map is board specific, so this should probably
go in the board.dts file rather than platform.dtsi.

For this particular board, the interrupt lines appear to have been badly
configured so all slots use the same interrupt 0 for IntA. IIRC This also
means you can probably use <0 0 0 7> as the mask and just specify each of
the four interrupts once. A properly wired board would swizzle the
interrupts so that each slot has a different IRQ for its IntA line.

	Arnd
Rob Herring Feb. 1, 2017, 4:02 p.m. UTC | #4
On Sat, Jan 28, 2017 at 09:48:36PM +0100, Linus Walleij wrote:
> This adds device tree bindings for the Cortina Systems Gemini PCI
> Host Bridge.
> 
> Cc: Janos Laube <janos.dev@gmail.com>
> Cc: Paulius Zaleckas <paulius.zaleckas@gmail.com>
> Cc: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This can be merged to the PCI tree whenever it is considered
> fine for inclusion.
> ---
>  .../devicetree/bindings/pci/cortina,gemini-pci.txt | 64 ++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt b/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
> new file mode 100644
> index 000000000000..e3090d995e1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
> @@ -0,0 +1,64 @@
> +* Cortina Systems Gemini PCI Host Bridge
> +
> +Mandatory properties:
> +
> +- compatible: should be "cortina,gemini-pci"
> +- reg: memory base and size for the host bridge
> +- interrupts: the four PCI interrupts
> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- #interrupt-cells: set to <1>
> +- bus-range: set to <0x00 0x00> (only root bus)
> +- device_type, set to "pci"
> +- ranges: see pci.txt

You need to specify what ranges should have for this bridge.

> +- interrupt-map-mask: see pci.txt
> +- interrupt-map: see pci.txt
> +
> +Mandatory subnodes:
> +- One node reprenting the interrupt-controller inside the host bridge
> +  with the following mandatory properties:
> +  - interrupt-controller: see interrupt-controller/interrupts.txt
> +  - #address-cells: set to <0>
> +  - #interrupt-cells: set to <1>
> +
> +Example:
> +
> +pci@50000000 {
> +	compatible = "cortina,gemini-pci";
> +	reg = <0x50000000 0x100>;

Config space is indirectly accessed?

> +	interrupts = <8 IRQ_TYPE_LEVEL_HIGH>, /* PCI A */
> +			<26 IRQ_TYPE_LEVEL_HIGH>, /* PCI B */
> +			<27 IRQ_TYPE_LEVEL_HIGH>, /* PCI C */
> +			<28 IRQ_TYPE_LEVEL_HIGH>; /* PCI D */
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	#interrupt-cells = <1>;
> +
> +	bus-range = <0x00 0x00>; /* Only root bus */
> +	ranges = /* 1MiB I/O space 0x50000000-0x500fffff */
> +		 <0x01000000 0 0          0x50000000 0 0x00100000>,
> +		 /* 128MiB non-prefetchable memory 0x58000000-0x5fffffff */
> +		 <0x02000000 0 0x58000000 0x58000000 0 0x08000000>;
> +	interrupt-map-mask = <0xff00 0 0 7>;
> +	interrupt-map = <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
> +			<0x4900 0 0 2 &pci_intc 1>,
> +			<0x4a00 0 0 3 &pci_intc 2>,
> +			<0x4b00 0 0 4 &pci_intc 3>,
> +			<0x5000 0 0 1 &pci_intc 0>, /* Slot 10 */
> +			<0x5100 0 0 2 &pci_intc 1>,
> +			<0x5200 0 0 3 &pci_intc 2>,
> +			<0x5300 0 0 4 &pci_intc 3>,
> +			<0x5800 0 0 1 &pci_intc 0>, /* Slot 11 */
> +			<0x5900 0 0 2 &pci_intc 1>,
> +			<0x5a00 0 0 3 &pci_intc 2>,
> +			<0x5b00 0 0 4 &pci_intc 3>,
> +			<0x6000 0 0 1 &pci_intc 0>, /* Slot 12 */
> +			<0x6100 0 0 2 &pci_intc 1>,
> +			<0x6200 0 0 3 &pci_intc 2>,
> +			<0x6300 0 0 4 &pci_intc 3>;
> +	pci_intc: interrupt-controller {
> +		interrupt-controller;
> +		#address-cells = <0>;
> +		#interrupt-cells = <1>;
> +	};
> +};
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Feb. 1, 2017, 8 p.m. UTC | #5
On Tue, Jan 31, 2017 at 1:31 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:

>> +- bus-range: set to <0x00 0x00> (only root bus)
>
> Why is this limited to bus 0?  Is everything completely soldered down
> with no possibility of adding or replacing PCI devices? The interrupt-map
> below suggests slots, though.  If there's a slot, we could plug in a card
> with a bridge, which would mean more than just bus 0.

I thought so because it's a NAS box (the typical usecase) but when I
dismantled it and turned it over, wow, there is indeed a MiniPCI
slot!

OK I will augment this to 0x00 0xff.

Yours,
Linus Walleij
Linus Walleij Feb. 1, 2017, 8:04 p.m. UTC | #6
On Wed, Feb 1, 2017 at 5:02 PM, Rob Herring <robh@kernel.org> wrote:
> On Sat, Jan 28, 2017 at 09:48:36PM +0100, Linus Walleij wrote:

>> +Example:
>> +
>> +pci@50000000 {
>> +     compatible = "cortina,gemini-pci";
>> +     reg = <0x50000000 0x100>;
>
> Config space is indirectly accessed?

Yes it is a really annoying construction. The first device, the Faraday
roob hub is at that address, the init code uses that to set up the bus.

As soon as the bus is up, we use bus accessors to talk to the root
hub on slot 0 (this manages interrupts etc).

I guess I could even unmap this reg range at that point ...

I guess I could also get the address directly from the IO range
and start poking around. I don't know if that is any better.

Yours,
Linus Walleij
Linus Walleij Feb. 5, 2017, 2:44 p.m. UTC | #7
On Wed, Feb 1, 2017 at 12:09 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday, January 28, 2017 9:48:36 PM CET Linus Walleij wrote:
>> +       pci_intc: interrupt-controller {
>> +               interrupt-controller;
>> +               #address-cells = <0>;
>> +               #interrupt-cells = <1>;
>> +       };
>
> I think this one needs at least an interrupt-parent property, otherwise
> you would recursively translate the numbers back through the interrupt-map
> property of its parent.

OK I added this, I think it was the cause of the error Hand Uli Kroll
was seeing. I guess I didn't run into it because I was only testing the
device in slot 9 so everything was defaulting to zero.

Yours,
Linus Walleij
Linus Walleij Feb. 5, 2017, 2:56 p.m. UTC | #8
On Wed, Feb 1, 2017 at 12:19 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday, January 28, 2017 9:48:36 PM CET Linus Walleij wrote:
>> +       interrupt-map-mask = <0xff00 0 0 7>;
>> +       interrupt-map = <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
>> +                       <0x4900 0 0 2 &pci_intc 1>,
>> +                       <0x4a00 0 0 3 &pci_intc 2>,
>> +                       <0x4b00 0 0 4 &pci_intc 3>,
>> +                       <0x5000 0 0 1 &pci_intc 0>, /* Slot 10 */
>> +                       <0x5100 0 0 2 &pci_intc 1>,
>> +                       <0x5200 0 0 3 &pci_intc 2>,
>> +                       <0x5300 0 0 4 &pci_intc 3>,
>> +                       <0x5800 0 0 1 &pci_intc 0>, /* Slot 11 */
>> +                       <0x5900 0 0 2 &pci_intc 1>,
>> +                       <0x5a00 0 0 3 &pci_intc 2>,
>> +                       <0x5b00 0 0 4 &pci_intc 3>,
>> +                       <0x6000 0 0 1 &pci_intc 0>, /* Slot 12 */
>> +                       <0x6100 0 0 2 &pci_intc 1>,
>> +                       <0x6200 0 0 3 &pci_intc 2>,
>> +                       <0x6300 0 0 4 &pci_intc 3>;
>>
>
> The mapping looks wrong here, we normally don't list interrupts per function
> so the mask should be 0xf800.

Yup it works that way too, and indeed the USB hub on function
2 was requesting the right pin and everything.

> Note that the interrupt map is board specific, so this should probably
> go in the board.dts file rather than platform.dtsi.

OK moving it down there.

> For this particular board, the interrupt lines appear to have been badly
> configured so all slots use the same interrupt 0 for IntA. IIRC This also
> means you can probably use <0 0 0 7> as the mask and just specify each of
> the four interrupts once. A properly wired board would swizzle the
> interrupts so that each slot has a different IRQ for its IntA line.

They are swizzled, I just have very bad hardware docs. (Only
code.) It turns out when I'm browsing through old board support
code that there is a comment followed by a piece of code like this:

/*
 *      No swizzle on SL2312
 */
static u8 __init sl2312_pci_swizzle(struct pci_dev *dev, u8 *pinp)
{
        return PCI_SLOT(dev->devfn);
}

/*
 * map the specified device/slot/pin to an IRQ.  This works out such
 * that slot 9 pin 1 is INT0, pin 2 is INT1, and slot 10 pin 1 is INT1.
 */
static int __init sl2312_pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin)
{
        int intnr = ((slot  + (pin - 1)) & 3) + 4;  /* the IRQ number
of PCI bridge */

        printk("%s : slot = %d  pin = %d \n",__func__,slot,pin);
    switch (slot)
    {
        case 12:
                if (pin==1)
                {
                        intnr = 3;
                    }
                    else
                    {
                        intnr = 0;
                    }
#ifdef CONFIG_DUAL_PCI
                    return IRQ_PCI_INTD;
#endif
            break;
        case 11:
                    intnr = (2 + (pin - 1)) & 3;
#ifdef CONFIG_DUAL_PCI
                    return IRQ_PCI_INTC;
#endif
            break;
        case 10:
                    intnr = (1 + (pin - 1)) & 3;
#ifdef CONFIG_DUAL_PCI
                    return IRQ_PCI_INTB;
#endif
            break;
        case  9:
                    intnr = (pin - 1) & 3;
            break;
    }
//      if (slot == 10)
//              intnr = (1 + (pin - 1)) & 3;
//      else if (slot == 9)
//              intnr = (pin - 1) & 3;
        return (IRQ_PCI_INTA + intnr);
}

If I understand correctly they say that the IRQs are not swizzled
on the PCI bridge side but on the IRQ handler side.

So if I put it in the device tree like so:

+                         interrupt-map =
+                               <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
+                               <0x4800 0 0 2 &pci_intc 1>,
+                               <0x4800 0 0 3 &pci_intc 2>,
+                               <0x4800 0 0 4 &pci_intc 3>,
+                               <0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
+                               <0x5000 0 0 2 &pci_intc 2>,
+                               <0x5000 0 0 3 &pci_intc 3>,
+                               <0x5000 0 0 4 &pci_intc 0>,
+                               <0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
+                               <0x5800 0 0 2 &pci_intc 3>,
+                               <0x5800 0 0 3 &pci_intc 0>,
+                               <0x5800 0 0 4 &pci_intc 1>,
+                               <0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
+                               <0x6000 0 0 2 &pci_intc 0>,
+                               <0x6000 0 0 3 &pci_intc 1>,
+                               <0x6000 0 0 4 &pci_intc 2>;

So the IRQs on the right side are swizzled instead
of the pins being swizzled.

...but that looks a bit insane.

Isn't that exactly the same thing just exposed in some
inverse way?

I'll try to get the RALink MiniPCI I have ín the slot going and
see if it works the same with just good old vanilla
swizzling.

Yours,
Linus Walleij
Arnd Bergmann Feb. 6, 2017, 4:05 p.m. UTC | #9
On Sun, Feb 5, 2017 at 3:56 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Feb 1, 2017 at 12:19 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Saturday, January 28, 2017 9:48:36 PM CET Linus Walleij wrote:
>>> +       interrupt-map-mask = <0xff00 0 0 7>;
>>> +       interrupt-map = <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
>>> +                       <0x4900 0 0 2 &pci_intc 1>,
>>> +                       <0x4a00 0 0 3 &pci_intc 2>,
>>> +                       <0x4b00 0 0 4 &pci_intc 3>,
>>> +                       <0x5000 0 0 1 &pci_intc 0>, /* Slot 10 */
>>> +                       <0x5100 0 0 2 &pci_intc 1>,
>>> +                       <0x5200 0 0 3 &pci_intc 2>,
>>> +                       <0x5300 0 0 4 &pci_intc 3>,
>>> +                       <0x5800 0 0 1 &pci_intc 0>, /* Slot 11 */
>>> +                       <0x5900 0 0 2 &pci_intc 1>,
>>> +                       <0x5a00 0 0 3 &pci_intc 2>,
>>> +                       <0x5b00 0 0 4 &pci_intc 3>,
>>> +                       <0x6000 0 0 1 &pci_intc 0>, /* Slot 12 */
>>> +                       <0x6100 0 0 2 &pci_intc 1>,
>>> +                       <0x6200 0 0 3 &pci_intc 2>,
>>> +                       <0x6300 0 0 4 &pci_intc 3>;
>>>
>>

>> For this particular board, the interrupt lines appear to have been badly
>> configured so all slots use the same interrupt 0 for IntA. IIRC This also
>> means you can probably use <0 0 0 7> as the mask and just specify each of
>> the four interrupts once. A properly wired board would swizzle the
>> interrupts so that each slot has a different IRQ for its IntA line.
>
> They are swizzled, I just have very bad hardware docs. (Only
> code.) It turns out when I'm browsing through old board support
> code that there is a comment followed by a piece of code like this:
>
> /*
>  *      No swizzle on SL2312
>  */
> static u8 __init sl2312_pci_swizzle(struct pci_dev *dev, u8 *pinp)
> {
>         return PCI_SLOT(dev->devfn);
> }
>
> /*
>  * map the specified device/slot/pin to an IRQ.  This works out such
>  * that slot 9 pin 1 is INT0, pin 2 is INT1, and slot 10 pin 1 is INT1.
>  */
> static int __init sl2312_pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin)
> {
>         int intnr = ((slot  + (pin - 1)) & 3) + 4;  /* the IRQ number
> of PCI bridge */
>
>         printk("%s : slot = %d  pin = %d \n",__func__,slot,pin);
>     switch (slot)
>     {
>         case 12:
>                 if (pin==1)
>                 {
>                         intnr = 3;
>                     }
>                     else
>                     {
>                         intnr = 0;
>                     }
> #ifdef CONFIG_DUAL_PCI
>                     return IRQ_PCI_INTD;
> #endif
>             break;
>         case 11:
>                     intnr = (2 + (pin - 1)) & 3;
> #ifdef CONFIG_DUAL_PCI
>                     return IRQ_PCI_INTC;
> #endif
>             break;
>         case 10:
>                     intnr = (1 + (pin - 1)) & 3;
> #ifdef CONFIG_DUAL_PCI
>                     return IRQ_PCI_INTB;
> #endif
>             break;
>         case  9:
>                     intnr = (pin - 1) & 3;
>             break;
>     }
> //      if (slot == 10)
> //              intnr = (1 + (pin - 1)) & 3;
> //      else if (slot == 9)
> //              intnr = (pin - 1) & 3;
>         return (IRQ_PCI_INTA + intnr);
> }
>
> If I understand correctly they say that the IRQs are not swizzled
> on the PCI bridge side but on the IRQ handler side.

That depends on whether CONFIG_DUAL_PCI is set or not:

It seems that slot 12 is hardwired to have hwirq 3 on IntA and
hwirq 0 on all others by default, but have hwirq on IntA through
IntD on boards that use CONFIG_DUAL_PCI.

Slot 10/11 use the default swizzling without CONFIG_DUAL_PCI but
uses hwirq 1 or 2 for all four pins with CONFIG_DUAL_PCI.

Slot 9 seems to always use the default swizzling, so it has all
four IRQ pins connected to the four hwirqs of the SoC.

This is still an awful wiring, but not unthinkable, as board
designers may have had a specific usage for the four interrupts
in mind, to spread the devices that are actually present across
the available hwirqs.

> So if I put it in the device tree like so:
>
> +                         interrupt-map =
> +                               <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
> +                               <0x4800 0 0 2 &pci_intc 1>,
> +                               <0x4800 0 0 3 &pci_intc 2>,
> +                               <0x4800 0 0 4 &pci_intc 3>,
> +                               <0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
> +                               <0x5000 0 0 2 &pci_intc 2>,
> +                               <0x5000 0 0 3 &pci_intc 3>,
> +                               <0x5000 0 0 4 &pci_intc 0>,
> +                               <0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
> +                               <0x5800 0 0 2 &pci_intc 3>,
> +                               <0x5800 0 0 3 &pci_intc 0>,
> +                               <0x5800 0 0 4 &pci_intc 1>,

For boards that require CONFIG_DUAL_PCI, this would be

                               <0x5000 0 0 1 &pci_intc 1>, /* Slot 10 */
                               <0x5000 0 0 2 &pci_intc 1>,
                               <0x5000 0 0 3 &pci_intc 1>,
                               <0x5000 0 0 4 &pci_intc 1>,
                               <0x5800 0 0 1 &pci_intc 2>, /* Slot 11 */
                               <0x5800 0 0 2 &pci_intc 2>,
                               <0x5800 0 0 3 &pci_intc 2>,
                               <0x5800 0 0 4 &pci_intc 2>,

It probably works either way if you put just one device in there,
as almost everything uses just IntA, but you'd notice the difference
if there is ever a PCI bridge behind that slot.

> +                               <0x6000 0 0 1 &pci_intc 3>, /* Slot 12 */
> +                               <0x6000 0 0 2 &pci_intc 0>,
> +                               <0x6000 0 0 3 &pci_intc 1>,
> +                               <0x6000 0 0 4 &pci_intc 2>;

The last two have to be

                               <0x6000 0 0 3 &pci_intc 0>, /* [sic] */
                               <0x6000 0 0 4 &pci_intc 0>; /* [sic] */

> So the IRQs on the right side are swizzled instead
> of the pins being swizzled.
>
> ...but that looks a bit insane.
>
> Isn't that exactly the same thing just exposed in some
> inverse way?

I don't understand your question. The mapping that you list
is the default swizzling, right? I think if you have a board with
sane wiring, you could write that as

                      interrupt-map-mask = <0x0 0x0 0x0 0x7>;
                      interrupt-map =
                              <0x0 0x0 0x0 0x1 &pci_intc 0>, /* int A */
                              <0x0 0x0 0x0 0x2 &pci_intc 1>, /* int B */
                              <0x0 0x0 0x0 0x3 &pci_intc 2>, /* int C */
                              <0x0 0x0 0x0 0x4 &pci_intc 3>; /* int D */

meaning that the IRQs are wired correctly to the root bus, and
all slots are connected one would normally do with the standard
swizzling.

    Arnd
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt b/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
new file mode 100644
index 000000000000..e3090d995e1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/cortina,gemini-pci.txt
@@ -0,0 +1,64 @@ 
+* Cortina Systems Gemini PCI Host Bridge
+
+Mandatory properties:
+
+- compatible: should be "cortina,gemini-pci"
+- reg: memory base and size for the host bridge
+- interrupts: the four PCI interrupts
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- #interrupt-cells: set to <1>
+- bus-range: set to <0x00 0x00> (only root bus)
+- device_type, set to "pci"
+- ranges: see pci.txt
+- interrupt-map-mask: see pci.txt
+- interrupt-map: see pci.txt
+
+Mandatory subnodes:
+- One node reprenting the interrupt-controller inside the host bridge
+  with the following mandatory properties:
+  - interrupt-controller: see interrupt-controller/interrupts.txt
+  - #address-cells: set to <0>
+  - #interrupt-cells: set to <1>
+
+Example:
+
+pci@50000000 {
+	compatible = "cortina,gemini-pci";
+	reg = <0x50000000 0x100>;
+	interrupts = <8 IRQ_TYPE_LEVEL_HIGH>, /* PCI A */
+			<26 IRQ_TYPE_LEVEL_HIGH>, /* PCI B */
+			<27 IRQ_TYPE_LEVEL_HIGH>, /* PCI C */
+			<28 IRQ_TYPE_LEVEL_HIGH>; /* PCI D */
+	#address-cells = <3>;
+	#size-cells = <2>;
+	#interrupt-cells = <1>;
+
+	bus-range = <0x00 0x00>; /* Only root bus */
+	ranges = /* 1MiB I/O space 0x50000000-0x500fffff */
+		 <0x01000000 0 0          0x50000000 0 0x00100000>,
+		 /* 128MiB non-prefetchable memory 0x58000000-0x5fffffff */
+		 <0x02000000 0 0x58000000 0x58000000 0 0x08000000>;
+	interrupt-map-mask = <0xff00 0 0 7>;
+	interrupt-map = <0x4800 0 0 1 &pci_intc 0>, /* Slot 9 */
+			<0x4900 0 0 2 &pci_intc 1>,
+			<0x4a00 0 0 3 &pci_intc 2>,
+			<0x4b00 0 0 4 &pci_intc 3>,
+			<0x5000 0 0 1 &pci_intc 0>, /* Slot 10 */
+			<0x5100 0 0 2 &pci_intc 1>,
+			<0x5200 0 0 3 &pci_intc 2>,
+			<0x5300 0 0 4 &pci_intc 3>,
+			<0x5800 0 0 1 &pci_intc 0>, /* Slot 11 */
+			<0x5900 0 0 2 &pci_intc 1>,
+			<0x5a00 0 0 3 &pci_intc 2>,
+			<0x5b00 0 0 4 &pci_intc 3>,
+			<0x6000 0 0 1 &pci_intc 0>, /* Slot 12 */
+			<0x6100 0 0 2 &pci_intc 1>,
+			<0x6200 0 0 3 &pci_intc 2>,
+			<0x6300 0 0 4 &pci_intc 3>;
+	pci_intc: interrupt-controller {
+		interrupt-controller;
+		#address-cells = <0>;
+		#interrupt-cells = <1>;
+	};
+};