linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* pci node question
@ 2012-04-20 18:37 Yoder Stuart-B08248
  2012-04-20 18:53 ` Kumar Gala
  0 siblings, 1 reply; 14+ messages in thread
From: Yoder Stuart-B08248 @ 2012-04-20 18:37 UTC (permalink / raw)
  To: Kumar Gala, linuxppc-dev

There was refactoring change a while back that moved=20
the interrupt map down into the virtual pci bridge.

example:=20
42 /* controller at 0x200000 */
43 &pci0 {
44         compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
45         device_type =3D "pci";
46         #size-cells =3D <2>;
47         #address-cells =3D <3>;
48         bus-range =3D <0x0 0xff>;
49         clock-frequency =3D <33333333>;
50         interrupts =3D <16 2 1 15>;
51         pcie@0 {
52                 reg =3D <0 0 0 0 0>;
53                 #interrupt-cells =3D <1>;
54                 #size-cells =3D <2>;
55                 #address-cells =3D <3>;
56                 device_type =3D "pci";
57                 interrupts =3D <16 2 1 15>;
58                 interrupt-map-mask =3D <0xf800 0 0 7>;
59                 interrupt-map =3D <
60                         /* IDSEL 0x0 */
61                         0000 0 0 1 &mpic 40 1 0 0
62                         0000 0 0 2 &mpic 1 1 0 0
63                         0000 0 0 3 &mpic 2 1 0 0
64                         0000 0 0 4 &mpic 3 1 0 0
65                         >;
66         };
67 };

Why was the interrupt-map moved here?

Do we really need the error interrupt specified twice?

Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??

Thanks,
Stuart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 18:37 pci node question Yoder Stuart-B08248
@ 2012-04-20 18:53 ` Kumar Gala
  2012-04-20 19:04   ` Scott Wood
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Kumar Gala @ 2012-04-20 18:53 UTC (permalink / raw)
  To: Yoder Stuart-B08248; +Cc: linuxppc-dev


On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:

> There was refactoring change a while back that moved=20
> the interrupt map down into the virtual pci bridge.
>=20
> example:=20
> 42 /* controller at 0x200000 */
> 43 &pci0 {
> 44         compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
> 45         device_type =3D "pci";
> 46         #size-cells =3D <2>;
> 47         #address-cells =3D <3>;
> 48         bus-range =3D <0x0 0xff>;
> 49         clock-frequency =3D <33333333>;
> 50         interrupts =3D <16 2 1 15>;
> 51         pcie@0 {
> 52                 reg =3D <0 0 0 0 0>;
> 53                 #interrupt-cells =3D <1>;
> 54                 #size-cells =3D <2>;
> 55                 #address-cells =3D <3>;
> 56                 device_type =3D "pci";
> 57                 interrupts =3D <16 2 1 15>;
> 58                 interrupt-map-mask =3D <0xf800 0 0 7>;
> 59                 interrupt-map =3D <
> 60                         /* IDSEL 0x0 */
> 61                         0000 0 0 1 &mpic 40 1 0 0
> 62                         0000 0 0 2 &mpic 1 1 0 0
> 63                         0000 0 0 3 &mpic 2 1 0 0
> 64                         0000 0 0 4 &mpic 3 1 0 0
> 65                         >;
> 66         };
> 67 };
>=20
> Why was the interrupt-map moved here?

Its been a while, but I think i moved it down because of which node is =
used for interrupt handling in linux.

> Do we really need the error interrupt specified twice?

I put it twice because it has multiple purposes, one has to do with =
interrupts defined by the PCI spec vs ones defined via FSL controller.

> Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??

scratching my head, what happens if you remove it?

- k

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 18:53 ` Kumar Gala
@ 2012-04-20 19:04   ` Scott Wood
  2012-04-20 20:37     ` Kumar Gala
  2012-04-20 21:19     ` Benjamin Herrenschmidt
  2012-04-20 19:50   ` Yoder Stuart-B08248
  2012-04-20 21:11   ` Benjamin Herrenschmidt
  2 siblings, 2 replies; 14+ messages in thread
From: Scott Wood @ 2012-04-20 19:04 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev

On 04/20/2012 01:53 PM, Kumar Gala wrote:
> 
> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
> 
>> There was refactoring change a while back that moved 
>> the interrupt map down into the virtual pci bridge.
>>
>> example: 
>> 42 /* controller at 0x200000 */
>> 43 &pci0 {
>> 44         compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
>> 45         device_type = "pci";
>> 46         #size-cells = <2>;
>> 47         #address-cells = <3>;
>> 48         bus-range = <0x0 0xff>;
>> 49         clock-frequency = <33333333>;
>> 50         interrupts = <16 2 1 15>;
>> 51         pcie@0 {
>> 52                 reg = <0 0 0 0 0>;
>> 53                 #interrupt-cells = <1>;
>> 54                 #size-cells = <2>;
>> 55                 #address-cells = <3>;
>> 56                 device_type = "pci";
>> 57                 interrupts = <16 2 1 15>;
>> 58                 interrupt-map-mask = <0xf800 0 0 7>;
>> 59                 interrupt-map = <
>> 60                         /* IDSEL 0x0 */
>> 61                         0000 0 0 1 &mpic 40 1 0 0
>> 62                         0000 0 0 2 &mpic 1 1 0 0
>> 63                         0000 0 0 3 &mpic 2 1 0 0
>> 64                         0000 0 0 4 &mpic 3 1 0 0
>> 65                         >;
>> 66         };
>> 67 };
>>
>> Why was the interrupt-map moved here?
> 
> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.

That's not supposed to be how device tree bindings are determined.

It's causing us problems with virtualization device assignment, because
if we just assign the parent bus we don't get the interrupt map, but if
we assign the child we need to deal with what it means to assign an
individual PCI device (e.g. on our internal KVM stuff we get an error on
that reg property).

What does this node represent in the first place?  Is there really a
PCI-to-PCI bridge?  Are all other PCI devices underneath it?

>> Do we really need the error interrupt specified twice?
> 
> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.

There are PCI-defined error conditions that cause a host controller
interrupt.  What does this have to do with the bridge node?

>> Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
> 
> scratching my head, what happens if you remove it?

Sigh.

-Scott

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: pci node question
  2012-04-20 18:53 ` Kumar Gala
  2012-04-20 19:04   ` Scott Wood
@ 2012-04-20 19:50   ` Yoder Stuart-B08248
  2012-04-20 21:11   ` Benjamin Herrenschmidt
  2 siblings, 0 replies; 14+ messages in thread
From: Yoder Stuart-B08248 @ 2012-04-20 19:50 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Wood Scott-B07421, linuxppc-dev



> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Friday, April 20, 2012 1:54 PM
> To: Yoder Stuart-B08248
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: Re: pci node question
>=20
>=20
> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>=20
> > There was refactoring change a while back that moved
> > the interrupt map down into the virtual pci bridge.
> >
> > example:
> > 42 /* controller at 0x200000 */
> > 43 &pci0 {
> > 44         compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
> > 45         device_type =3D "pci";
> > 46         #size-cells =3D <2>;
> > 47         #address-cells =3D <3>;
> > 48         bus-range =3D <0x0 0xff>;
> > 49         clock-frequency =3D <33333333>;
> > 50         interrupts =3D <16 2 1 15>;
> > 51         pcie@0 {
> > 52                 reg =3D <0 0 0 0 0>;
> > 53                 #interrupt-cells =3D <1>;
> > 54                 #size-cells =3D <2>;
> > 55                 #address-cells =3D <3>;
> > 56                 device_type =3D "pci";
> > 57                 interrupts =3D <16 2 1 15>;
> > 58                 interrupt-map-mask =3D <0xf800 0 0 7>;
> > 59                 interrupt-map =3D <
> > 60                         /* IDSEL 0x0 */
> > 61                         0000 0 0 1 &mpic 40 1 0 0
> > 62                         0000 0 0 2 &mpic 1 1 0 0
> > 63                         0000 0 0 3 &mpic 2 1 0 0
> > 64                         0000 0 0 4 &mpic 3 1 0 0
> > 65                         >;
> > 66         };
> > 67 };
> >
> > Why was the interrupt-map moved here?
>=20
> Its been a while, but I think i moved it down because of which node is us=
ed for interrupt handling in
> linux.
>=20
> > Do we really need the error interrupt specified twice?
>=20
> I put it twice because it has multiple purposes, one has to do with inter=
rupts defined by the PCI spec
> vs ones defined via FSL controller.
>=20
> > Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??
>=20
> scratching my head, what happens if you remove it?

Tried removing the zeroed reg, and get this error when bringing
up an e1000 interface:

[root@kvmtst1 root]# ifconfig eth0 192.168.1.20
e1000e 0000:01:00.0: eth0: Unable to allocate interrupt, Error: -22
SIOCSIFFLAGS: Invalid argument

Stuart

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 19:04   ` Scott Wood
@ 2012-04-20 20:37     ` Kumar Gala
  2012-04-20 20:53       ` Scott Wood
  2012-04-20 21:20       ` Benjamin Herrenschmidt
  2012-04-20 21:19     ` Benjamin Herrenschmidt
  1 sibling, 2 replies; 14+ messages in thread
