diff mbox

[RFC,2/3] arm64: dts: APM X-Gene PCIe device tree nodes

Message ID 1387785725-24262-3-git-send-email-tinamdar@apm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tanmay Inamdar Dec. 23, 2013, 8:02 a.m. UTC
This patch adds the device tree nodes for APM X-Gene PCIe controller and
PCIe clock interface. Since X-Gene SOC supports maximum 5 ports, 5 dts nodes
are added.

Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
---
 arch/arm64/boot/dts/apm-mustang.dts |    4 +
 arch/arm64/boot/dts/apm-storm.dtsi  |  140 +++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+)

Comments

Jason Gunthorpe Dec. 23, 2013, 5:46 p.m. UTC | #1
On Mon, Dec 23, 2013 at 01:32:03PM +0530, Tanmay Inamdar wrote:
> This patch adds the device tree nodes for APM X-Gene PCIe controller and
> PCIe clock interface. Since X-Gene SOC supports maximum 5 ports, 5 dts nodes
> are added.

Can you include an lspci dump for PCI DT bindings please? It is
impossible to review otherwise..

Regards,
Jason
Tanmay Inamdar Jan. 2, 2014, 9:56 p.m. UTC | #2
On Mon, Dec 23, 2013 at 9:46 AM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Mon, Dec 23, 2013 at 01:32:03PM +0530, Tanmay Inamdar wrote:
>> This patch adds the device tree nodes for APM X-Gene PCIe controller and
>> PCIe clock interface. Since X-Gene SOC supports maximum 5 ports, 5 dts nodes
>> are added.
>
> Can you include an lspci dump for PCI DT bindings please? It is
> impossible to review otherwise..
>

On the X-Gene evaluation platform, there is only one PCIe port
enabled. Here is the 'lspci' dump

# lspci -vvv
00:00.0 Class 0604: Device 19aa:e008 (rev 04)
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr+ Stepping- SERR+ FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0, Cache Line Size: 64 bytes
        Region 0: Memory at <ignored> (64-bit, prefetchable)
        Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
        I/O behind bridge: 0000f000-00000fff
        Memory behind bridge: 00c00000-00cfffff
        Prefetchable memory behind bridge: 0000000000000000-0000000000bfffff
        Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- <SERR- <PERR-
        BridgeCtl: Parity+ SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
                PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
        Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00
                DevCap: MaxPayload 16384 bytes, PhantFunc 1, Latency
L0s <1us, L1 unlimited
                        ExtTag- RBE+ FLReset-
                DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
Unsupported-
                        RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                        MaxPayload 128 bytes, MaxReadReq 512 bytes
                DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq-
AuxPwr- TransPend+
                LnkCap: Port #0, Speed 8GT/s, Width x8, ASPM L0s L1,
Latency L0 unlimited, L1 unlimited
                        ClockPM- Surprise+ LLActRep+ BwNot+
                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
                LnkSta: Speed 8GT/s, Width x8, TrErr- Train- SlotClk+
DLActive+ BWMgmt- ABWMgmt-
                SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd-
HotPlug- Surprise-
                        Slot #1, PowerLimit 10.000W; Interlock- NoCompl-
                SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet-
CmdCplt- HPIrq- LinkChg-
                        Control: AttnInd Off, PwrInd Off, Power- Interlock-
                SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt-
PresDet- Interlock-
                        Changed: MRL- PresDet- LinkState+
                RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal-
PMEIntEna- CRSVisible-
                RootCap: CRSVisible-
                RootSta: PME ReqID 0000, PMEStatus- PMEPending-
                DevCap2: Completion Timeout: Not Supported,
TimeoutDis+, LTR-, OBFF Not Supported ARIFwd-
                DevCtl2: Completion Timeout: 50us to 50ms,
TimeoutDis-, LTR-, OBFF Disabled ARIFwd-
                LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- SpeedDis-
                         Transmit Margin: Normal Operating Range,
EnterModifiedCompliance- ComplianceSOS-
                         Compliance De-emphasis: -6dB
                LnkSta2: Current De-emphasis Level: -6dB,
EqualizationComplete+, EqualizationPhase1+
                         EqualizationPhase2+, EqualizationPhase3+,
LinkEqualizationRequest-
        Capabilities: [80] Power Management version 3
                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [100 v1] Advanced Error Reporting
                UESta:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UEMsk:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt-
UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
                CESta:  RxErr+ BadTLP+ BadDLLP+ Rollover- Timeout+ NonFatalErr-
                CEMsk:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
                AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
        Capabilities: [180 v1] #19
        Capabilities: [150 v1] Vendor Specific Information: ID=0001
Rev=1 Len=010 <?>

01:00.0 Class 0200: Device 15b3:1003
        Subsystem: Device 15b3:0049
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr+ Stepping- SERR+ FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0, Cache Line Size: 64 bytes
        Interrupt: pin A routed to IRQ 226
        Region 0: Memory at e000c00000 (64-bit, non-prefetchable) [size=1M]
        Region 2: Memory at e000000000 (64-bit, prefetchable) [size=8M]
        Expansion ROM at e000800000 [size=1M]
        Capabilities: [40] Power Management version 3
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [48] Vital Product Data
                Product Name: CX312A - ConnectX-3 SFP+
                Read-only fields:
                        [PN] Part number: MCX312A-XCBT
                        [EC] Engineering changes: A5
                        [SN] Serial number: MT1338X00578
                        [V0] Vendor specific: PCIe Gen3 x8
                        [RV] Reserved: checksum good, 0 byte(s) reserved
                Read/write fields:
                        [V1] Vendor specific: N/A
                        [YA] Asset tag: N/A
                        [RW] Read-write area: 105 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 253 byte(s) free
                        [RW] Read-write area: 252 byte(s) free
                End
        Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
                Vector table: BAR=0 offset=0007c000
                PBA: BAR=0 offset=0007d000
        Capabilities: [60] Express (v2) Endpoint, MSI 00
                DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
<64ns, L1 unlimited
                        ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
                DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
Unsupported-
                        RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset-
                        MaxPayload 128 bytes, MaxReadReq 512 bytes
                DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq-
AuxPwr- TransPend-
                LnkCap: Port #8, Speed 8GT/s, Width x8, ASPM L0s,
Latency L0 unlimited, L1 unlimited
                        ClockPM- Surprise- LLActRep- BwNot-
                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
                LnkSta: Speed 8GT/s, Width x8, TrErr- Train- SlotClk+
DLActive- BWMgmt- ABWMgmt-
                DevCap2: Completion Timeout: Range ABCD, TimeoutDis+,
LTR-, OBFF Not Supported
                DevCtl2: Completion Timeout: 50us to 50ms,
TimeoutDis-, LTR-, OBFF Disabled
                LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- SpeedDis-
                         Transmit Margin: Normal Operating Range,
EnterModifiedCompliance- ComplianceSOS-
                         Compliance De-emphasis: -6dB
                LnkSta2: Current De-emphasis Level: -6dB,
EqualizationComplete-, EqualizationPhase1-
                         EqualizationPhase2-, EqualizationPhase3-,
LinkEqualizationRequest-
        Capabilities: [100 v1] Alternative Routing-ID Interpretation (ARI)
                ARICap: MFVC- ACS-, Next Function: 0
                ARICtl: MFVC- ACS-, Function Group: 0
        Capabilities: [148 v1] Device Serial Number f4-52-14-03-00-0b-dc-90
        Capabilities: [154 v2] Advanced Error Reporting
                UESta:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UEMsk:  DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
                UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt-
UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
                CESta:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
                CEMsk:  RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
                AERCap: First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
        Capabilities: [18c v1] #19
        Kernel driver in use: mlx4_core


> Regards,
> Jason
Jason Gunthorpe Jan. 3, 2014, 12:52 a.m. UTC | #3
On Thu, Jan 02, 2014 at 01:56:51PM -0800, Tanmay Inamdar wrote:
> On Mon, Dec 23, 2013 at 9:46 AM, Jason Gunthorpe
> <jgunthorpe@obsidianresearch.com> wrote:
> > On Mon, Dec 23, 2013 at 01:32:03PM +0530, Tanmay Inamdar wrote:
> >> This patch adds the device tree nodes for APM X-Gene PCIe controller and
> >> PCIe clock interface. Since X-Gene SOC supports maximum 5 ports, 5 dts nodes
> >> are added.
> >
> > Can you include an lspci dump for PCI DT bindings please? It is
> > impossible to review otherwise..
> >
> 
> On the X-Gene evaluation platform, there is only one PCIe port
> enabled. Here is the 'lspci' dump

This is a bit hard to read withouth more context, but:

> # lspci -vvv
> 00:00.0 Class 0604: Device 19aa:e008 (rev 04)
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-

