| Message ID | 00c501ce277c$30e49dc0$92add940$%han@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Mar 23, 2013 at 01:09:18PM +0900, Jingoo Han wrote: > + pcie0@40000000 { > + compatible = "samsung,exynos5440-pcie"; > + reg = <0x40000000 0x4000 > + 0x290000 0x1000 > + 0x270000 0x1000 > + 0x271000 0x40>; > + interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + bus-range = <0x0 0xf>; > + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > + }; Can you send the lspci output so these bindings can be properly reviewed? What PCI devices are internal to the SOC? What is behind 'exynos_pcie_wr_own_conf' ? Is this a root port bridge config space? What line is it in the lspci output? Can you include a lspci -vv for it as well? Your DT has overlapping bus-ranges, and two top level nodes. This is going to require separate PCI domains in Linux. However, based on your driver this HW looks similar to tegra, did you review how tegra is setup? Merging all the ports into a single domain is certainly preferred. > + pcie1@60000000 { > + compatible = "samsung,exynos5440-pcie"; > + reg = <0x60000000 0x4000 > + 0x2a0000 0x1000 > + 0x272000 0x1000 > + 0x271040 0x40>; > + interrupts = <0 23 0>, <0 24 0>, <0 25 0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + bus-range = <0x0 0xf>; > + ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00200000 /* configuration space */ Do not include configuration space in ranges > + 0x81000000 0 0 0x60200000 0 0x00004000 /* downstream I/O */ Please confirm that an MMIO to 0x60200000 produces a PCI-E IO TLP to address 0 > + 0x82000000 0 0 0x60204000 0 0x10000000>; /* non-prefetchable memory */ Please check this, generally it should be: 0x82000000 0 0x60204000 0x60204000 0 0x10000000>; /* non-prefetchable memory */ Reflecting an identity mapping for MMIO - eg MMIO access to 0x60204000 producse a PCI Memory TLP to address 0x60204000 - unless your hardware is actually doing address translation (then there are other things to confirm..) It is usual to have an interrupt-map - have you tested that interrupts resolve properly? It looks like the INTx's should be routed by an interrupt-map to the pulse pin. Consider an interrupt controller to decode the INT ABCD. Regards, Jason
On Tuesday, March 26, 2013 2:05 AM, Jason Gunthorpe wrote: > > On Sat, Mar 23, 2013 at 01:09:18PM +0900, Jingoo Han wrote: > > > + pcie0@40000000 { > > + compatible = "samsung,exynos5440-pcie"; > > + reg = <0x40000000 0x4000 > > + 0x290000 0x1000 > > + 0x270000 0x1000 > > + 0x271000 0x40>; > > + interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + device_type = "pci"; > > + bus-range = <0x0 0xf>; > > + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > > + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > > + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > > + }; > > Can you send the lspci output so these bindings can be properly > reviewed? What PCI devices are internal to the SOC? > > What is behind 'exynos_pcie_wr_own_conf' ? Is this a root port bridge > config space? What line is it in the lspci output? Can you include a > lspci -vv for it as well? Hi Jason Gunthorpe, Thank you for your comment :) Here is the lspci -vv output. I tested Exynos PCIe with e1000e lan card. 00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) 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 Bus: primary=00, secondary=01, subordinate=01, sec-latency=0 I/O behind bridge: 00000000-00000fff Memory behind bridge: 40300000-403fffff Prefetchable memory behind bridge: 40400000-404fffff 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: <access denied> Kernel driver in use: pcieport 01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection Subsystem: Intel Corporation Gigabit CT Desktop Adapter 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 53 Region 0: Memory at 40380000 (32-bit, non-prefetchable) [size=128K] Region 1: Memory at 40300000 (32-bit, non-prefetchable) [size=512K] Region 2: I/O ports at 40200000 [disabled] [size=32] Region 3: Memory at 403a0000 (32-bit, non-prefetchable) [size=16K] [virtual] Expansion ROM at 40400000 [disabled] [size=256K] Capabilities: <access denied> Kernel driver in use: e1000e 10:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) 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 Bus: primary=10, secondary=11, subordinate=11, sec-latency=0 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: <access denied> Kernel driver in use: pcieport > > Your DT has overlapping bus-ranges, and two top level nodes. This is > going to require separate PCI domains in Linux. > > However, based on your driver this HW looks similar to tegra, did you > review how tegra is setup? Merging all the ports into a single domain > is certainly preferred. In Tegra case, the address of IO control register is same. + pcie-controller { + compatible = "nvidia,tegra20-pcie"; + reg = <0x80003000 0x00000800 /* PADS registers */ + 0x80003800 0x00000200 /* AFI registers */ + 0x81000000 0x01000000 /* configuration space */ + 0x90000000 0x10000000>; /* extended configuration space */ But, in Exynos case, address of IP control register is different between PCIe0 and PCIe1. + pcie0@40000000 { + compatible = "samsung,exynos5440-pcie"; + reg = <0x40000000 0x4000 + 0x290000 0x1000 + 0x270000 0x1000 + 0x271000 0x40>; + pcie1@60000000 { + compatible = "samsung,exynos5440-pcie"; + reg = <0x60000000 0x4000 + 0x2a0000 0x1000 + 0x272000 0x1000 + 0x271040 0x40>; > > > + pcie1@60000000 { > > + compatible = "samsung,exynos5440-pcie"; > > + reg = <0x60000000 0x4000 > > + 0x2a0000 0x1000 > > + 0x272000 0x1000 > > + 0x271040 0x40>; > > + interrupts = <0 23 0>, <0 24 0>, <0 25 0>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + device_type = "pci"; > > + bus-range = <0x0 0xf>; > > + ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00200000 /* configuration space */ > > Do not include configuration space in ranges How can I include configuration space? Please let me know kindly :) > > > + 0x81000000 0 0 0x60200000 0 0x00004000 /* downstream I/O */ > > Please confirm that an MMIO to 0x60200000 produces a PCI-E IO TLP to > address 0 > > > + 0x82000000 0 0 0x60204000 0 0x10000000>; /* non-prefetchable memory */ > > Please check this, generally it should be: > > 0x82000000 0 0x60204000 0x60204000 0 0x10000000>; /* non-prefetchable memory */ > > Reflecting an identity mapping for MMIO - eg MMIO access to 0x60204000 > producse a PCI Memory TLP to address 0x60204000 - unless your hardware > is actually doing address translation (then there are other things to > confirm..) OK, I will change it. > > It is usual to have an interrupt-map - have you tested that interrupts > resolve properly? There is no problem about interrupts. However, I will consider an interrupt-map. > > It looks like the INTx's should be routed by an interrupt-map to the > pulse pin. Consider an interrupt controller to decode the INT ABCD. > > Regards, > Jason
On Wed, Mar 27, 2013 at 05:35:48PM +0900, Jingoo Han wrote: > Here is the lspci -vv output. > I tested Exynos PCIe with e1000e lan card. > > 00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > 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 > Bus: primary=00, secondary=01, subordinate=01, sec-latency=0 > I/O behind bridge: 00000000-00000fff > Memory behind bridge: 40300000-403fffff > Prefetchable memory behind bridge: 40400000-404fffff > 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: <access denied> > Kernel driver in use: pcieport This is very similar to tegra, you should try to follow the same path as Thierry did. Basically, have only one PCI host from Linux's perspective. This means the driver only calls pci_create_root_bus once. The driver makes all the ports available under that single root_bus. It does this by routing the config access to the four different MMIO config regions depending on the bus number and device number. When done, lspci should look something like this: 00:01.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) 00:02.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) Notice the bus number of both root port bridges is 0. This is now compliant with the PCI-E specification for root complex behavior. Bus 0 is the root complex bus. The driver can then use this information in the bridges: > Bus: primary=00, secondary=01, subordinate=01, sec-latency=0 To route config transactions for bus != 0 to the proper port and correctly generate type 0 or type 1 config TLPs. I hope this makes sense. Tegra's implementation is very close to this, but the bus != 0 case will be more like Marvell. (If I read your driver right) > 10:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) > (prog-if 00 [Normal decode]) There is something funny here, this is bus 0x10, but your bindings had bus-range 0 -> 0xf on both nodes. > > However, based on your driver this HW looks similar to tegra, did you > > review how tegra is setup? Merging all the ports into a single domain > > is certainly preferred. > > In Tegra case, the address of IO control register is same. > + pcie-controller { > + compatible = "nvidia,tegra20-pcie"; > + reg = <0x80003000 0x00000800 /* PADS registers */ > + 0x80003800 0x00000200 /* AFI registers */ > + 0x81000000 0x01000000 /* configuration space */ > + 0x90000000 0x10000000>; /* extended configuration space */ > > But, in Exynos case, address of IP control register is different > between PCIe0 and PCIe1. tegra has both shared registers and per-port registers. The ones above are shared. This message has the final DT bindings for the Marvell and tegra cases: http://permalink.gmane.org/gmane.linux.kernel.pci/21209 The per-port registers are being passed to the driver here: pci@1,0 { device_type = "pci"; assigned-addresses = <0x82000800 0 0x80000000 0 0x1000>; reg = <0x000800 0 0 0 0>; via assigned-addresses. Marvell has no shared registers, you can see in its binding they are all passed through assigned-addresses. Also, the above DT nodes is representing the root port bridge PCI device. In your case it would be this: 00:01.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) 0x800 is the DT encoding of 00:01.0 > > > + ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00200000 /* configuration space */ > > > > Do not include configuration space in ranges > > How can I include configuration space? > Please let me know kindly :) There is no need to specify configuration space at all. If you copied this from an older tegra binding it was an early way to try and define per-port registers. After discussion the use of assigned-addresses was choosen instead. > > It is usual to have an interrupt-map - have you tested that interrupts > > resolve properly? > > There is no problem about interrupts. I see, you have exynos_pcie_map_irq in code. interrupt-map replaces that functionality in a standard way, and is more capable for edge cases. > However, I will consider an interrupt-map. You can copy the interrupt-map style from the Marvell driver, which seems like it matches your case.. Jason
On Saturday, March 23, 2013 1:09 PM, Jingoo Han wrote: > > Exynos5440 has two PCIe controllers which can be used as root complex > for PCIe interface. > > Signed-off-by: Jingoo Han <jg1.han@samsung.com> > --- > arch/arm/boot/dts/exynos5440-ssdk5440.dts | 8 +++++++ > arch/arm/boot/dts/exynos5440.dtsi | 32 +++++++++++++++++++++++++++++ > 2 files changed, 40 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/boot/dts/exynos5440-ssdk5440.dts b/arch/arm/boot/dts/exynos5440-ssdk5440.dts > index a21eb4c..746f9fc 100644 > --- a/arch/arm/boot/dts/exynos5440-ssdk5440.dts > +++ b/arch/arm/boot/dts/exynos5440-ssdk5440.dts > @@ -34,4 +34,12 @@ > clock-frequency = <50000000>; > }; > }; > + > + pcie0@40000000 { > + reset-gpio = <5>; > + }; > + > + pcie1@60000000 { > + reset-gpio = <22>; > + }; > }; > diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi > index c374a31..41b2d2c 100644 > --- a/arch/arm/boot/dts/exynos5440.dtsi > +++ b/arch/arm/boot/dts/exynos5440.dtsi > @@ -178,4 +178,36 @@ > clocks = <&clock 21>; > clock-names = "rtc"; > }; > + > + pcie0@40000000 { > + compatible = "samsung,exynos5440-pcie"; > + reg = <0x40000000 0x4000 > + 0x290000 0x1000 > + 0x270000 0x1000 > + 0x271000 0x40>; > + interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + bus-range = <0x0 0xf>; > + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > + }; > + > + pcie1@60000000 { > + compatible = "samsung,exynos5440-pcie"; > + reg = <0x60000000 0x4000 > + 0x2a0000 0x1000 > + 0x272000 0x1000 > + 0x271040 0x40>; > + interrupts = <0 23 0>, <0 24 0>, <0 25 0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + bus-range = <0x0 0xf>; > + ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00200000 /* configuration space */ > + 0x81000000 0 0 0x60200000 0 0x00004000 /* downstream I/O */ > + 0x82000000 0 0 0x60204000 0 0x10000000>; /* non-prefetchable memory */ > + }; Hi Jason, I have a question. Now, I am reviewing the Tegra PCIe, Marvell PCIe patchset. However, in the case of Exynos PCIe, 'downstream I/O' and 'non-prefetchable memory' are different between PCIe0 and PCIe1. These regions are not shared. PCIe0: ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ PCIe1: ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ PCIe0 uses 0x40000000~0x5fffffff, PCI1 uses 0x60000000~0x7fffffff. How can I handle this? :) The following is right? + pcie-controller { ..... + ranges = <0x82000000 0 0x40000000 0x40000000 0 0x00200000 /* port 0 registers */ + 0x82000000 0 0x60000000 0x60000000 0 0x00200000 /* port 1 registers */ + 0x81000000 0 0 0x40200000 0 0x00004000 /* port 0 downstream I/O */ + 0x81000000 0 0 0x60200000 0 0x00004000 /* port 1 downstream I/O */ + 0x82000000 0 0x40204000 0x40204000 0 0x10000000>; /* port 0 non-prefetchable memory */ + 0x82000000 0 0x40204000 0x60204000 0 0x10000000>; /* port 1 non-prefetchable memory */ + + pci@1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x40000000 0 0x00200000 + 0x81000800 0 0x40200000 0 0x00004000 + 0x81000800 0 0x40204000 0 0x10000000>; ..... + pci@2,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x60000000 0 0x00200000 + 0x81000800 0 0x60200000 0 0x00004000 + 0x81000800 0 0x60204000 0 0x10000000>; Best regards, Jingoo Han > }; > -- > 1.7.2.5
On Mon, Apr 08, 2013 at 06:08:53PM +0900, Jingoo Han wrote: > I have a question. Now, I am reviewing the Tegra PCIe, Marvell PCIe > patchset. However, in the case of Exynos PCIe, 'downstream I/O' and > 'non-prefetchable memory' are different between PCIe0 and PCIe1. > These regions are not shared. > > PCIe0: > ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > > PCIe1: > ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > > PCIe0 uses 0x40000000~0x5fffffff, PCI1 uses 0x60000000~0x7fffffff. > > How can I handle this? :) You need to dig into where this range restriction comes from, and how it interacts with the PCI-E root bridge's window registers. Is there another set of registers that control this? Is it hardwired into the silicon? Do the root port window registers control this? I'm looking at functions like exynos_pcie_prog_viewport_mem_outbound and wondering if the driver already controls this window.. But it looks like there may be some restrictions. Marvell also has unshared regions, but the driver arranges for those ranges to be setup dynamically based on writes to the bridge's window registers from the Linux PCI core, so the region is always in sync with what the Linux PCI core is trying to do. The desired perfect outcome is to have a single logical 'shared' region for memory and I/O - give that region to the PCI core via struct resources, then the PCI core tells the driver and HW what portion of that region belongs to each root port via a write to the root port bridge's window registers. The net result is still non-overlapping regions, but the allocation of space between port 0 and port 1 is performed at run time. I don't really know enough about your hardware to give you better advice, sorry. The general guidance to try and follow the PCI-E spec for a root complex is good, but if the HW can't do it, or it would make the driver too complex, then one PCI domain per port will always work (this is similar to your original driver, but with domains). The main advantage to following the PCI-E specs and allowing for dynamic allocation of address space is that it lets you reserve less address space for PCI-E, and this in turn gives you more low mem address space to use for DRAM. > The following is right? > + pcie-controller { > ..... > + ranges = <0x82000000 0 0x40000000 0x40000000 0 0x00200000 /* port 0 registers */ > + 0x82000000 0 0x60000000 0x60000000 0 0x00200000 /* port 1 registers */ > + 0x81000000 0 0 0x40200000 0 0x00004000 /* port 0 downstream I/O */ > + 0x81000000 0 0 0x60200000 0 0x00004000 /* port 1 downstream I/O */ > + 0x82000000 0 0x40204000 0x40204000 0 0x10000000>; /* port 0 non-prefetchable memory */ > + 0x82000000 0 0x40204000 0x60204000 0 0x10000000>; /* port 1 non-prefetchable memory */ > + > + pci@1,0 { > + device_type = "pci"; > + assigned-addresses = <0x82000800 0 0x40000000 0 0x00200000 > + 0x81000800 0 0x40200000 0 0x00004000 > + 0x81000800 0 0x40204000 0 0x10000000>; Would be: ranges = <0x81000800 0 0x40200000 0x81000800 0 0x40200000 0 0x00004000 0x81000800 0 0x40204000 0x81000800 0 0x40204000 0 0x10000000>; assigned-addresses = <0x82000800 0 0x40000000 0 0x00200000>; Jason
On Tuesday, April 09, 2013 1:56 AM, Jason Gunthorpe wrote: > On Mon, Apr 08, 2013 at 06:08:53PM +0900, Jingoo Han wrote: > > > I have a question. Now, I am reviewing the Tegra PCIe, Marvell PCIe > > patchset. However, in the case of Exynos PCIe, 'downstream I/O' and > > 'non-prefetchable memory' are different between PCIe0 and PCIe1. > > These regions are not shared. > > > > PCIe0: > > ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > > 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > > 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > > > > PCIe1: > > ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > > 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > > 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > > > > PCIe0 uses 0x40000000~0x5fffffff, PCI1 uses 0x60000000~0x7fffffff. > > > > How can I handle this? :) > > You need to dig into where this range restriction comes from, and how > it interacts with the PCI-E root bridge's window registers. Is there > another set of registers that control this? Is it hardwired into the > silicon? Do the root port window registers control this? > > I'm looking at functions like exynos_pcie_prog_viewport_mem_outbound > and wondering if the driver already controls this window.. But it > looks like there may be some restrictions. > > Marvell also has unshared regions, but the driver arranges for those > ranges to be setup dynamically based on writes to the bridge's window > registers from the Linux PCI core, so the region is always in sync > with what the Linux PCI core is trying to do. > > The desired perfect outcome is to have a single logical 'shared' > region for memory and I/O - give that region to the PCI core via > struct resources, then the PCI core tells the driver and HW what > portion of that region belongs to each root port via a write to the > root port bridge's window registers. The net result is still > non-overlapping regions, but the allocation of space between port 0 > and port 1 is performed at run time. > > I don't really know enough about your hardware to give you better > advice, sorry. The general guidance to try and follow the PCI-E spec > for a root complex is good, but if the HW can't do it, or it would > make the driver too complex, then one PCI domain per port will always > work (this is similar to your original driver, but with domains). > > The main advantage to following the PCI-E specs and allowing for > dynamic allocation of address space is that it lets you reserve less > address space for PCI-E, and this in turn gives you more low mem > address space to use for DRAM. Hi Jason Gunthorpe, I implemented 'Single domain' with Exynos PCIe for last two months; however, it cannot work properly due to the hardware restriction. Each MEM region is hard-wired. Thus, I will send Exynos PCIe V3 patch as 'Separate domains'. Best regards, Jingoo Han > > > The following is right? > > > + pcie-controller { > > ..... > > + ranges = <0x82000000 0 0x40000000 0x40000000 0 0x00200000 /* port 0 registers */ > > + 0x82000000 0 0x60000000 0x60000000 0 0x00200000 /* port 1 registers */ > > + 0x81000000 0 0 0x40200000 0 0x00004000 /* port 0 downstream I/O */ > > + 0x81000000 0 0 0x60200000 0 0x00004000 /* port 1 downstream I/O */ > > + 0x82000000 0 0x40204000 0x40204000 0 0x10000000>; /* port 0 non-prefetchable > memory */ > > + 0x82000000 0 0x40204000 0x60204000 0 0x10000000>; /* port 1 non-prefetchable > memory */ > > > > + > > + pci@1,0 { > > + device_type = "pci"; > > + assigned-addresses = <0x82000800 0 0x40000000 0 0x00200000 > > + 0x81000800 0 0x40200000 0 0x00004000 > > + 0x81000800 0 0x40204000 0 0x10000000>; > > Would be: > > ranges = <0x81000800 0 0x40200000 0x81000800 0 0x40200000 0 0x00004000 > 0x81000800 0 0x40204000 0x81000800 0 0x40204000 0 0x10000000>; > assigned-addresses = <0x82000800 0 0x40000000 0 0x00200000>; > > Jason
On Friday 07 June 2013 18:19:40 Jingoo Han wrote: > Hi Jason Gunthorpe, > > I implemented 'Single domain' with Exynos PCIe for last two months; > however, it cannot work properly due to the hardware restriction. > Each MEM region is hard-wired. > > Thus, I will send Exynos PCIe V3 patch as 'Separate domains'. Yes, I think that is best, if the hardware is clearly designed as separate domains, this is what we should do by default in the driver. For the Marvell case with its 10 separate ports, much more address space would be wasted by having one domain per port and that hardware let us work around it by remapping the physical address space windows. For Exynos there is much less to lose and I too cannot see how it would be done in the first place. Arnd
On Fri, Jun 07, 2013 at 01:59:43PM +0200, Arnd Bergmann wrote: > On Friday 07 June 2013 18:19:40 Jingoo Han wrote: > > Hi Jason Gunthorpe, > > > > I implemented 'Single domain' with Exynos PCIe for last two months; > > however, it cannot work properly due to the hardware restriction. > > Each MEM region is hard-wired. > > > > Thus, I will send Exynos PCIe V3 patch as 'Separate domains'. > > Yes, I think that is best, if the hardware is clearly designed as > separate domains, this is what we should do by default in the > driver. For the Marvell case with its 10 separate ports, much > more address space would be wasted by having one domain per > port and that hardware let us work around it by remapping the > physical address space windows. For Exynos there is much less to > lose and I too cannot see how it would be done in the first > place. Sounds fair to me. But when we talk about multiple domains we don't mean a disjoint range bus bus numbers, as your other email shows: 00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) 10:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) We mean multiple domains, it should look like this: 0000:00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) 0001:00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) ie lspci -D. Each domain gets a unique bus number range, config space, io range, etc. This is much clearer to everyone than trying to pretend there is only one domain when the HW is actually multi-domain. Jason
On Friday 07 June 2013, Jason Gunthorpe wrote: > Sounds fair to me. > > But when we talk about multiple domains we don't mean a disjoint range > bus bus numbers, as your other email shows: > > 00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > 10:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > > We mean multiple domains, it should look like this: > > 0000:00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > 0001:00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > > ie lspci -D. > > Each domain gets a unique bus number range, config space, io range, > etc. This is much clearer to everyone than trying to pretend there is > only one domain when the HW is actually multi-domain. Yes, absolutely. This means we also don't need a bus-range property in DT, since each domain will allow all 255 buses. Arnd
On Saturday, June 08, 2013 2:43 AM, Arnd Bergmann wrote: > On Friday 07 June 2013, Jason Gunthorpe wrote: > > Sounds fair to me. > > > > But when we talk about multiple domains we don't mean a disjoint range > > bus bus numbers, as your other email shows: > > > > 00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > > 10:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > > > > We mean multiple domains, it should look like this: > > > > 0000:00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > > 0001:00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) > > > > ie lspci -D. > > > > Each domain gets a unique bus number range, config space, io range, > > etc. This is much clearer to everyone than trying to pretend there is > > only one domain when the HW is actually multi-domain. > > Yes, absolutely. This means we also don't need a bus-range property in DT, since each > domain will allow all 255 buses. After removing a bus-range property in DT, it looks like: 00:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) 02:00.0 PCI bridge: Samsung Electronics Co Ltd Device a549 (rev 01) (prog-if 00 [Normal decode]) For multiple domains, how can I fix the DT properties? Current DT properties are as below: + pcie0@40000000 { + compatible = "samsung,exynos5440-pcie"; + reg = <0x40000000 0x4000 + 0x290000 0x1000 + 0x270000 0x1000 + 0x271000 0x40>; + interrupts = <0 20 0>, <0 21 0>, <0 22 0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ + }; + + pcie1@60000000 { + compatible = "samsung,exynos5440-pcie"; + reg = <0x60000000 0x4000 + 0x2a0000 0x1000 + 0x272000 0x1000 + 0x271040 0x40>; + interrupts = <0 23 0>, <0 24 0>, <0 25 0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00200000 /* configuration space */ + 0x81000000 0 0 0x60200000 0 0x00004000 /* downstream I/O */ + 0x82000000 0 0 0x60204000 0 0x10000000>; /* non-prefetchable memory */ + }; Best regards, Jingoo Han > > Arnd > -- > 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
On Monday 10 June 2013, Jingoo Han wrote: > On Saturday, June 08, 2013 2:43 AM, Arnd Bergmann wrote: > For multiple domains, how can I fix the DT properties? Domains are a Linux concept, you have to pick a new domain number for each struct hw_pci you register. > Current DT properties are as below: > > + pcie0@40000000 { > + compatible = "samsung,exynos5440-pcie"; > + reg = <0x40000000 0x4000 > + 0x290000 0x1000 > + 0x270000 0x1000 > + 0x271000 0x40>; > + interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; > + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > + }; An unrelated comment: your first "reg" field seems to overlap with part of your configuration space. Is that intentional? Also, shouldn't your memory space end on a 256MB boundary, rather than extend up to 0x50203fff? Arnd
On Tuesday, June 11, 2013 12:22 AM, Arnd Bergmann wrote: > On Monday 10 June 2013, Jingoo Han wrote: > > On Saturday, June 08, 2013 2:43 AM, Arnd Bergmann wrote: > > For multiple domains, how can I fix the DT properties? > > Domains are a Linux concept, you have to pick a new domain number for each > struct hw_pci you register. Hi Arnd, Thank you for your reply. It is very helpful. :) I will set domain numbers for each struct hw_pci. > > > Current DT properties are as below: > > > > + pcie0@40000000 { > > + compatible = "samsung,exynos5440-pcie"; > > + reg = <0x40000000 0x4000 > > + 0x290000 0x1000 > > + 0x270000 0x1000 > > + 0x271000 0x40>; > > + interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + device_type = "pci"; > > + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > > + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > > + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > > + }; > > An unrelated comment: your first "reg" field seems to overlap with part > of your configuration space. Is that intentional? Yes, intentional. But, I will try to remove it. > > Also, shouldn't your memory space end on a 256MB boundary, rather than > extend up to 0x50203fff? According to the manual of Exynos PCIe, each memory space for Exynos PCIe can support 512MB, including I/O, CFG regions. Is there any problem when over 256MB boundary is used? Please let me know. :) Best regards, Jingoo Han > > Arnd
On Tuesday 11 June 2013, Jingoo Han wrote: > > > + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ > > > + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ > > > + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ > > > + }; > > > ... > > Also, shouldn't your memory space end on a 256MB boundary, rather than > > extend up to 0x50203fff? > > According to the manual of Exynos PCIe, each memory space for Exynos PCIe > can support 512MB, including I/O, CFG regions. > > Is there any problem when over 256MB boundary is used? > Please let me know. :) No, that's not a problem, but I think you should have the window span the entire space that is provided in hardware. If there are 512 MB total, why not use them? You could use ranges = <0x82000000 0 0 0x40204000 0 0x1fdfc000>; to pass a range for the memory space that extends all the way until 0x5fffffff. Arnd
diff --git a/arch/arm/boot/dts/exynos5440-ssdk5440.dts b/arch/arm/boot/dts/exynos5440-ssdk5440.dts index a21eb4c..746f9fc 100644 --- a/arch/arm/boot/dts/exynos5440-ssdk5440.dts +++ b/arch/arm/boot/dts/exynos5440-ssdk5440.dts @@ -34,4 +34,12 @@ clock-frequency = <50000000>; }; }; + + pcie0@40000000 { + reset-gpio = <5>; + }; + + pcie1@60000000 { + reset-gpio = <22>; + }; }; diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi index c374a31..41b2d2c 100644 --- a/arch/arm/boot/dts/exynos5440.dtsi +++ b/arch/arm/boot/dts/exynos5440.dtsi @@ -178,4 +178,36 @@ clocks = <&clock 21>; clock-names = "rtc"; }; + + pcie0@40000000 { + compatible = "samsung,exynos5440-pcie"; + reg = <0x40000000 0x4000 + 0x290000 0x1000 + 0x270000 0x1000 + 0x271000 0x40>; + interrupts = <0 20 0>, <0 21 0>, <0 22 0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + bus-range = <0x0 0xf>; + ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00200000 /* configuration space */ + 0x81000000 0 0 0x40200000 0 0x00004000 /* downstream I/O */ + 0x82000000 0 0 0x40204000 0 0x10000000>; /* non-prefetchable memory */ + }; + + pcie1@60000000 { + compatible = "samsung,exynos5440-pcie"; + reg = <0x60000000 0x4000 + 0x2a0000 0x1000 + 0x272000 0x1000 + 0x271040 0x40>; + interrupts = <0 23 0>, <0 24 0>, <0 25 0>; + #address-cells = <3>; + #size-cells = <2>; + device_type = "pci"; + bus-range = <0x0 0xf>; + ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00200000 /* configuration space */ + 0x81000000 0 0 0x60200000 0 0x00004000 /* downstream I/O */ + 0x82000000 0 0 0x60204000 0 0x10000000>; /* non-prefetchable memory */ + }; };
Exynos5440 has two PCIe controllers which can be used as root complex for PCIe interface. Signed-off-by: Jingoo Han <jg1.han@samsung.com> --- arch/arm/boot/dts/exynos5440-ssdk5440.dts | 8 +++++++ arch/arm/boot/dts/exynos5440.dtsi | 32 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 0 deletions(-)