From: Kumar Gala @ 2012-04-20 20:37 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev


On Apr 20, 2012, at 2:04 PM, Scott Wood wrote:

> On 04/20/2012 01:53 PM, Kumar Gala wrote:
>>=20
>> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>>=20
>>> There was refactoring change a while back that moved=20
>>> the interrupt map down into the virtual pci bridge.
>>>=20
>>> example:=20
>>> 42 /* controller at 0x200000 */
>>> 43 &pci0 {
>>> 44         compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
>>> 45         device_type =3D "pci";
>>> 46         #size-cells =3D <2>;
>>> 47         #address-cells =3D <3>;
>>> 48         bus-range =3D <0x0 0xff>;
>>> 49         clock-frequency =3D <33333333>;
>>> 50         interrupts =3D <16 2 1 15>;
>>> 51         pcie@0 {
>>> 52                 reg =3D <0 0 0 0 0>;
>>> 53                 #interrupt-cells =3D <1>;
>>> 54                 #size-cells =3D <2>;
>>> 55                 #address-cells =3D <3>;
>>> 56                 device_type =3D "pci";
>>> 57                 interrupts =3D <16 2 1 15>;
>>> 58                 interrupt-map-mask =3D <0xf800 0 0 7>;
>>> 59                 interrupt-map =3D <
>>> 60                         /* IDSEL 0x0 */
>>> 61                         0000 0 0 1 &mpic 40 1 0 0
>>> 62                         0000 0 0 2 &mpic 1 1 0 0
>>> 63                         0000 0 0 3 &mpic 2 1 0 0
>>> 64                         0000 0 0 4 &mpic 3 1 0 0
>>> 65                         >;
>>> 66         };
>>> 67 };
>>>=20
>>> Why was the interrupt-map moved here?
>>=20
>> Its been a while, but I think i moved it down because of which node =
is used for interrupt handling in linux.
>=20
> That's not supposed to be how device tree bindings are determined.
>=20
> It's causing us problems with virtualization device assignment, =
because
> if we just assign the parent bus we don't get the interrupt map, but =
if
> we assign the child we need to deal with what it means to assign an
> individual PCI device (e.g. on our internal KVM stuff we get an error =
on
> that reg property).
>=20
> What does this node represent in the first place?  Is there really a
> PCI-to-PCI bridge?  Are all other PCI devices underneath it?