This is an on-chip device? (19aa does not seem to be a VID I can find)

Ideally this is the on-chip PCI-PCI bridge which represents the port.

The problem I see is that your DT binding has a top level stanza per
port.

We *really* prefer to see a single stanza for all ports - but this
requires the HW to be able to fit into the Linux resource assignment
model - a single resource pool for all ports and standard PCI-PCI
bridge config access to assign the resource to a port.

If your HW can't do this (eg because the port aperture 0xe000000000 is
hard wired) then the fall back is to place every port in a distinct
domain, with a distinct DT node and have overlapping bus numbers
and fixed windows. I don't see PCI domain support in your driver..

There is some kind of an addressing problem because you've done this:

+static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
+{
+	int i;
+
+	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+		dev->resource[i].start = dev->resource[i].end = 0;
+		dev->resource[i].flags = 0;
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_BRIDGE_DEVICEID,
+			 xgene_pcie_fixup_bridge);

Which is usually a sign that something is wonky with how the HW is
being fit into the PCI core.

> ParErr+ Stepping- SERR+ FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0, Cache Line Size: 64 bytes
>         Region 0: Memory at <ignored> (64-bit, prefetchable)
>         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>         I/O behind bridge: 0000f000-00000fff
>         Memory behind bridge: 00c00000-00cfffff

[..]
 
> 01:00.0 Class 0200: Device 15b3:1003
>         Region 0: Memory at e000c00000 (64-bit, non-prefetchable) [size=1M]
>         Region 2: Memory at e000000000 (64-bit, prefetchable)
>         [size=8M]

Something funky is going on here too, the 64 bit address e000000000
should be reflected in the 'memory behind bridge' above, not
truncated.

ranges = <0x02000000 0x0 0x00000000 0x90 0x00000000 0x0 0x10000000 /* mem*/
+				  0x01000000 0x0 0x80000000 0x90 0x80000000 0x0 0x00010000 /* io */
+				  0x00000000 0x0 0xd0000000 0x90 0xd0000000 0x0 0x00200000 /* cfg */
+				  0x00000000 0x0 0x79000000 0x00 0x79000000 0x0 0x00800000>; /* msi */

Ranges has a defined meaning, MSI shouldn't be in ranges, and 'cfg' is
only OK if the address encoding exactly matches the funky PCI-E extended
configuration address format. You can move these to regs or other
properties

(MSI is tricky, I'm not aware of DT binding work for MSI :()

Also, unrelated, can you please double check that your HW cannot
generate 8 and 16 bit configuration write TLPs natively? The
xgene_pcie_cfg_out8/16 hack is not desirable if it can be avoided.

Regards,
Jason
Tanmay Inamdar Jan. 7, 2014, 2:56 a.m. UTC | #4
On Thu, Jan 2, 2014 at 4:52 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Thu, Jan 02, 2014 at 01:56:51PM -0800, Tanmay Inamdar wrote:
>> On Mon, Dec 23, 2013 at 9:46 AM, Jason Gunthorpe
>> <jgunthorpe@obsidianresearch.com> wrote:
>> > On Mon, Dec 23, 2013 at 01:32:03PM +0530, Tanmay Inamdar wrote:
>> >> This patch adds the device tree nodes for APM X-Gene PCIe controller and
>> >> PCIe clock interface. Since X-Gene SOC supports maximum 5 ports, 5 dts nodes
>> >> are added.
>> >
>> > Can you include an lspci dump for PCI DT bindings please? It is
>> > impossible to review otherwise..
>> >
>>
>> On the X-Gene evaluation platform, there is only one PCIe port
>> enabled. Here is the 'lspci' dump
>
> This is a bit hard to read withouth more context, but:
>
>> # lspci -vvv
>> 00:00.0 Class 0604: Device 19aa:e008 (rev 04)
>>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
>
> This is an on-chip device? (19aa does not seem to be a VID I can find)

oops.. looks like vendor and device IDs are jumbled. You can look for
e008 vendor ID.
I will fix it in next version.

>
> Ideally this is the on-chip PCI-PCI bridge which represents the port.
>
> The problem I see is that your DT binding has a top level stanza per
> port.
>
> We *really* prefer to see a single stanza for all ports - but this
> requires the HW to be able to fit into the Linux resource assignment
> model - a single resource pool for all ports and standard PCI-PCI
> bridge config access to assign the resource to a port.
>
> If your HW can't do this (eg because the port aperture 0xe000000000 is
> hard wired) then the fall back is to place every port in a distinct
> domain, with a distinct DT node and have overlapping bus numbers
> and fixed windows. I don't see PCI domain support in your driver..

