| Message ID | CAJ+vNU050mtdiyXBXue_JGFJhFLiKCvoyf031Uda2e5U+vA12g@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tim, On Wed, Dec 04, 2013 at 04:14:47AM +0800, Tim Harvey wrote: > On Wed, Nov 27, 2013 at 12:44 AM, Marek Vasut <marex@denx.de> wrote: > > Dear Pratyush Anand, > > > >> Dear Marek Vasut, > >> > >> Please replace imx6 with designware in subject line of this patch. > > > > OK, this one has to be fixed anyway. We need to figure out why do we even need > > this one. > > > >> On Wed, Nov 27, 2013 at 05:10:43AM +0800, Marek Vasut wrote: > >> > From: Tim Harvey <tharvey@gateworks.com> > >> > > >> > The IMX6 iATU is used for address translation between the AXI bus > >> > address space and PCI address space. This is used for type0 and type1 > >> > config cycles but is not necessary for outbound io/mem regions. > >> > > >> > This patch removes the calls that inappropriately re-configures the ATU > >> > viewport for outbound memory and IO after config cycles and removes them > >> > altogether as they are not necessary. > >> > >> Yes, they are not necessary for all the cases where translation is one > >> to one. So so for sure all the platform till now introduced should > >> work. > >> But, what about a platform where memory translation is not one to one? > >> > >> Existing code should work with all sort of memory translation on a > >> platform having atleast two viewports capable of programming any type > >> of outbound transaction. > > > > Full ACK, that's why it's called RFC. > > > >> > This resolves issues with PCI devices behind switches and has been tested > >> > with a Gige device behind a PLX PEX860x switch. More testing is needed > >> > for other configurations. > >> > >> I do not understand if MX6 has 4 Outbound Viewport then how this patch > >> helps? > >> -- PCI devices behind switches would not have been working because > >> CFG1 transaction would not have been correct. > >> -- It works with this patch. This patch changes viewport for CFG1 from > >> 1 to 0. > >> -- Can it be possible that MX6 has some restriction on viewport > >> programming capability. I mean,like only viewport0 can be programmed > >> for CFG0/1? > > > > Tim ? > > > > Here is the MX6 datasheet [1], the section 48.3.9.1.1 and 48.3.9.1.2 describe > > the iATU configuration on MX6. My understanding from this description is that > > the MX6 has 4 inbound and 4 outbound iATU regions. Am I wrong ? > > > > [1] http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf > > > >> Otherwise I do not find any convincing reason that why this patch > >> helps in resolving issues with PCI devices behind switches. > > In my configuration I have a PLX switch off the IMX root complex with > several devices behind it. I find that the devices enumerate > correctly but as soon as data is transferred to or from (not sure > which) the device the system hangs (INFO: rcu_sched detected stalls on > CPUs/tasks: { 0} (detected by 3, t=2135 jiffies, g=20, c=19, q=66)). > My current test case is a GigE ethernet device and when I connect a > network to the device I hange when a packet is responded to. > > I can't claim to fully understand PCI resource mappings or the iATU > and I don't understand why dw_pcie_rd_other_conf/dw_pcie_wr_other_conf > need to change the viewport then change it back instead of using > multiple viewports (perhaps because some hardware may not have more > than the 2 viewports currently being used?). The current driver uses Yes, driver has been written keeping in mind that there exist minimum of two programmable outbound viewport. > viewport0 for cfg0 and mem and viewport1 for cfg1 and io. If I remove > the call to dw_pcie_prog_viewport_io_outbound to reconfigure viewport1 > for io after its altered for type1 cfg cycles, devices behind the > bridge work for me: This is strange !!! When you say it does not work, what exactly happens? What does kernel crash log says? Which register access forces CPU to hang? Is it possible to capture traffic with PCIe analyzer? > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designwa > index e33b68b..a064bc5 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -556,7 +556,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struc > } else { > dw_pcie_prog_viewport_cfg1(pp, busdev); > ret = cfg_read(pp->va_cfg1_base + address, where, size, val); > - dw_pcie_prog_viewport_io_outbound(pp); > +// dw_pcie_prog_viewport_io_outbound(pp); > } > > return ret; > @@ -579,7 +579,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struc > } else { > dw_pcie_prog_viewport_cfg1(pp, busdev); > ret = cfg_write(pp->va_cfg1_base + address, where, size, val); > - dw_pcie_prog_viewport_io_outbound(pp); > +// dw_pcie_prog_viewport_io_outbound(pp); > } > > Can anyone explain whats going on here? I would think this would > perhaps break devices that use IO resources? Yes, without this devices with IO resource will not work. However, I see some issues with IO translation implementation, and as per my understanding IO transfer should not work with current implementation. Anyway, that is unrelated to the issue you are facing..so better I will discuss that in another thread. Regards Pratyush > > Thanks, > > Tim > > >> > >> > >> Regards > >> Pratyush > >> > >> > From: Tim Harvey <tharvey@gateworks.com> > >> > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > >> > Cc: Bjorn Helgaas <bhelgaas@google.com> > >> > Cc: Frank Li <lznuaa@gmail.com> > >> > Cc: Harro Haan <hrhaan@gmail.com> > >> > Cc: Jingoo Han <jg1.han@samsung.com> > >> > Cc: Mohit KUMAR <Mohit.KUMAR@st.com> > >> > Cc: Pratyush Anand <pratyush.anand@st.com> > >> > Cc: Richard Zhu <r65037@freescale.com> > >> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > >> > Cc: Sean Cross <xobs@kosagi.com> > >> > Cc: Shawn Guo <shawn.guo@linaro.org> > >> > Cc: Siva Reddy Kallam <siva.kallam@samsung.com> > >> > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com> > >> > Cc: Tim Harvey <tharvey@gateworks.com> > >> > Cc: Troy Kisky <troy.kisky@boundarydevices.com> > >> > Cc: Yinghai Lu <yinghai@kernel.org> > >> > --- > >> > > >> > drivers/pci/host/pcie-designware.c | 41 > >> > +++----------------------------------- 1 file changed, 3 insertions(+), > >> > 38 deletions(-) > >> > > >> > diff --git a/drivers/pci/host/pcie-designware.c > >> > b/drivers/pci/host/pcie-designware.c index ed9b11b..01d76ad 100644 > >> > --- a/drivers/pci/host/pcie-designware.c > >> > +++ b/drivers/pci/host/pcie-designware.c > >> > @@ -46,7 +46,6 @@ > >> > > >> > #define PCIE_ATU_VIEWPORT 0x900 > >> > #define PCIE_ATU_REGION_INBOUND (0x1 << 31) > >> > #define PCIE_ATU_REGION_OUTBOUND (0x0 << 31) > >> > > >> > -#define PCIE_ATU_REGION_INDEX1 (0x1 << 0) > >> > > >> > #define PCIE_ATU_REGION_INDEX0 (0x0 << 0) > >> > #define PCIE_ATU_CR1 0x904 > >> > #define PCIE_ATU_TYPE_MEM (0x0 << 0) > >> > > >> > @@ -494,8 +493,8 @@ static void dw_pcie_prog_viewport_cfg0(struct > >> > pcie_port *pp, u32 busdev) > >> > > >> > static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev) > >> > { > >> > > >> > - /* Program viewport 1 : OUTBOUND : CFG1 */ > >> > - dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | > >> > PCIE_ATU_REGION_INDEX1, + /* Program viewport 0 : OUTBOUND : CFG1 */ > >> > + dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | > >> > PCIE_ATU_REGION_INDEX0, > >> > > >> > PCIE_ATU_VIEWPORT); > >> > > >> > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1); > >> > dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2); > >> > > >> > @@ -505,38 +504,8 @@ static void dw_pcie_prog_viewport_cfg1(struct > >> > pcie_port *pp, u32 busdev) > >> > > >> > PCIE_ATU_LIMIT); > >> > > >> > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET); > >> > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); > >> > > >> > -} > >> > - > >> > -static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp) > >> > -{ > >> > - /* Program viewport 0 : OUTBOUND : MEM */ > >> > - dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | > >> > PCIE_ATU_REGION_INDEX0, - PCIE_ATU_VIEWPORT); > >> > - dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1); > >> > - dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2); > >> > - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE); > >> > - dw_pcie_writel_rc(pp, (pp->mem_base >> 32), PCIE_ATU_UPPER_BASE); > >> > - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1, > >> > - PCIE_ATU_LIMIT); > >> > - dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, PCIE_ATU_LOWER_TARGET); > >> > - dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr), > >> > - PCIE_ATU_UPPER_TARGET); > >> > -} > >> > - > >> > -static void dw_pcie_prog_viewport_io_outbound(struct pcie_port *pp) > >> > -{ > >> > - /* Program viewport 1 : OUTBOUND : IO */ > >> > - dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | > >> > PCIE_ATU_REGION_INDEX1, - PCIE_ATU_VIEWPORT); > >> > - dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_IO, PCIE_ATU_CR1); > >> > + dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1); > >> > > >> > dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2); > >> > > >> > - dw_pcie_writel_rc(pp, pp->io_base, PCIE_ATU_LOWER_BASE); > >> > - dw_pcie_writel_rc(pp, (pp->io_base >> 32), PCIE_ATU_UPPER_BASE); > >> > - dw_pcie_writel_rc(pp, pp->io_base + pp->config.io_size - 1, > >> > - PCIE_ATU_LIMIT); > >> > - dw_pcie_writel_rc(pp, pp->config.io_bus_addr, PCIE_ATU_LOWER_TARGET); > >> > - dw_pcie_writel_rc(pp, upper_32_bits(pp->config.io_bus_addr), > >> > - PCIE_ATU_UPPER_TARGET); > >> > > >> > } > >> > > >> > static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus > >> > *bus, > >> > > >> > @@ -552,11 +521,9 @@ static int dw_pcie_rd_other_conf(struct pcie_port > >> > *pp, struct pci_bus *bus, > >> > > >> > if (bus->parent->number == pp->root_bus_nr) { > >> > > >> > dw_pcie_prog_viewport_cfg0(pp, busdev); > >> > ret = cfg_read(pp->va_cfg0_base + address, where, size, val); > >> > > >> > - dw_pcie_prog_viewport_mem_outbound(pp); > >> > > >> > } else { > >> > > >> > dw_pcie_prog_viewport_cfg1(pp, busdev); > >> > ret = cfg_read(pp->va_cfg1_base + address, where, size, val); > >> > > >> > - dw_pcie_prog_viewport_io_outbound(pp); > >> > > >> > } > >> > > >> > return ret; > >> > > >> > @@ -575,11 +542,9 @@ static int dw_pcie_wr_other_conf(struct pcie_port > >> > *pp, struct pci_bus *bus, > >> > > >> > if (bus->parent->number == pp->root_bus_nr) { > >> > > >> > dw_pcie_prog_viewport_cfg0(pp, busdev); > >> > ret = cfg_write(pp->va_cfg0_base + address, where, size, val); > >> > > >> > - dw_pcie_prog_viewport_mem_outbound(pp); > >> > > >> > } else { > >> > > >> > dw_pcie_prog_viewport_cfg1(pp, busdev); > >> > ret = cfg_write(pp->va_cfg1_base + address, where, size, val); > >> > > >> > - dw_pcie_prog_viewport_io_outbound(pp); > >> > > >> > } > >> > > >> > return ret;
On Wed, Dec 4, 2013 at 2:33 AM, Pratyush Anand <pratyush.anand@st.com> wrote: > Hi Tim, > >> >> In my configuration I have a PLX switch off the IMX root complex with >> several devices behind it. I find that the devices enumerate >> correctly but as soon as data is transferred to or from (not sure >> which) the device the system hangs (INFO: rcu_sched detected stalls on >> CPUs/tasks: { 0} (detected by 3, t=2135 jiffies, g=20, c=19, q=66)). >> My current test case is a GigE ethernet device and when I connect a >> network to the device I hange when a packet is responded to. >> >> I can't claim to fully understand PCI resource mappings or the iATU >> and I don't understand why dw_pcie_rd_other_conf/dw_pcie_wr_other_conf >> need to change the viewport then change it back instead of using >> multiple viewports (perhaps because some hardware may not have more >> than the 2 viewports currently being used?). The current driver uses > > Yes, driver has been written keeping in mind that there exist minimum > of two programmable outbound viewport. > >> viewport0 for cfg0 and mem and viewport1 for cfg1 and io. If I remove >> the call to dw_pcie_prog_viewport_io_outbound to reconfigure viewport1 >> for io after its altered for type1 cfg cycles, devices behind the >> bridge work for me: > > This is strange !!! > When you say it does not work, what exactly happens? What does kernel > crash log says? Which register access forces CPU to hang? Is it > possible to capture traffic with PCIe analyzer? > Pratyush, The device behind the switch is a Marvell Yukon 2 GigE 88E8057: $ lspci -n 00:00.0 0604: 16c3:abcd (rev 01) 01:00.0 0604: 10b5:8609 (rev ba) 01:00.1 0880: 10b5:8609 (rev ba) 02:01.0 0604: 10b5:8609 (rev ba) 02:04.0 0604: 10b5:8609 (rev ba) 02:05.0 0604: 10b5:8609 (rev ba) 02:06.0 0604: 10b5:8609 (rev ba) 02:07.0 0604: 10b5:8609 (rev ba) 02:08.0 0604: 10b5:8609 (rev ba) 02:09.0 0604: 10b5:8609 (rev ba) 07:00.0 0280: 168c:002b (rev 01) 08:00.0 0200: 11ab:4380 $ lspci -s 08:00.0 -v 08:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8057 PCI-E Gigabit Ethernet Controller Subsystem: Marvell Technology Group Ltd. 88E8057 PCI-E Gigabit Ethernet Controller Flags: bus master, fast devsel, latency 0, IRQ 155 Memory at 01200000 (64-bit, non-prefetchable) [size=16K] I/O ports at 1000 [size=256] Capabilities: [48] Power Management version 3 Capabilities: [5c] MSI: Enable- Count=1/1 Maskable- 64bit+ Capabilities: [c0] Express Legacy Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [130] Device Serial Number 00-00-00-00-00-00-00-00 Kernel driver in use: sky2 $ ifconfig eth1 eth1 Link encap:Ethernet HWaddr 00:D0:12:9D:EF:E7 BROADCAST MULTICAST MTU:1500 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:0 (0.0 B) TX bytes:0 (0.0 B) Interrupt:155 $ ifconfig eth1 up [ 70.207493] sky2 0000:08:00.0 eth1: enabling interface [ 70.212855] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready [ 72.703741] sky2 0000:08:00.0 eth1: Link is up at 1000 Mbps, full duplex, flow control both [ 72.712392] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready board hangs here for several minutes then an rcu_sched stall is detected minutes later (I can't explain why the printk time below is wrong... it seems to within several minutes after the hang) [ 63.559915] INFO: rcu_sched detected stalls on CPUs/tasks: { 0} (detected by 1, t=2102 jiffies, g=4294967231, c=4294967230, q=10) I do not have an anaylzer that can get on the PCIe bus. Tim
On Wednesday, December 04, 2013 at 07:51:22 PM, Tim Harvey wrote: > On Wed, Dec 4, 2013 at 2:33 AM, Pratyush Anand <pratyush.anand@st.com> wrote: > > Hi Tim, > > > >> In my configuration I have a PLX switch off the IMX root complex with > >> several devices behind it. I find that the devices enumerate > >> correctly but as soon as data is transferred to or from (not sure > >> which) the device the system hangs (INFO: rcu_sched detected stalls on > >> CPUs/tasks: { 0} (detected by 3, t=2135 jiffies, g=20, c=19, q=66)). > >> My current test case is a GigE ethernet device and when I connect a > >> network to the device I hange when a packet is responded to. > >> > >> I can't claim to fully understand PCI resource mappings or the iATU > >> and I don't understand why dw_pcie_rd_other_conf/dw_pcie_wr_other_conf > >> need to change the viewport then change it back instead of using > >> multiple viewports (perhaps because some hardware may not have more > >> than the 2 viewports currently being used?). The current driver uses > > > > Yes, driver has been written keeping in mind that there exist minimum > > of two programmable outbound viewport. > > > >> viewport0 for cfg0 and mem and viewport1 for cfg1 and io. If I remove > >> the call to dw_pcie_prog_viewport_io_outbound to reconfigure viewport1 > >> for io after its altered for type1 cfg cycles, devices behind the > > > >> bridge work for me: > > This is strange !!! > > When you say it does not work, what exactly happens? What does kernel > > crash log says? Which register access forces CPU to hang? Is it > > possible to capture traffic with PCIe analyzer? > > Pratyush, > > The device behind the switch is a Marvell Yukon 2 GigE 88E8057: > > $ lspci -n > 00:00.0 0604: 16c3:abcd (rev 01) > 01:00.0 0604: 10b5:8609 (rev ba) > 01:00.1 0880: 10b5:8609 (rev ba) > 02:01.0 0604: 10b5:8609 (rev ba) > 02:04.0 0604: 10b5:8609 (rev ba) > 02:05.0 0604: 10b5:8609 (rev ba) > 02:06.0 0604: 10b5:8609 (rev ba) > 02:07.0 0604: 10b5:8609 (rev ba) > 02:08.0 0604: 10b5:8609 (rev ba) > 02:09.0 0604: 10b5:8609 (rev ba) > 07:00.0 0280: 168c:002b (rev 01) > 08:00.0 0200: 11ab:4380 > $ lspci -s 08:00.0 -v > 08:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8057 > PCI-E Gigabit Ethernet Controller > Subsystem: Marvell Technology Group Ltd. 88E8057 PCI-E Gigabit > Ethernet Controller > Flags: bus master, fast devsel, latency 0, IRQ 155 > Memory at 01200000 (64-bit, non-prefetchable) [size=16K] > I/O ports at 1000 [size=256] > Capabilities: [48] Power Management version 3 > Capabilities: [5c] MSI: Enable- Count=1/1 Maskable- 64bit+ > Capabilities: [c0] Express Legacy Endpoint, MSI 00 > Capabilities: [100] Advanced Error Reporting > Capabilities: [130] Device Serial Number 00-00-00-00-00-00-00-00 > Kernel driver in use: sky2 > > $ ifconfig eth1 > eth1 Link encap:Ethernet HWaddr 00:D0:12:9D:EF:E7 > BROADCAST MULTICAST MTU:1500 Metric:1 > RX packets:0 errors:0 dropped:0 overruns:0 frame:0 > TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:0 (0.0 B) TX bytes:0 (0.0 B) > Interrupt:155 > > $ ifconfig eth1 up > [ 70.207493] sky2 0000:08:00.0 eth1: enabling interface > [ 70.212855] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready > [ 72.703741] sky2 0000:08:00.0 eth1: Link is up at 1000 Mbps, full > duplex, flow control both > [ 72.712392] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready > > board hangs here for several minutes then an rcu_sched stall is > detected minutes later (I can't explain why the printk time below is > wrong... it seems to within several minutes after the hang) > > [ 63.559915] INFO: rcu_sched detected stalls on CPUs/tasks: { 0} > (detected by 1, t=2102 jiffies, g=4294967231, c=4294967230, q=10) > > I do not have an anaylzer that can get on the PCIe bus. I wonder, if you remove all IO space accesses from your Yukon driver, will the system hang as well? That way, we can be sure that the IO accesses are what kills the system.
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designwa index e33b68b..a064bc5 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -556,7 +556,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struc } else { dw_pcie_prog_viewport_cfg1(pp, busdev); ret = cfg_read(pp->va_cfg1_base + address, where, size, val); - dw_pcie_prog_viewport_io_outbound(pp); +// dw_pcie_prog_viewport_io_outbound(pp); } return ret; @@ -579,7 +579,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struc } else { dw_pcie_prog_viewport_cfg1(pp, busdev); ret = cfg_write(pp->va_cfg1_base + address, where, size, val); - dw_pcie_prog_viewport_io_outbound(pp); +// dw_pcie_prog_viewport_io_outbound(pp); } Can anyone explain whats going on here? I would think this would