PCIe has this concept of a "virtual" bridge between the root-comples, so =
that is what the node represents.  Its always been a bit confusing to me =
as its not 100% standardized by PCI sig.

Maybe Ben's got some comments to add here from a generic PCIe point of =
view.

>>> Do we really need the error interrupt specified twice?
>>=20
>> I put it twice because it has multiple purposes, one has to do with =
interrupts defined by the PCI spec vs ones defined via FSL controller.
>=20
> There are PCI-defined error conditions that cause a host controller
> interrupt.  What does this have to do with the bridge node?

Think of the "error" IRQ as shared between to classes of interrupts.  =
One set are controller errors defined by FSL, the other are errors =
defined by PCI sig / bus point of view.

I'd expect controller errors to be handled by something like EDAC would =
bind at "fsl,qoriq-pcie-v2.2" level node.  The PCI bus code would looks =
at the virtual bridge down.

>=20
>>> Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??
>>=20
>> scratching my head, what happens if you remove it?
>=20
> Sigh.

:)

- k=

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 20:37     ` Kumar Gala
@ 2012-04-20 20:53       ` Scott Wood
  2012-04-20 21:24         ` Benjamin Herrenschmidt
  2012-04-20 21:20       ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 14+ messages in thread
From: Scott Wood @ 2012-04-20 20:53 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev

On 04/20/2012 03:37 PM, Kumar Gala wrote:
> 
> On Apr 20, 2012, at 2:04 PM, Scott Wood wrote:
> 
>> On 04/20/2012 01:53 PM, Kumar Gala wrote:
>>>
>>> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>>>
>>>> There was refactoring change a while back that moved 
>>>> the interrupt map down into the virtual pci bridge.
>>>>
>>>> example: 
>>>> 42 /* controller at 0x200000 */
>>>> 43 &pci0 {
>>>> 44         compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
>>>> 45         device_type = "pci";
>>>> 46         #size-cells = <2>;
>>>> 47         #address-cells = <3>;
>>>> 48         bus-range = <0x0 0xff>;
>>>> 49         clock-frequency = <33333333>;
>>>> 50         interrupts = <16 2 1 15>;
>>>> 51         pcie@0 {
>>>> 52                 reg = <0 0 0 0 0>;
>>>> 53                 #interrupt-cells = <1>;
>>>> 54                 #size-cells = <2>;
>>>> 55                 #address-cells = <3>;
>>>> 56                 device_type = "pci";
>>>> 57                 interrupts = <16 2 1 15>;
>>>> 58                 interrupt-map-mask = <0xf800 0 0 7>;
>>>> 59                 interrupt-map = <
>>>> 60                         /* IDSEL 0x0 */
>>>> 61                         0000 0 0 1 &mpic 40 1 0 0
>>>> 62                         0000 0 0 2 &mpic 1 1 0 0
>>>> 63                         0000 0 0 3 &mpic 2 1 0 0
>>>> 64                         0000 0 0 4 &mpic 3 1 0 0
>>>> 65                         >;
>>>> 66         };
>>>> 67 };
>>>>
>>>> Why was the interrupt-map moved here?
>>>
>>> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.
>>
>> That's not supposed to be how device tree bindings are determined.
>>
>> It's causing us problems with virtualization device assignment, because
>> if we just assign the parent bus we don't get the interrupt map, but if
>> we assign the child we need to deal with what it means to assign an
>> individual PCI device (e.g. on our internal KVM stuff we get an error on
>> that reg property).
>>
>> What does this node represent in the first place?  Is there really a
>> PCI-to-PCI bridge?  Are all other PCI devices underneath it?
> 
> PCIe has this concept of a "virtual" bridge between the root-comples,

Between the root complex and whatever's plugged in?

> so that is what the node represents.  Its always been a bit confusing to me as its not 100% standardized by PCI sig.

Is it documented anywhere (e.g. in the chip manual)?  Is it common (even
if 100% standardized), or a Freescale-ism?

Is there an actual PCIe-to-PCIe bridge that shows up in the root
complex's config space (not talking about the host bridge PCI device
that has always been there on FSL PCI controllers, which we didn't
represent in the device tree on non-express PCI)?  Why does this bridge
need to be represented in the device tree (assuming no ISA devices need
to be represented), when other PCI devices (including bridges) don't?

> Maybe Ben's got some comments to add here from a generic PCIe point of view.
> 
>>>> Do we really need the error interrupt specified twice?
>>>
>>> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
>>
>> There are PCI-defined error conditions that cause a host controller
>> interrupt.  What does this have to do with the bridge node?
> 
> Think of the "error" IRQ as shared between to classes of interrupts.  One set are controller errors defined by FSL, the other are errors defined by PCI sig / bus point of view.
> 
> I'd expect controller errors to be handled by something like EDAC would bind at "fsl,qoriq-pcie-v2.2" level node.  The PCI bus code would looks at the virtual bridge down.

This shouldn't be about where a specific piece of Linux code looks.

I don't see why the split of PCI-defined errors versus FSL-defined
errors maps to bridge node versus controller node.  What if we didn't
have the bridge?  We'd still have PCI-defined errors, right?

-Scott

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 18:53 ` Kumar Gala
  2012-04-20 19:04   ` Scott Wood
  2012-04-20 19:50   ` Yoder Stuart-B08248