Thanks for the suggestion. I will think over this.

>
> There is some kind of an addressing problem because you've done this:
>
> +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
> +{
> +       int i;
> +
> +       for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> +               dev->resource[i].start = dev->resource[i].end = 0;
> +               dev->resource[i].flags = 0;
> +       }
> +}
> +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_BRIDGE_DEVICEID,
> +                        xgene_pcie_fixup_bridge);
>
> Which is usually a sign that something is wonky with how the HW is
> being fit into the PCI core.

We map the whole DDR range (eg 256 GB) into host's BAR. The Linux PCI
resource management tries to fit the host's memory into the ranges
provided (eg 0xe000000000).
Please let me know if there is any use case to do this mapping.

>
>> ParErr+ Stepping- SERR+ FastB2B- DisINTx-
>>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>> <TAbort- <MAbort- >SERR- <PERR- INTx-
>>         Latency: 0, Cache Line Size: 64 bytes
>>         Region 0: Memory at <ignored> (64-bit, prefetchable)
>>         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>>         I/O behind bridge: 0000f000-00000fff
>>         Memory behind bridge: 00c00000-00cfffff
>
> [..]
>
>> 01:00.0 Class 0200: Device 15b3:1003
>>         Region 0: Memory at e000c00000 (64-bit, non-prefetchable) [size=1M]
>>         Region 2: Memory at e000000000 (64-bit, prefetchable)
>>         [size=8M]
>
> Something funky is going on here too, the 64 bit address e000000000
> should be reflected in the 'memory behind bridge' above, not
> truncated.

That's the Mellanox device that is plugged into the system. The
device's memory gets mapped at '0xe0xxxxxxxx'

>
> ranges = <0x02000000 0x0 0x00000000 0x90 0x00000000 0x0 0x10000000 /* mem*/
> +                                 0x01000000 0x0 0x80000000 0x90 0x80000000 0x0 0x00010000 /* io */
> +                                 0x00000000 0x0 0xd0000000 0x90 0xd0000000 0x0 0x00200000 /* cfg */
> +                                 0x00000000 0x0 0x79000000 0x00 0x79000000 0x0 0x00800000>; /* msi */
>
> Ranges has a defined meaning, MSI shouldn't be in ranges, and 'cfg' is
> only OK if the address encoding exactly matches the funky PCI-E extended
> configuration address format. You can move these to regs or other
> properties

ok

>
> (MSI is tricky, I'm not aware of DT binding work for MSI :()
>

It does not. This is the range required to be mapped by the controller
to support MSI. I know it is not a standard way of doing. I was just
trying to utilize 'of_pci_range_to_resource' api.

> Also, unrelated, can you please double check that your HW cannot
> generate 8 and 16 bit configuration write TLPs natively? The
> xgene_pcie_cfg_out8/16 hack is not desirable if it can be avoided.
>

Sadly HW cannot generate 8 and 16 bit configuration transactions.

> Regards,
> Jason
Jason Gunthorpe Jan. 7, 2014, 5:27 p.m. UTC | #5
On Mon, Jan 06, 2014 at 06:56:21PM -0800, Tanmay Inamdar wrote:

> > There is some kind of an addressing problem because you've done this:
> >
> > +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> > +               dev->resource[i].start = dev->resource[i].end = 0;
> > +               dev->resource[i].flags = 0;
> > +       }
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_BRIDGE_DEVICEID,
> > +                        xgene_pcie_fixup_bridge);
> >
> > Which is usually a sign that something is wonky with how the HW is
> > being fit into the PCI core.
> 
> We map the whole DDR range (eg 256 GB) into host's BAR. The Linux PCI
> resource management tries to fit the host's memory into the ranges
> provided (eg 0xe000000000).
> Please let me know if there is any use case to do this mapping.

If you need to set the bridge's BAR like this, then the bridge is not
non-conforming.. Bridge BAR's should be 0 size unless the bridge
itself has registers.

Do any registers in this config space work properly? Does the
secondary status reflect the physical link status properly?

If it is *really* broken you might just consider hiding it from the
Linux core.

