| Message ID | 1385500248-6551-2-git-send-email-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Dear Marek Vasut, Please replace imx6 with designware in subject line of this patch. 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. > > 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? Otherwise I do not find any convincing reason that why this patch helps in resolving issues with PCI devices behind switches. 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; > -- > 1.8.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > > > 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; -- 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 Wednesday, November 27, 2013 5:44 PM, Marek Vasut wrote: > Wednesday, November 27, 2013 1:30 PM, Pratyush Anand wrote: > > 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 ? The numbers of inbound and outbound are hardware-configurable features. Thus, these numbers can be confirmed by Freescale hardware engineers. How about asking Freescale hardware engineers, if possible? Best regards, Jingoo Han > > [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. -- 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
Dear Jingoo Han, > On Wednesday, November 27, 2013 5:44 PM, Marek Vasut wrote: > > Wednesday, November 27, 2013 1:30 PM, Pratyush Anand wrote: > > > 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 ? > > The numbers of inbound and outbound are hardware-configurable features. > Thus, these numbers can be confirmed by Freescale hardware engineers. > How about asking Freescale hardware engineers, if possible? Richard is on the CC. Richard, can you please confirm those? Thanks! > Best regards, > Jingoo Han > > > [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. Best regards, Marek Vasut -- 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
Hi Marek: Best Regards Richard Zhu > -----Original Message----- > From: Marek Vasut [mailto:marex@denx.de] > Sent: Friday, November 29, 2013 1:56 AM > To: Jingoo Han > Cc: 'Tim Harvey'; 'Pratyush Anand'; linux-arm-kernel@lists.infradead.org; > linux-pci@vger.kernel.org; 'Bjorn Helgaas'; 'Frank Li'; 'Harro Haan'; 'Mohit > KUMAR DCG'; Zhu Richard-R65037; 'Sascha Hauer'; 'Sean Cross'; 'Shawn Guo'; > 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky'; 'Yinghai Lu' > Subject: Re: [PATCH 2/7] [RFC] PCI: imx6: remove outbound io/mem ATU region > mapping > > Dear Jingoo Han, > > > On Wednesday, November 27, 2013 5:44 PM, Marek Vasut wrote: > > > Wednesday, November 27, 2013 1:30 PM, Pratyush Anand wrote: > > > > 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 ? > > > > The numbers of inbound and outbound are hardware-configurable features. > > Thus, these numbers can be confirmed by Freescale hardware engineers. > > How about asking Freescale hardware engineers, if possible? > > Richard is on the CC. Richard, can you please confirm those? > [Richard] One Pericom PI7C9X2G303EL pcie switch, and two pcie ep deivces(one is intel e1000e nic, the other is one xhci device) are tested on imx6q sabresd board. Without removing outbound io/mem regions view map during the cfg0/1 read/write cycle, both of these devices can't work well at my side. Works well after remove them during the cfg0/1 read/write cycles. Same to Tim's situation about switch implementation. > Thanks! > > > Best regards, > > Jingoo Han > > > > > [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. > > Best regards, > Marek Vasut -- 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 Wednesday, December 04, 2013 at 06:49:55 AM, Richard Zhu wrote: > Hi Marek: > > Best Regards > Richard Zhu > > > -----Original Message----- > > From: Marek Vasut [mailto:marex@denx.de] > > Sent: Friday, November 29, 2013 1:56 AM > > To: Jingoo Han > > Cc: 'Tim Harvey'; 'Pratyush Anand'; linux-arm-kernel@lists.infradead.org; > > linux-pci@vger.kernel.org; 'Bjorn Helgaas'; 'Frank Li'; 'Harro Haan'; > > 'Mohit KUMAR DCG'; Zhu Richard-R65037; 'Sascha Hauer'; 'Sean Cross'; > > 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky'; > > 'Yinghai Lu' Subject: Re: [PATCH 2/7] [RFC] PCI: imx6: remove outbound > > io/mem ATU region mapping > > > > Dear Jingoo Han, > > > > > On Wednesday, November 27, 2013 5:44 PM, Marek Vasut wrote: > > > > Wednesday, November 27, 2013 1:30 PM, Pratyush Anand wrote: > > > > > 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 ? > > > > > > The numbers of inbound and outbound are hardware-configurable features. > > > Thus, these numbers can be confirmed by Freescale hardware engineers. > > > How about asking Freescale hardware engineers, if possible? > > > > Richard is on the CC. Richard, can you please confirm those? > > [Richard] One Pericom PI7C9X2G303EL pcie switch, and two pcie ep > deivces(one is intel e1000e nic, the other is one xhci device) are tested > on imx6q sabresd board. Without removing outbound io/mem regions view map > during the cfg0/1 read/write cycle, both of these devices can't work well > at my side. Works well after remove them during the cfg0/1 read/write > cycles. > Same to Tim's situation about switch implementation. Thanks Richard! I am just wondering what kind of conclusion can we draw from this. Let me reiterate what we have: - i.MX6 has 4 inbound and 4 outbound iATU regions, is that correct, Richard? - If we program the iATU regions, the i.MX6 PCIe doesn't work with switch. Is there some special sause to the iATU on MX6 maybe ? -- 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
Hi Marek: > -----Original Message----- > From: Marek Vasut [mailto:marex@denx.de] > Sent: Wednesday, December 04, 2013 3:52 PM > To: Zhu Richard-R65037 > Cc: Jingoo Han; 'Tim Harvey'; 'Pratyush Anand'; linux-arm- > kernel@lists.infradead.org; linux-pci@vger.kernel.org; 'Bjorn Helgaas'; 'Frank > Li'; 'Harro Haan'; 'Mohit KUMAR DCG'; 'Sascha Hauer'; 'Sean Cross'; 'Shawn > Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky'; 'Yinghai Lu' > Subject: Re: [PATCH 2/7] [RFC] PCI: imx6: remove outbound io/mem ATU region > mapping > > On Wednesday, December 04, 2013 at 06:49:55 AM, Richard Zhu wrote: > > Hi Marek: > > > > Best Regards > > Richard Zhu > > > > > -----Original Message----- > > > From: Marek Vasut [mailto:marex@denx.de] > > > Sent: Friday, November 29, 2013 1:56 AM > > > To: Jingoo Han > > > Cc: 'Tim Harvey'; 'Pratyush Anand'; > > > linux-arm-kernel@lists.infradead.org; > > > linux-pci@vger.kernel.org; 'Bjorn Helgaas'; 'Frank Li'; 'Harro > > > Haan'; 'Mohit KUMAR DCG'; Zhu Richard-R65037; 'Sascha Hauer'; 'Sean > > > Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; > > > 'Troy Kisky'; 'Yinghai Lu' Subject: Re: [PATCH 2/7] [RFC] PCI: imx6: > > > remove outbound io/mem ATU region mapping > > > > > > Dear Jingoo Han, > > > > > > > On Wednesday, November 27, 2013 5:44 PM, Marek Vasut wrote: > > > > > Wednesday, November 27, 2013 1:30 PM, Pratyush Anand wrote: > > > > > > 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 ? > > > > > > > > The numbers of inbound and outbound are hardware-configurable features. > > > > Thus, these numbers can be confirmed by Freescale hardware engineers. > > > > How about asking Freescale hardware engineers, if possible? > > > > > > Richard is on the CC. Richard, can you please confirm those? > > > > [Richard] One Pericom PI7C9X2G303EL pcie switch, and two pcie ep > > deivces(one is intel e1000e nic, the other is one xhci device) are > > tested on imx6q sabresd board. Without removing outbound io/mem > > regions view map during the cfg0/1 read/write cycle, both of these > > devices can't work well at my side. Works well after remove them > > during the cfg0/1 read/write cycles. > > Same to Tim's situation about switch implementation. > > Thanks Richard! I am just wondering what kind of conclusion can we draw from > this. Let me reiterate what we have: > > - i.MX6 has 4 inbound and 4 outbound iATU regions, is that correct, Richard? > - If we program the iATU regions, the i.MX6 PCIe doesn't work with switch. > > Is there some special sause to the iATU on MX6 maybe ? [Richard] welcome, it's correct, imx6 pcie has 4 inbount and 4 outbound iATU regions. Imx6 pcie doesn't work with switch if the following view switches are not removed in The cfg0/1 read/write cycles. @@ -516,9 +560,11 @@ 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; @@ -537,9 +583,11 @@ 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; Best Regards Richard Zhu -- 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
Hi Merak: > -----Original Message----- > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] > On Behalf Of Richard Zhu > Sent: Thursday, December 05, 2013 8:45 AM > To: Marek Vasut > Cc: Jingoo Han; 'Tim Harvey'; 'Pratyush Anand'; linux-arm- > kernel@lists.infradead.org; linux-pci@vger.kernel.org; 'Bjorn Helgaas'; 'Frank > Li'; 'Harro Haan'; 'Mohit KUMAR DCG'; 'Sascha Hauer'; 'Sean Cross'; 'Shawn > Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky'; 'Yinghai Lu' > Subject: RE: [PATCH 2/7] [RFC] PCI: imx6: remove outbound io/mem ATU region > mapping > > Hi Marek: > > > > > -----Original Message----- > > From: Marek Vasut [mailto:marex@denx.de] > > Sent: Wednesday, December 04, 2013 3:52 PM > > To: Zhu Richard-R65037 > > Cc: Jingoo Han; 'Tim Harvey'; 'Pratyush Anand'; linux-arm- > > kernel@lists.infradead.org; linux-pci@vger.kernel.org; 'Bjorn > > Helgaas'; 'Frank Li'; 'Harro Haan'; 'Mohit KUMAR DCG'; 'Sascha Hauer'; > > 'Sean Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; > 'Troy Kisky'; 'Yinghai Lu' > > Subject: Re: [PATCH 2/7] [RFC] PCI: imx6: remove outbound io/mem ATU > > region mapping > > > > On Wednesday, December 04, 2013 at 06:49:55 AM, Richard Zhu wrote: > > > Hi Marek: > > > > > > Best Regards > > > Richard Zhu > > > > > > > -----Original Message----- > > > > From: Marek Vasut [mailto:marex@denx.de] > > > > Sent: Friday, November 29, 2013 1:56 AM > > > > To: Jingoo Han > > > > Cc: 'Tim Harvey'; 'Pratyush Anand'; > > > > linux-arm-kernel@lists.infradead.org; > > > > linux-pci@vger.kernel.org; 'Bjorn Helgaas'; 'Frank Li'; 'Harro > > > > Haan'; 'Mohit KUMAR DCG'; Zhu Richard-R65037; 'Sascha Hauer'; > > > > 'Sean Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T > > > > Shivanand'; 'Troy Kisky'; 'Yinghai Lu' Subject: Re: [PATCH 2/7] [RFC] > PCI: imx6: > > > > remove outbound io/mem ATU region mapping > > > > > > > > Dear Jingoo Han, > > > > > > > > > On Wednesday, November 27, 2013 5:44 PM, Marek Vasut wrote: > > > > > > Wednesday, November 27, 2013 1:30 PM, Pratyush Anand wrote: > > > > > > > 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 ? > > > > > > > > > > The numbers of inbound and outbound are hardware-configurable features. > > > > > Thus, these numbers can be confirmed by Freescale hardware engineers. > > > > > How about asking Freescale hardware engineers, if possible? > > > > > > > > Richard is on the CC. Richard, can you please confirm those? > > > > > > [Richard] One Pericom PI7C9X2G303EL pcie switch, and two pcie ep > > > deivces(one is intel e1000e nic, the other is one xhci device) are > > > tested on imx6q sabresd board. Without removing outbound io/mem > > > regions view map during the cfg0/1 read/write cycle, both of these > > > devices can't work well at my side. Works well after remove them > > > during the cfg0/1 read/write cycles. > > > Same to Tim's situation about switch implementation. > > > > Thanks Richard! I am just wondering what kind of conclusion can we > > draw from this. Let me reiterate what we have: > > > > - i.MX6 has 4 inbound and 4 outbound iATU regions, is that correct, Richard? > > - If we program the iATU regions, the i.MX6 PCIe doesn't work with switch. > > > > Is there some special sause to the iATU on MX6 maybe ? > [Richard] welcome, it's correct, imx6 pcie has 4 inbount and 4 outbound iATU > regions. > Imx6 pcie doesn't work with switch if the following view switches are not > removed in The cfg0/1 read/write cycles. > > @@ -516,9 +560,11 @@ 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; > @@ -537,9 +583,11 @@ 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; > > Best Regards > Richard Zhu > [Richard] Bisect the code-modifications, only the dw_pcie_prog_viewport_io_outbound() should be masked if I want the switch works in my imx6 pcie switch support tests. Suspect that the mem emulated io space introduce this problem, debug is on-going. > Best Regards Richard Zhu > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in the > body of a message to majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, December 05, 2013 at 02:05:40 AM, Richard Zhu wrote: > Hi Merak: > > -----Original Message----- > > From: linux-pci-owner@vger.kernel.org > > [mailto:linux-pci-owner@vger.kernel.org] On Behalf Of Richard Zhu > > Sent: Thursday, December 05, 2013 8:45 AM > > To: Marek Vasut > > Cc: Jingoo Han; 'Tim Harvey'; 'Pratyush Anand'; linux-arm- > > kernel@lists.infradead.org; linux-pci@vger.kernel.org; 'Bjorn Helgaas'; > > 'Frank Li'; 'Harro Haan'; 'Mohit KUMAR DCG'; 'Sascha Hauer'; 'Sean > > Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy > > Kisky'; 'Yinghai Lu' Subject: RE: [PATCH 2/7] [RFC] PCI: imx6: remove > > outbound io/mem ATU region mapping > > > > Hi Marek: > > > -----Original Message----- > > > From: Marek Vasut [mailto:marex@denx.de] > > > Sent: Wednesday, December 04, 2013 3:52 PM > > > To: Zhu Richard-R65037 > > > Cc: Jingoo Han; 'Tim Harvey'; 'Pratyush Anand'; linux-arm- > > > kernel@lists.infradead.org; linux-pci@vger.kernel.org; 'Bjorn > > > Helgaas'; 'Frank Li'; 'Harro Haan'; 'Mohit KUMAR DCG'; 'Sascha Hauer'; > > > 'Sean Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; > > > > 'Troy Kisky'; 'Yinghai Lu' > > > > > Subject: Re: [PATCH 2/7] [RFC] PCI: imx6: remove outbound io/mem ATU > > > region mapping > > > > > > On Wednesday, December 04, 2013 at 06:49:55 AM, Richard Zhu wrote: > > > > Hi Marek: > > > > > > > > Best Regards > > > > Richard Zhu > > > > > > > > > -----Original Message----- > > > > > From: Marek Vasut [mailto:marex@denx.de] > > > > > Sent: Friday, November 29, 2013 1:56 AM > > > > > To: Jingoo Han > > > > > Cc: 'Tim Harvey'; 'Pratyush Anand'; > > > > > linux-arm-kernel@lists.infradead.org; > > > > > linux-pci@vger.kernel.org; 'Bjorn Helgaas'; 'Frank Li'; 'Harro > > > > > Haan'; 'Mohit KUMAR DCG'; Zhu Richard-R65037; 'Sascha Hauer'; > > > > > 'Sean Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T > > > > > Shivanand'; 'Troy Kisky'; 'Yinghai Lu' Subject: Re: [PATCH 2/7] > > > > > [RFC] > > > > PCI: imx6: > > > > > remove outbound io/mem ATU region mapping > > > > > > > > > > Dear Jingoo Han, > > > > > > > > > > > On Wednesday, November 27, 2013 5:44 PM, Marek Vasut wrote: > > > > > > > Wednesday, November 27, 2013 1:30 PM, Pratyush Anand wrote: > > > > > > > > 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 ? > > > > > > > > > > > > The numbers of inbound and outbound are hardware-configurable > > > > > > features. Thus, these numbers can be confirmed by Freescale > > > > > > hardware engineers. How about asking Freescale hardware > > > > > > engineers, if possible? > > > > > > > > > > Richard is on the CC. Richard, can you please confirm those? > > > > > > > > [Richard] One Pericom PI7C9X2G303EL pcie switch, and two pcie ep > > > > deivces(one is intel e1000e nic, the other is one xhci device) are > > > > tested on imx6q sabresd board. Without removing outbound io/mem > > > > regions view map during the cfg0/1 read/write cycle, both of these > > > > devices can't work well at my side. Works well after remove them > > > > during the cfg0/1 read/write cycles. > > > > Same to Tim's situation about switch implementation. > > > > > > Thanks Richard! I am just wondering what kind of conclusion can we > > > draw from this. Let me reiterate what we have: > > > > > > - i.MX6 has 4 inbound and 4 outbound iATU regions, is that correct, > > > Richard? - If we program the iATU regions, the i.MX6 PCIe doesn't work > > > with switch. > > > > > > Is there some special sause to the iATU on MX6 maybe ? > > > > [Richard] welcome, it's correct, imx6 pcie has 4 inbount and 4 outbound > > iATU regions. > > Imx6 pcie doesn't work with switch if the following view switches are not > > removed in The cfg0/1 read/write cycles. > > > > @@ -516,9 +560,11 @@ 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; > > > > @@ -537,9 +583,11 @@ 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; > > > > Best Regards > > Richard Zhu > > [Richard] Bisect the code-modifications, only the > dw_pcie_prog_viewport_io_outbound() should be masked if I want the switch > works in my imx6 pcie switch support tests. Suspect that the mem emulated > io space introduce this problem, debug is on-going. Freescale is now looking into this issue ? Cool, thanks! :) -- 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
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;