@ 2012-04-20 21:11   ` Benjamin Herrenschmidt
  2012-04-23 15:48     ` Yoder Stuart-B08248
  2 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2012-04-20 21:11 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev

On Fri, 2012-04-20 at 13:53 -0500, Kumar Gala wrote:
> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
> 
> > There was refactoring change a while back that moved 
> > the interrupt map down into the virtual pci bridge.
> > 
> > example: 
> > 42 /* controller at 0x200000 */
> > 43 &pci0 {
> > 44         compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
> > 45         device_type = "pci";
> > 46         #size-cells = <2>;
> > 47         #address-cells = <3>;
> > 48         bus-range = <0x0 0xff>;
> > 49         clock-frequency = <33333333>;
> > 50         interrupts = <16 2 1 15>;
> > 51         pcie@0 {
> > 52                 reg = <0 0 0 0 0>;
> > 53                 #interrupt-cells = <1>;
> > 54                 #size-cells = <2>;
> > 55                 #address-cells = <3>;
> > 56                 device_type = "pci";
> > 57                 interrupts = <16 2 1 15>;
> > 58                 interrupt-map-mask = <0xf800 0 0 7>;
> > 59                 interrupt-map = <
> > 60                         /* IDSEL 0x0 */
> > 61                         0000 0 0 1 &mpic 40 1 0 0
> > 62                         0000 0 0 2 &mpic 1 1 0 0
> > 63                         0000 0 0 3 &mpic 2 1 0 0
> > 64                         0000 0 0 4 &mpic 3 1 0 0
> > 65                         >;
> > 66         };
> > 67 };
> > 
> > Why was the interrupt-map moved here?
> 
> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.

Yeah, it would swizzle if going accross the virtual P2P bridge. On the
other hand, if it's PCIe, the children of the vP2P should always be at
device 0 and thus the swizzling should be a NOP no ? So we -could- leave
it in the PHB unless I'm mistaken...

On the other hand, we probably want to fix the mapping code to handle
things like SR-IOV PFs with different device numbers... do those get
swizzled ? In that case we might want to keep things down the virtual
bridge.

> > Do we really need the error interrupt specified twice?
> 
> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
> 
> > Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
> 
> scratching my head, what happens if you remove it?

If you remove it, the kernel will fail to match the vP2P with the
corresponding struct pci_dev.... It's not "zeroed out". It contains a
valid bus/dev/fn ... of 0,0,0 :-)

Cheers,
Ben.

> - k
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 19:04   ` Scott Wood
  2012-04-20 20:37     ` Kumar Gala
@ 2012-04-20 21:19     ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2012-04-20 21:19 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Fri, 2012-04-20 at 14:04 -0500, Scott Wood wrote:

> That's not supposed to be how device tree bindings are determined.

Ugh ? This is nothing to do with Linux, I think Kumar is confused :-)
This has to do with PCI bindings.

If you put the interrupt-map in the parent, then crossing the virtual
p2p will cause a swizzling which is not what you want. As long as the
device has a device number (in devfn) of 0 that's fine but that may not
always be the case.

> It's causing us problems with virtualization device assignment, because
> if we just assign the parent bus we don't get the interrupt map, but if
> we assign the child we need to deal with what it means to assign an
> individual PCI device (e.g. on our internal KVM stuff we get an error on
> that reg property).

I'm not sure what you are doing with device-assignement in KVM, we are
doing something different for server I suppose since the existing
assignment code in qemu-kvm is a dead end... But you should probably
synthetize an interrupt-map for the guest.

> What does this node represent in the first place?  Is there really a
> PCI-to-PCI bridge?  Are all other PCI devices underneath it?