> >>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> >> <TAbort- <MAbort- >SERR- <PERR- INTx-
> >>         Latency: 0, Cache Line Size: 64 bytes
> >>         Region 0: Memory at <ignored> (64-bit, prefetchable)
> >>         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> >>         I/O behind bridge: 0000f000-00000fff
> >>         Memory behind bridge: 00c00000-00cfffff
> >
> > [..]
> >
> >> 01:00.0 Class 0200: Device 15b3:1003
> >>         Region 0: Memory at e000c00000 (64-bit, non-prefetchable) [size=1M]
> >>         Region 2: Memory at e000000000 (64-bit, prefetchable)
> >>         [size=8M]
> >
> > Something funky is going on here too, the 64 bit address e000000000
> > should be reflected in the 'memory behind bridge' above, not
> > truncated.
> 
> That's the Mellanox device that is plugged into the system. The
> device's memory gets mapped at '0xe0xxxxxxxx'

Right, but the bridge setup above has:

> >>         Memory behind bridge: 00c00000-00cfffff

Which is wrong, it doesn't include the range '0xe0xxxxxxxx'

Jason
Tanmay Inamdar Jan. 10, 2014, 1:30 a.m. UTC | #6
On Tue, Jan 7, 2014 at 9:27 AM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Mon, Jan 06, 2014 at 06:56:21PM -0800, Tanmay Inamdar wrote:
>
>> > There is some kind of an addressing problem because you've done this:
>> >
>> > +static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
>> > +{
>> > +       int i;
>> > +
>> > +       for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
>> > +               dev->resource[i].start = dev->resource[i].end = 0;
>> > +               dev->resource[i].flags = 0;
>> > +       }
>> > +}
>> > +DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_BRIDGE_DEVICEID,
>> > +                        xgene_pcie_fixup_bridge);
>> >
>> > Which is usually a sign that something is wonky with how the HW is
>> > being fit into the PCI core.
>>
>> We map the whole DDR range (eg 256 GB) into host's BAR. The Linux PCI
>> resource management tries to fit the host's memory into the ranges
>> provided (eg 0xe000000000).
>> Please let me know if there is any use case to do this mapping.
>
> If you need to set the bridge's BAR like this, then the bridge is not
> non-conforming.. Bridge BAR's should be 0 size unless the bridge
> itself has registers.

They are not set to 0 as per our hardware implementation. We have to
hide it using the fixup API. I don't know the reason but
"arch/powerpc/sysdev/xilinx_pci.c" is doing the same thing.

>
> Do any registers in this config space work properly? Does the
> secondary status reflect the physical link status properly?

Link status information is seen correctly.

>
> If it is *really* broken you might just consider hiding it from the
> Linux core.
>
>> >>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
>> >> <TAbort- <MAbort- >SERR- <PERR- INTx-
>> >>         Latency: 0, Cache Line Size: 64 bytes
>> >>         Region 0: Memory at <ignored> (64-bit, prefetchable)
>> >>         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
>> >>         I/O behind bridge: 0000f000-00000fff
>> >>         Memory behind bridge: 00c00000-00cfffff
>> >
>> > [..]
>> >
>> >> 01:00.0 Class 0200: Device 15b3:1003
>> >>         Region 0: Memory at e000c00000 (64-bit, non-prefetchable) [size=1M]
>> >>         Region 2: Memory at e000000000 (64-bit, prefetchable)
>> >>         [size=8M]
>> >
>> > Something funky is going on here too, the 64 bit address e000000000
>> > should be reflected in the 'memory behind bridge' above, not
>> > truncated.
>>
>> That's the Mellanox device that is plugged into the system. The
>> device's memory gets mapped at '0xe0xxxxxxxx'
>
> Right, but the bridge setup above has:
>
>> >>         Memory behind bridge: 00c00000-00cfffff
>
> Which is wrong, it doesn't include the range '0xe0xxxxxxxx'
>
> Jason
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/apm-mustang.dts b/arch/arm64/boot/dts/apm-mustang.dts
index 1247ca1..ab2b95f 100644
--- a/arch/arm64/boot/dts/apm-mustang.dts
+++ b/arch/arm64/boot/dts/apm-mustang.dts
@@ -24,3 +24,7 @@ 
 		reg = < 0x1 0x00000000 0x0 0x80000000 >; /* Updated by bootloader */
 	};
 };
+
+&pcie0 {
+	status = "ok";
+};
diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index d37d736..b82f430 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -176,6 +176,146 @@ 
 				reg-names = "csr-reg";
 				clock-output-names = "eth8clk";
 			};