Yes, PCIe 101 :-) It's the root complex, it's "virtual" in that it's a
construct of the host bridge, there's no physical "bridge" in the
system, but yes, PCIe always has a virtual P2P at the top to represent
the root complex.

This was done to fix the long standing problem that there wasn't a
proper way to represent host bridges as parent of their devices in PCI
land.

It allows PCIe to guarantee that a device always has a bridge above
itself, with the corresponding link control registers etc... which is
useful for point to point links :-)

That's also why for example PCIe switches look like stacks of bridges,
for example, a 2 fork switch looks like:

    |
   P2P
   / \
 P2P P2P
  |   |

That way each downstream device gets its own parent P2P with the
corresponding PCIe link control registers etc...
 
> >> Do we really need the error interrupt specified twice?
> > 
> > I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
> 
> There are PCI-defined error condition s that cause a host controller
> interrupt.  What does this have to do with the bridge node?
> 
> >> Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
> > 
> > scratching my head, what happens if you remove it?
> 
> Sigh.

As I said earlier, this is not some black magic, it's a proper reg value
for the root complex virtual bridge. It has bus 0, devfn 0, so the reg
property for the config space has a value of 0.

Without it, the kernel won't properly match it to the corresponding
pci_dev.

Cheers,
Ben.

> -Scott
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 20:37     ` Kumar Gala
  2012-04-20 20:53       ` Scott Wood
@ 2012-04-20 21:20       ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2012-04-20 21:20 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev

On Fri, 2012-04-20 at 15:37 -0500, Kumar Gala wrote:
> > What does this node represent in the first place?  Is there really a
> > PCI-to-PCI bridge?  Are all other PCI devices underneath it?
> 
> PCIe has this concept of a "virtual" bridge between the root-comples,
> so that is what the node represents.  Its always been a bit confusing
> to me as its not 100% standardized by PCI sig.

It actually is. The root complex virtual p2p is absolutely part of the
standard.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: pci node question
  2012-04-20 20:53       ` Scott Wood
@ 2012-04-20 21:24         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2012-04-20 21:24 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev


> Between the root complex and whatever's plugged in?

Yes.

> > so that is what the node represents.  Its always been a bit confusing to me as its not 100% standardized by PCI sig.

It's absolutely standard. The only case where you have "siblings" to
that RC is when it's some kind of integrated chipset with non-PCI
devices in it (still common in Intel world).

Any real PCIe device -must- have a P2P above it with the PCIe capability
& associated control registers.

> Is it documented anywhere (e.g. in the chip manual)?  Is it common (even
> if 100% standardized), or a Freescale-ism?

PCIe spec.

> Is there an actual PCIe-to-PCIe bridge that shows up in the root
> complex's config space

Yes.

>  (not talking about the host bridge PCI device
> that has always been there on FSL PCI controllers, which we didn't
> represent in the device tree on non-express PCI)?  Why does this bridge
> need to be represented in the device tree (assuming no ISA devices need
> to be represented), when other PCI devices (including bridges) don't?

You don't have to, but we sometimes do it so we can put the
interrupt-map in the right place. But again, since on PCIe the device
underneath should always have dev 0 for non-SRIOV stuff, the swizzling
shall be NOP and so having the interrupt-map all the way at the top
might work. However I'm not sure if that will work with PFs and ARI
where the dev is non-0.

> > Maybe Ben's got some comments to add here from a generic PCIe point of view.
> > 
> >>>> Do we really need the error interrupt specified twice?
> >>>
> >>> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
> >>
> >> There are PCI-defined error conditions that cause a host controller
> >> interrupt.  What does this have to do with the bridge node?
> > 
> > Think of the "error" IRQ as shared between to classes of interrupts.  One set are controller errors defined by FSL, the other are errors defined by PCI sig / bus point of view.
> > 
> > I'd expect controller errors to be handled by something like EDAC would bind at "fsl,qoriq-pcie-v2.2" level node.  The PCI bus code would looks at the virtual bridge down.
> 
> This shouldn't be about where a specific piece of Linux code looks.
> 
> I don't see why the split of PCI-defined errors versus FSL-defined
> errors maps to bridge node versus controller node.  What if we didn't
> have the bridge?  We'd still have PCI-defined errors, right?

The linux generic PCIe port driver looks for the interrupt of the bridge
for standardized PCIe AER interrupts.

Cheers,
Ben.

> -Scott
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: pci node question
  2012-04-20 21:11   ` Benjamin Herrenschmidt
@ 2012-04-23 15:48     ` Yoder Stuart-B08248
  2012-04-23 20:21       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Yoder Stuart-B08248 @ 2012-04-23 15:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Kumar Gala; +Cc: linuxppc-dev

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVuamFtaW4gSGVycmVu
c2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gU2VudDogRnJpZGF5
LCBBcHJpbCAyMCwgMjAxMiA0OjExIFBNDQo+IFRvOiBLdW1hciBHYWxhDQo+IENjOiBZb2RlciBT
dHVhcnQtQjA4MjQ4OyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZw0KPiBTdWJqZWN0OiBS
ZTogcGNpIG5vZGUgcXVlc3Rpb24NCj4gDQo+IE9uIEZyaSwgMjAxMi0wNC0yMCBhdCAxMzo1MyAt
MDUwMCwgS3VtYXIgR2FsYSB3cm90ZToNCj4gPiBPbiBBcHIgMjAsIDIwMTIsIGF0IDE6MzcgUE0s
IFlvZGVyIFN0dWFydC1CMDgyNDggd3JvdGU6DQo+ID4NCj4gPiA+IFRoZXJlIHdhcyByZWZhY3Rv
cmluZyBjaGFuZ2UgYSB3aGlsZSBiYWNrIHRoYXQgbW92ZWQNCj4gPiA+IHRoZSBpbnRlcnJ1cHQg
bWFwIGRvd24gaW50byB0aGUgdmlydHVhbCBwY2kgYnJpZGdlLg0KPiA+ID4NCj4gPiA+IGV4YW1w
bGU6DQo+ID4gPiA0MiAvKiBjb250cm9sbGVyIGF0IDB4MjAwMDAwICovDQo+ID4gPiA0MyAmcGNp
MCB7DQo+ID4gPiA0NCAgICAgICAgIGNvbXBhdGlibGUgPSAiZnNsLHAyMDQxLXBjaWUiLCAiZnNs
LHFvcmlxLXBjaWUtdjIuMiI7DQo+ID4gPiA0NSAgICAgICAgIGRldmljZV90eXBlID0gInBjaSI7
DQo+ID4gPiA0NiAgICAgICAgICNzaXplLWNlbGxzID0gPDI+Ow0KPiA+ID4gNDcgICAgICAgICAj
YWRkcmVzcy1jZWxscyA9IDwzPjsNCj4gPiA+IDQ4ICAgICAgICAgYnVzLXJhbmdlID0gPDB4MCAw
eGZmPjsNCj4gPiA+IDQ5ICAgICAgICAgY2xvY2stZnJlcXVlbmN5ID0gPDMzMzMzMzMzPjsNCj4g
PiA+IDUwICAgICAgICAgaW50ZXJydXB0cyA9IDwxNiAyIDEgMTU+Ow0KPiA+ID4gNTEgICAgICAg
ICBwY2llQDAgew0KPiA+ID4gNTIgICAgICAgICAgICAgICAgIHJlZyA9IDwwIDAgMCAwIDA+Ow0K
PiA+ID4gNTMgICAgICAgICAgICAgICAgICNpbnRlcnJ1cHQtY2VsbHMgPSA8MT47DQo+ID4gPiA1
NCAgICAgICAgICAgICAgICAgI3NpemUtY2VsbHMgPSA8Mj47DQo+ID4gPiA1NSAgICAgICAgICAg
ICAgICAgI2FkZHJlc3MtY2VsbHMgPSA8Mz47DQo+ID4gPiA1NiAgICAgICAgICAgICAgICAgZGV2
aWNlX3R5cGUgPSAicGNpIjsNCj4gPiA+IDU3ICAgICAgICAgICAgICAgICBpbnRlcnJ1cHRzID0g
PDE2IDIgMSAxNT47DQo+ID4gPiA1OCAgICAgICAgICAgICAgICAgaW50ZXJydXB0LW1hcC1tYXNr
ID0gPDB4ZjgwMCAwIDAgNz47DQo+ID4gPiA1OSAgICAgICAgICAgICAgICAgaW50ZXJydXB0LW1h
cCA9IDwNCj4gPiA+IDYwICAgICAgICAgICAgICAgICAgICAgICAgIC8qIElEU0VMIDB4MCAqLw0K
PiA+ID4gNjEgICAgICAgICAgICAgICAgICAgICAgICAgMDAwMCAwIDAgMSAmbXBpYyA0MCAxIDAg
MA0KPiA+ID4gNjIgICAgICAgICAgICAgICAgICAgICAgICAgMDAwMCAwIDAgMiAmbXBpYyAxIDEg
MCAwDQo+ID4gPiA2MyAgICAgICAgICAgICAgICAgICAgICAgICAwMDAwIDAgMCAzICZtcGljIDIg
MSAwIDANCj4gPiA+IDY0ICAgICAgICAgICAgICAgICAgICAgICAgIDAwMDAgMCAwIDQgJm1waWMg
MyAxIDAgMA0KPiA+ID4gNjUgICAgICAgICAgICAgICAgICAgICAgICAgPjsNCj4gPiA+IDY2ICAg
ICAgICAgfTsNCj4gPiA+IDY3IH07DQo+ID4gPg0KPiA+ID4gV2h5IHdhcyB0aGUgaW50ZXJydXB0
LW1hcCBtb3ZlZCBoZXJlPw0KPiA+DQo+ID4gSXRzIGJlZW4gYSB3aGlsZSwgYnV0IEkgdGhpbmsg
aSBtb3ZlZCBpdCBkb3duIGJlY2F1c2Ugb2Ygd2hpY2ggbm9kZSBpcyB1c2VkIGZvciBpbnRlcnJ1
cHQgaGFuZGxpbmcgaW4NCj4gbGludXguDQo+IA0KPiBZZWFoLCBpdCB3b3VsZCBzd2l6emxlIGlm
IGdvaW5nIGFjY3Jvc3MgdGhlIHZpcnR1YWwgUDJQIGJyaWRnZS4gT24gdGhlDQo+IG90aGVyIGhh
bmQsIGlmIGl0J3MgUENJZSwgdGhlIGNoaWxkcmVuIG9mIHRoZSB2UDJQIHNob3VsZCBhbHdheXMg
YmUgYXQNCj4gZGV2aWNlIDAgYW5kIHRodXMgdGhlIHN3aXp6bGluZyBzaG91bGQgYmUgYSBOT1Ag
bm8gPyBTbyB3ZSAtY291bGQtIGxlYXZlDQo+IGl0IGluIHRoZSBQSEIgdW5sZXNzIEknbSBtaXN0
YWtlbi4uLg0KPiANCj4gT24gdGhlIG90aGVyIGhhbmQsIHdlIHByb2JhYmx5IHdhbnQgdG8gZml4
IHRoZSBtYXBwaW5nIGNvZGUgdG8gaGFuZGxlDQo+IHRoaW5ncyBsaWtlIFNSLUlPViBQRnMgd2l0
aCBkaWZmZXJlbnQgZGV2aWNlIG51bWJlcnMuLi4gZG8gdGhvc2UgZ2V0DQo+IHN3aXp6bGVkID8g
SW4gdGhhdCBjYXNlIHdlIG1pZ2h0IHdhbnQgdG8ga2VlcCB0aGluZ3MgZG93biB0aGUgdmlydHVh
bA0KPiBicmlkZ2UuDQo+IA0KPiA+ID4gRG8gd2UgcmVhbGx5IG5lZWQgdGhlIGVycm9yIGludGVy
cnVwdCBzcGVjaWZpZWQgdHdpY2U/DQo+ID4NCj4gPiBJIHB1dCBpdCB0d2ljZSBiZWNhdXNlIGl0
IGhhcyBtdWx0aXBsZSBwdXJwb3Nlcywgb25lIGhhcyB0byBkbyB3aXRoIGludGVycnVwdHMgZGVm
aW5lZCBieSB0aGUgUENJIHNwZWMNCj4gdnMgb25lcyBkZWZpbmVkIHZpYSBGU0wgY29udHJvbGxl
ci4NCj4gPg0KPiA+ID4gV2h5IGlzIHRoZXJlIGEgemVyb2VkIG91dCByZWcgcHJvcGVydHk6IHJl
ZyA9IDwwIDAgMCAwIDA+ID8/DQo+ID4NCj4gPiBzY3JhdGNoaW5nIG15IGhlYWQsIHdoYXQgaGFw
cGVucyBpZiB5b3UgcmVtb3ZlIGl0Pw0KPiANCj4gSWYgeW91IHJlbW92ZSBpdCwgdGhlIGtlcm5l
bCB3aWxsIGZhaWwgdG8gbWF0Y2ggdGhlIHZQMlAgd2l0aCB0aGUNCj4gY29ycmVzcG9uZGluZyBz
dHJ1Y3QgcGNpX2Rldi4uLi4gSXQncyBub3QgInplcm9lZCBvdXQiLiBJdCBjb250YWlucyBhDQo+
IHZhbGlkIGJ1cy9kZXYvZm4gLi4uIG9mIDAsMCwwIDotKQ0KDQpIbW1tLi4uIEkgdmFndWVseSB1
bmRlcnN0YW5kIHdoYXQgeW91IGFyZSBzYXlpbmcuICAgQnV0IHdoeSBpcyB0aGUNCmxlbmd0aCB6
ZXJvLCB0aGF0IGlzIHdoYXQgZG9lc24ndCBtYWtlIHNlbnNlIG9uIHRoZSBzdXJmYWNlPw0KDQpX
ZSByZWFsbHkgc2hvdWxkIHB1dCBhIGNvbW1lbnQgb24gdGhhdCByZWcgcHJvcGVydHktLSBjYW4g
eW91IHN1Z2dlc3QNCmEgc2hvcnQgY29tbWVudCB0aGF0IHdvdWxkIGNhcHR1cmUgd2hhdCB0aGUg
cmVnIHdpdGggYWxsIHplcm9zIA0KaXMgZm9yPyAgLi4uSSdsbCBzdWJtaXQgYSBwYXRjaC4NCg0K
VGhhbmtzLA0KU3R1YXJ0DQo=

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: pci node question
  2012-04-23 15:48     ` Yoder Stuart-B08248