+
+			pcie0clk: pcie0clk@1f2bc000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "pcie0clk";
+				reg = <0x0 0x1f2bc000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie0clk";
+			};
+
+			pcie1clk: pcie1clk@1f2cc000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "pcie1clk";
+				reg = <0x0 0x1f2cc000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie1clk";
+			};
+
+			pcie2clk: pcie2clk@1f2dc000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "pcie2clk";
+				reg = <0x0 0x1f2dc000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie2clk";
+			};
+
+			pcie3clk: pcie3clk@1f50c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "pcie3clk";
+				reg = <0x0 0x1f50c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie3clk";
+			};
+
+			pcie4clk: pcie4clk@1f51c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "pcie4clk";
+				reg = <0x0 0x1f51c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie4clk";
+			};
+		};
+
+		pcie0: pcie@1f2b0000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = < 0x00 0x1f2b0000 0x0 0x00010000>;
+			ranges = <0x02000000 0x0 0x00000000 0xe0 0x00000000 0x0 0x10000000 /* mem*/
+				  0x01000000 0x0 0x80000000 0xe0 0x80000000 0x0 0x00010000 /* io */
+				  0x00000000 0x0 0xd0000000 0xe0 0xd0000000 0x0 0x00200000 /* cfg */
+				  0x00000000 0x0 0x79000000 0x00 0x79000000 0x0 0x00800000>; /* msi */
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xc2 0x1>;
+			clocks = <&pcie0clk 0>;
+			clock-names = "pcieclk";
+		};
+
+		pcie1: pcie@1f2c0000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = <0x00 0x1f2c0000 0x0 0x00010000>;
+			ranges = <0x02000000 0x0 0x00000000 0xd0 0x00000000 0x0 0x10000000 /* mem*/
+				  0x01000000 0x0 0x80000000 0xd0 0x80000000 0x0 0x00010000 /* io */
+				  0x00000000 0x0 0xd0000000 0xd0 0xd0000000 0x0 0x00200000 /* cfg */
+				  0x00000000 0x0 0x79000000 0x00 0x79000000 0x0 0x00800000>; /* msi */
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xc8 0x1>;
+			clocks = <&pcie1clk 0>;
+			clock-names = "pcieclk";
+		};
+
+		pcie2: pcie@1f2d0000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg =  <0x00 0x1f2d0000 0x0 0x00010000>;
+			ranges = <0x02000000 0x0 0x00000000 0x90 0x00000000 0x0 0x10000000 /* mem*/
+				  0x01000000 0x0 0x80000000 0x90 0x80000000 0x0 0x00010000 /* io */
+				  0x00000000 0x0 0xd0000000 0x90 0xd0000000 0x0 0x00200000 /* cfg */
+				  0x00000000 0x0 0x79000000 0x00 0x79000000 0x0 0x00800000>; /* msi */
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xce 0x1>;
+			clocks = <&pcie3clk 0>;
+			clock-names = "pcieclk";
+		};
+
+		pcie3: pcie@1f500000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = <0x00 0x1f500000 0x0 0x00010000>;
+			ranges = <0x02000000 0x0 0x00000000 0xa0 0x00000000 0x0 0x10000000 /* mem*/
+				  0x01000000 0x0 0x80000000 0xa0 0x80000000 0x0 0x00010000 /* io */
+				  0x00000000 0x0 0xd0000000 0xa0 0xd0000000 0x0 0x00200000 /* cfg */
+				  0x00000000 0x0 0x79000000 0x00 0x79000000 0x0 0x00800000>; /* msi */
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xd4 0x1>;
+			clocks = <&pcie3clk 0>;
+			clock-names = "pcieclk";
+		};
+
+		pcie4: pcie@1f510000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = <0x00 0x1f510000 0x0 0x00010000>;
+			ranges = <0x02000000 0x0 0x00000000 0xc0 0x00000000 0x0 0x10000000 /* mem*/
+				  0x01000000 0x0 0x80000000 0xc0 0x80000000 0x0 0x00010000 /* io */
+				  0x00000000 0x0 0xd0000000 0xc0 0xd0000000 0x0 0x00200000 /* cfg */
+				  0x00000000 0x0 0x79000000 0x00 0x79000000 0x0 0x00800000>; /* msi */
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xda 0x1>;
+			clocks = <&pcie4clk 0>;
+			clock-names = "pcieclk";
 		};
 
 		serial0: serial@1c020000 {