@ 2012-04-23 20:21       ` Benjamin Herrenschmidt
  2012-04-23 20:32         ` Yoder Stuart-B08248
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2012-04-23 20:21 UTC (permalink / raw)
  To: Yoder Stuart-B08248; +Cc: linuxppc-dev

On Mon, 2012-04-23 at 15:48 +0000, Yoder Stuart-B08248 wrote:
> Hmmm... I vaguely understand what you are saying.   But why is the
> length zero, that is what doesn't make sense on the surface?

Ah indeed, on would think that we could represent the real size of the
config space there... but I've always seen this as 0 in real OF
implementations as well, at least the G5 device-tree I'm looking at now
has it that way.

> We really should put a comment on that reg property-- can you suggest
> a short comment that would capture what the reg with all zeros 
> is for?  ...I'll submit a patch. 

Hrm.. "config address of the device: 0,0,0" ?

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: pci node question
  2012-04-23 20:21       ` Benjamin Herrenschmidt
@ 2012-04-23 20:32         ` Yoder Stuart-B08248
  2012-04-23 20:40           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Yoder Stuart-B08248 @ 2012-04-23 20:32 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVuamFtaW4gSGVycmVu
c2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gU2VudDogTW9uZGF5
LCBBcHJpbCAyMywgMjAxMiAzOjIyIFBNDQo+IFRvOiBZb2RlciBTdHVhcnQtQjA4MjQ4DQo+IENj
OiBLdW1hciBHYWxhOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZw0KPiBTdWJqZWN0OiBS
RTogcGNpIG5vZGUgcXVlc3Rpb24NCj4gDQo+IE9uIE1vbiwgMjAxMi0wNC0yMyBhdCAxNTo0OCAr
MDAwMCwgWW9kZXIgU3R1YXJ0LUIwODI0OCB3cm90ZToNCj4gPiBIbW1tLi4uIEkgdmFndWVseSB1
bmRlcnN0YW5kIHdoYXQgeW91IGFyZSBzYXlpbmcuICAgQnV0IHdoeSBpcyB0aGUNCj4gPiBsZW5n
dGggemVybywgdGhhdCBpcyB3aGF0IGRvZXNuJ3QgbWFrZSBzZW5zZSBvbiB0aGUgc3VyZmFjZT8N
Cj4gDQo+IEFoIGluZGVlZCwgb24gd291bGQgdGhpbmsgdGhhdCB3ZSBjb3VsZCByZXByZXNlbnQg
dGhlIHJlYWwgc2l6ZSBvZiB0aGUNCj4gY29uZmlnIHNwYWNlIHRoZXJlLi4uIGJ1dCBJJ3ZlIGFs
d2F5cyBzZWVuIHRoaXMgYXMgMCBpbiByZWFsIE9GDQo+IGltcGxlbWVudGF0aW9ucyBhcyB3ZWxs
LCBhdCBsZWFzdCB0aGUgRzUgZGV2aWNlLXRyZWUgSSdtIGxvb2tpbmcgYXQgbm93DQo+IGhhcyBp
dCB0aGF0IHdheS4NCj4gDQo+ID4gV2UgcmVhbGx5IHNob3VsZCBwdXQgYSBjb21tZW50IG9uIHRo
YXQgcmVnIHByb3BlcnR5LS0gY2FuIHlvdSBzdWdnZXN0DQo+ID4gYSBzaG9ydCBjb21tZW50IHRo
YXQgd291bGQgY2FwdHVyZSB3aGF0IHRoZSByZWcgd2l0aCBhbGwgemVyb3MNCj4gPiBpcyBmb3I/
ICAuLi5JJ2xsIHN1Ym1pdCBhIHBhdGNoLg0KPiANCj4gSHJtLi4gImNvbmZpZyBhZGRyZXNzIG9m
IHRoZSBkZXZpY2U6IDAsMCwwIiA/DQoNCkhvdyBhYm91dDoNCg0KICAvKiBjb25maWcgc3BhY2Ug
YWRkcmVzcyBvZiB0aGUgZGV2aWNlIDAsMCwwLiBPRiBjb252ZW50aW9uIGlzIHRoYXQNCiAgICAg
dGhlIHNpemUgb2YgY29uZmlnIHNwYWNlIGlzIDAuICovDQoNCi4uLmlzIHRoYXQgYWNjdXJhdGU/
DQoNClN0dWFydA0K

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: pci node question
  2012-04-23 20:32         ` Yoder Stuart-B08248
@ 2012-04-23 20:40           ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2012-04-23 20:40 UTC (permalink / raw)
  To: Yoder Stuart-B08248; +Cc: linuxppc-dev

On Mon, 2012-04-23 at 20:32 +0000, Yoder Stuart-B08248 wrote:
>   /* config space address of the device 0,0,0. OF convention is that
>      the size of config space is 0. */
> 
> ...is that accurate? 

Dunno about OF convention here, but it's what I've seen done. In any
case, anything with a PCI device in the .dts will have that, I don't
know whether that's worth putting such a large comment in every .dts
around...

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-04-23 20:40 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-20 18:37 pci node question Yoder Stuart-B08248
2012-04-20 18:53 ` Kumar Gala
2012-04-20 19:04   ` Scott Wood
2012-04-20 20:37     ` Kumar Gala
2012-04-20 20:53       ` Scott Wood
2012-04-20 21:24         ` Benjamin Herrenschmidt
2012-04-20 21:20       ` Benjamin Herrenschmidt
2012-04-20 21:19     ` Benjamin Herrenschmidt
2012-04-20 19:50   ` Yoder Stuart-B08248
2012-04-20 21:11   ` Benjamin Herrenschmidt
2012-04-23 15:48     ` Yoder Stuart-B08248
2012-04-23 20:21       ` Benjamin Herrenschmidt
2012-04-23 20:32         ` Yoder Stuart-B08248
2012-04-23 20:40           ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).