diff mbox

[RFC] PCI: imx6: remove outbound io/mem ATU region mapping

Message ID 1382504143-27915-1-git-send-email-tharvey@gateworks.com
State Rejected
Headers show

Commit Message

Tim Harvey Oct. 23, 2013, 4:55 a.m. UTC
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.

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.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 drivers/pci/host/pcie-designware.c |   41 +++---------------------------------
 1 file changed, 3 insertions(+), 38 deletions(-)

Comments

Marek Vasut Oct. 23, 2013, 5:04 a.m. UTC | #1
Dear Tim Harvey,

> 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.
> 
> 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.
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>

This probably needs further fixes to the DT on exynos and omap. And it also 
needs testing on those platforms as this was developed on freescale mx6.

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
Pratyush ANAND Oct. 23, 2013, 6:40 a.m. UTC | #2
Hi,

On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> 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.
> 
> 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.

It seems to me that in your controller you have only one view port.
Also mem_base and mem_bus_addr is same. Thats why it works in your
case. 

I would suggest to wait for Kishon's work who would be trying to optimize
viewport allocations.

Regards
Pratyush

> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  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 5d1f268..6cce779 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)
> @@ -502,8 +501,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);
> @@ -513,38 +512,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,
> @@ -560,11 +529,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;
> @@ -583,11 +550,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.7.9.5
--
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
Bjorn Helgaas Nov. 25, 2013, 11:09 p.m. UTC | #3
On Tue, Oct 22, 2013 at 09:55:43PM -0700, Tim Harvey wrote:
> 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.
> 
> 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.
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>

Dropping for now, per Marek and Pratyush's concerns.  Poke me if/when those
are resolved.

> ---
>  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 5d1f268..6cce779 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)
> @@ -502,8 +501,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);
> @@ -513,38 +512,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,
> @@ -560,11 +529,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;
> @@ -583,11 +550,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.7.9.5
> 
--
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
Marek Vasut Nov. 26, 2013, 8:46 p.m. UTC | #4
Dear Pratyush Anand,

> Hi,
> 
> On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > 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.
> > 
> > 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.
> 
> It seems to me that in your controller you have only one view port.
> Also mem_base and mem_bus_addr is same. Thats why it works in your
> case.

MX6 has 4 In/4 Out viewports AFAICT.

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
Marek Vasut Nov. 26, 2013, 8:47 p.m. UTC | #5
Dear Bjorn Helgaas,

> On Tue, Oct 22, 2013 at 09:55:43PM -0700, Tim Harvey wrote:
> > 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.
> > 
> > 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.
> > 
> > Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> 
> Dropping for now, per Marek and Pratyush's concerns.  Poke me if/when those
> are resolved.

I hope (unless of course I again fail to under load of other stuff to do) to 
look at the PCIe stuff again next week once I'm in the office. In the meantime, 
I'd love to submit new set of patches.

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
Pratyush ANAND Nov. 27, 2013, 3:50 a.m. UTC | #6
On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> Dear Pratyush Anand,
> 
> > Hi,
> > 
> > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > 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.
> > > 
> > > 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.
> > 
> > It seems to me that in your controller you have only one view port.
> > Also mem_base and mem_bus_addr is same. Thats why it works in your
> > case.
> 
> MX6 has 4 In/4 Out viewports AFAICT.

Then if I do not miss anything, MX6 should work even without this patch.

Regards
Pratyush
> 
> 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
Richard Zhu Nov. 27, 2013, 7:42 a.m. UTC | #7
Hi Pratyush:

> -----Original Message-----
> From: Pratyush Anand [mailto:pratyush.anand@st.com]
> Sent: Wednesday, November 27, 2013 11:51 AM
> To: Marek Vasut
> Cc: Tim Harvey; Kishon Vijay Abraham I; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Bjorn Helgaas; Frank Li; Jingoo Han; 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 RFC] PCI: imx6: remove outbound io/mem ATU region mapping
> 
> On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > Dear Pratyush Anand,
> >
> > > Hi,
> > >
> > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > 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.
> > > >
> > > > 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.
> > >
> > > It seems to me that in your controller you have only one view port.
> > > Also mem_base and mem_bus_addr is same. Thats why it works in your
> > > case.
> >
> > MX6 has 4 In/4 Out viewports AFAICT.
> 
> Then if I do not miss anything, MX6 should work even without this patch.

[Richard] Regarding to the commit message, and tests at my side, 
the switch wouldn't work well on imx6 platforms, if "the calls that inappropriately re-configures
 the ATU viewport for outbound memory and IO after config cycles " are not removed.

> 
> Regards
> Pratyush
> >
> > Best regards,
> > Marek Vasut

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
Marek Vasut Nov. 27, 2013, 8:24 a.m. UTC | #8
Dear Pratyush Anand,

> On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > Dear Pratyush Anand,
> > 
> > > Hi,
> > > 
> > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > 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.
> > > > 
> > > > 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.
> > > 
> > > It seems to me that in your controller you have only one view port.
> > > Also mem_base and mem_bus_addr is same. Thats why it works in your
> > > case.
> > 
> > MX6 has 4 In/4 Out viewports AFAICT.
> 
> Then if I do not miss anything, MX6 should work even without this patch.

Yes, yet, it does not. On the other hand, this defect is only problematic if you 
use the devices behind a switch. Do you use a PCIe switch on your platform 
please?

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
Pratyush ANAND Nov. 27, 2013, 8:36 a.m. UTC | #9
On Wed, Nov 27, 2013 at 04:24:47PM +0800, Marek Vasut wrote:
> Dear Pratyush Anand,
> 
> > On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > > Dear Pratyush Anand,
> > > 
> > > > Hi,
> > > > 
> > > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > > 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.
> > > > > 
> > > > > 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.
> > > > 
> > > > It seems to me that in your controller you have only one view port.
> > > > Also mem_base and mem_bus_addr is same. Thats why it works in your
> > > > case.
> > > 
> > > MX6 has 4 In/4 Out viewports AFAICT.
> > 
> > Then if I do not miss anything, MX6 should work even without this patch.
> 
> Yes, yet, it does not. On the other hand, this defect is only problematic if you 
> use the devices behind a switch. Do you use a PCIe switch on your platform 
> please?

I had tested with device under bridge.
Mohit is in process of validating SPEAr patches with latest kernel.
Mohit, see if we can arrange a switch and test, once SPEAr platform is
ready with latest kernel.

Regards
Pratyush
> 
> 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
Marek Vasut Nov. 27, 2013, 8:45 a.m. UTC | #10
Hi Pratyush,

> On Wed, Nov 27, 2013 at 04:24:47PM +0800, Marek Vasut wrote:
> > Dear Pratyush Anand,
> > 
> > > On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > > > Dear Pratyush Anand,
> > > > 
> > > > > Hi,
> > > > > 
> > > > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > > > 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.
> > > > > > 
> > > > > > 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.
> > > > > 
> > > > > It seems to me that in your controller you have only one view port.
> > > > > Also mem_base and mem_bus_addr is same. Thats why it works in your
> > > > > case.
> > > > 
> > > > MX6 has 4 In/4 Out viewports AFAICT.
> > > 
> > > Then if I do not miss anything, MX6 should work even without this
> > > patch.
> > 
> > Yes, yet, it does not. On the other hand, this defect is only problematic
> > if you use the devices behind a switch. Do you use a PCIe switch on your
> > platform please?
> 
> I had tested with device under bridge.
> Mohit is in process of validating SPEAr patches with latest kernel.
> Mohit, see if we can arrange a switch and test, once SPEAr platform is
> ready with latest kernel.

Thanks!

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
Jingoo Han Nov. 28, 2013, 5:54 a.m. UTC | #11
On Wednesday, November 27, 2013 5:37 PM, Pratyush Anand wrote:
> On Wed, Nov 27, 2013 at 04:24:47PM +0800, Marek Vasut wrote:
> > > On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > > > Dear Pratyush Anand,
> > > >
> > > > > Hi,
> > > > >
> > > > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > > > 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.
> > > > > >
> > > > > > 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.
> > > > >
> > > > > It seems to me that in your controller you have only one view port.
> > > > > Also mem_base and mem_bus_addr is same. Thats why it works in your
> > > > > case.
> > > >
> > > > MX6 has 4 In/4 Out viewports AFAICT.
> > >
> > > Then if I do not miss anything, MX6 should work even without this patch.
> >
> > Yes, yet, it does not. On the other hand, this defect is only problematic if you
> > use the devices behind a switch. Do you use a PCIe switch on your platform
> > please?
> 
> I had tested with device under bridge.
> Mohit is in process of validating SPEAr patches with latest kernel.
> Mohit, see if we can arrange a switch and test, once SPEAr platform is
> ready with latest kernel.

I agree with Pratyush Anand's opinion.
One of our engineers had tested PCI cards under switch.
As far as I know, there was no issue about this.
However, currently, I don't have any switches. So, I
Cannot test this on Exynos platform.

Mohit,
If you test a switch on SPEAr platform, it will be
very helpful. Thank you.

Best regards,
Jingoo Han

--
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
Marek Vasut Nov. 29, 2013, 2:21 a.m. UTC | #12
Dear Jingoo Han,

> On Wednesday, November 27, 2013 5:37 PM, Pratyush Anand wrote:
> > On Wed, Nov 27, 2013 at 04:24:47PM +0800, Marek Vasut wrote:
> > > > On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > > > > Dear Pratyush Anand,
> > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > > > > 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.
> > > > > > > 
> > > > > > > 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.
> > > > > > 
> > > > > > It seems to me that in your controller you have only one view
> > > > > > port. Also mem_base and mem_bus_addr is same. Thats why it works
> > > > > > in your case.
> > > > > 
> > > > > MX6 has 4 In/4 Out viewports AFAICT.
> > > > 
> > > > Then if I do not miss anything, MX6 should work even without this
> > > > patch.
> > > 
> > > Yes, yet, it does not. On the other hand, this defect is only
> > > problematic if you use the devices behind a switch. Do you use a PCIe
> > > switch on your platform please?
> > 
> > I had tested with device under bridge.
> > Mohit is in process of validating SPEAr patches with latest kernel.
> > Mohit, see if we can arrange a switch and test, once SPEAr platform is
> > ready with latest kernel.
> 
> I agree with Pratyush Anand's opinion.
> One of our engineers had tested PCI cards under switch.
> As far as I know, there was no issue about this.
> However, currently, I don't have any switches. So, I
> Cannot test this on Exynos platform.

OK, I will wait for Richard to come up with confirmation if possible. Looks like 
he's out of the office, so it might take a bit.

> Mohit,
> If you test a switch on SPEAr platform, it will be
> very helpful. Thank you.
> 
> Best regards,
> Jingoo Han

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
Richard Zhu Dec. 3, 2013, 3:04 a.m. UTC | #13
Hi Marek:


> -----Original Message-----
> From: Marek Vasut [mailto:marex@denx.de]
> Sent: Friday, November 29, 2013 10:21 AM
> To: Jingoo Han
> Cc: 'Pratyush Anand'; 'Mohit KUMAR DCG'; 'Tim Harvey'; 'Kishon Vijay Abraham
> I'; linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; 'Bjorn
> Helgaas'; 'Frank Li'; Zhu Richard-R65037; 'Sascha Hauer'; 'Sean Cross'; 'Shawn
> Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky'; 'Yinghai Lu'
> Subject: Re: [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping
> 
> Dear Jingoo Han,
> 
> > On Wednesday, November 27, 2013 5:37 PM, Pratyush Anand wrote:
> > > On Wed, Nov 27, 2013 at 04:24:47PM +0800, Marek Vasut wrote:
> > > > > On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > > > > > Dear Pratyush Anand,
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > > > > > 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.
> > > > > > > >
> > > > > > > > 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.
> > > > > > >
> > > > > > > It seems to me that in your controller you have only one
> > > > > > > view port. Also mem_base and mem_bus_addr is same. Thats why
> > > > > > > it works in your case.
> > > > > >
> > > > > > MX6 has 4 In/4 Out viewports AFAICT.
> > > > >
> > > > > Then if I do not miss anything, MX6 should work even without
> > > > > this patch.
> > > >
> > > > Yes, yet, it does not. On the other hand, this defect is only
> > > > problematic if you use the devices behind a switch. Do you use a
> > > > PCIe switch on your platform please?
> > >
> > > I had tested with device under bridge.
> > > Mohit is in process of validating SPEAr patches with latest kernel.
> > > Mohit, see if we can arrange a switch and test, once SPEAr platform
> > > is ready with latest kernel.
> >
> > I agree with Pratyush Anand's opinion.
> > One of our engineers had tested PCI cards under switch.
> > As far as I know, there was no issue about this.
> > However, currently, I don't have any switches. So, I Cannot test this
> > on Exynos platform.
> 
> OK, I will wait for Richard to come up with confirmation if possible. Looks
> like he's out of the office, so it might take a bit.
> 
[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.

> > Mohit,
> > If you test a switch on SPEAr platform, it will be very helpful. Thank
> > you.
> >
> > Best regards,
> > Jingoo Han
> 
> Best regards,
> Marek Vasut

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
Marek Vasut Dec. 3, 2013, 9:19 a.m. UTC | #14
Dear Richard Zhu,

> Hi Marek:
> > -----Original Message-----
> > From: Marek Vasut [mailto:marex@denx.de]
> > Sent: Friday, November 29, 2013 10:21 AM
> > To: Jingoo Han
> > Cc: 'Pratyush Anand'; 'Mohit KUMAR DCG'; 'Tim Harvey'; 'Kishon Vijay
> > Abraham I'; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; 'Bjorn Helgaas'; 'Frank Li'; Zhu
> > Richard-R65037; 'Sascha Hauer'; 'Sean Cross'; 'Shawn Guo'; 'Siva Reddy
> > Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky'; 'Yinghai Lu' Subject: Re:
> > [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping
> > 
> > Dear Jingoo Han,
> > 
> > > On Wednesday, November 27, 2013 5:37 PM, Pratyush Anand wrote:
> > > > On Wed, Nov 27, 2013 at 04:24:47PM +0800, Marek Vasut wrote:
> > > > > > On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > > > > > > Dear Pratyush Anand,
> > > > > > > 
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > > > > > > 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.
> > 
> > > > > > > > > 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.
> > > > > > > > 
> > > > > > > > It seems to me that in your controller you have only one
> > > > > > > > view port. Also mem_base and mem_bus_addr is same. Thats why
> > > > > > > > it works in your case.
> > > > > > > 
> > > > > > > MX6 has 4 In/4 Out viewports AFAICT.
> > > > > > 
> > > > > > Then if I do not miss anything, MX6 should work even without
> > > > > > this patch.
> > > > > 
> > > > > Yes, yet, it does not. On the other hand, this defect is only
> > > > > problematic if you use the devices behind a switch. Do you use a
> > > > > PCIe switch on your platform please?
> > > > 
> > > > I had tested with device under bridge.
> > > > Mohit is in process of validating SPEAr patches with latest kernel.
> > > > Mohit, see if we can arrange a switch and test, once SPEAr platform
> > > > is ready with latest kernel.
> > > 
> > > I agree with Pratyush Anand's opinion.
> > > One of our engineers had tested PCI cards under switch.
> > > As far as I know, there was no issue about this.
> > > However, currently, I don't have any switches. So, I Cannot test this
> > > on Exynos platform.
> > 
> > OK, I will wait for Richard to come up with confirmation if possible.
> > Looks like he's out of the office, so it might take a bit.
> 
> [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.

How/what does have such pericom switch and can be attached to an MX6 sabresdp ? 
Where can I get it?

> 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.

Understood. Given that the iATU programming works on other CPUs (confirmed on st 
spear and ti dra7xx), we might have some issues with the iATU on MX6 . Is there 
anything special about the iATU on the MX6 ?
--
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
Richard Zhu Dec. 4, 2013, 2:38 a.m. UTC | #15
Hi Marek:

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org]
> On Behalf Of Marek Vasut
> Sent: Tuesday, December 03, 2013 5:20 PM
> To: Zhu Richard-R65037
> Cc: Jingoo Han; 'Pratyush Anand'; 'Mohit KUMAR DCG'; 'Tim Harvey'; 'Kishon
> Vijay Abraham I'; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; 'Bjorn Helgaas'; 'Frank Li'; 'Sascha Hauer'; 'Sean
> Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky';
> 'Yinghai Lu'
> Subject: Re: [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping
> 
> Dear Richard Zhu,
> 
> > Hi Marek:
> > > -----Original Message-----
> > > From: Marek Vasut [mailto:marex@denx.de]
> > > Sent: Friday, November 29, 2013 10:21 AM
> > > To: Jingoo Han
> > > Cc: 'Pratyush Anand'; 'Mohit KUMAR DCG'; 'Tim Harvey'; 'Kishon Vijay
> > > Abraham I'; linux-pci@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; 'Bjorn Helgaas'; 'Frank Li';
> > > Zhu Richard-R65037; 'Sascha Hauer'; 'Sean Cross'; 'Shawn Guo'; 'Siva
> > > Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky'; 'Yinghai Lu' Subject:
> Re:
> > > [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping
> > >
> > > Dear Jingoo Han,
> > >
> > > > On Wednesday, November 27, 2013 5:37 PM, Pratyush Anand wrote:
> > > > > On Wed, Nov 27, 2013 at 04:24:47PM +0800, Marek Vasut wrote:
> > > > > > > On Wed, Nov 27, 2013 at 04:46:09AM +0800, Marek Vasut wrote:
> > > > > > > > Dear Pratyush Anand,
> > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > On Wed, Oct 23, 2013 at 12:55:43PM +0800, Tim Harvey wrote:
> > > > > > > > > > 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.
> > >
> > > > > > > > > > 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.
> > > > > > > > >
> > > > > > > > > It seems to me that in your controller you have only one
> > > > > > > > > view port. Also mem_base and mem_bus_addr is same. Thats
> > > > > > > > > why it works in your case.
> > > > > > > >
> > > > > > > > MX6 has 4 In/4 Out viewports AFAICT.
> > > > > > >
> > > > > > > Then if I do not miss anything, MX6 should work even without
> > > > > > > this patch.
> > > > > >
> > > > > > Yes, yet, it does not. On the other hand, this defect is only
> > > > > > problematic if you use the devices behind a switch. Do you use
> > > > > > a PCIe switch on your platform please?
> > > > >
> > > > > I had tested with device under bridge.
> > > > > Mohit is in process of validating SPEAr patches with latest kernel.
> > > > > Mohit, see if we can arrange a switch and test, once SPEAr
> > > > > platform is ready with latest kernel.
> > > >
> > > > I agree with Pratyush Anand's opinion.
> > > > One of our engineers had tested PCI cards under switch.
> > > > As far as I know, there was no issue about this.
> > > > However, currently, I don't have any switches. So, I Cannot test
> > > > this on Exynos platform.
> > >
> > > OK, I will wait for Richard to come up with confirmation if possible.
> > > Looks like he's out of the office, so it might take a bit.
> >
> > [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.
> 
> How/what does have such pericom switch and can be attached to an MX6 sabresdp ?
> Where can I get it?
[Richard] You can apply the example from Pericom.
> 
> > 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.
> 
> Understood. Given that the iATU programming works on other CPUs (confirmed on
> st spear and ti dra7xx), we might have some issues with the iATU on MX6 . Is
> there anything special about the iATU on the MX6 ?
[Richard] As I know that there is no anything special about the iATU on MX6.
Let me to make a double check with IC team later.
> --
> 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

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
Marek Vasut Dec. 4, 2013, 3:45 p.m. UTC | #16
On Wednesday, December 04, 2013 at 03:38:49 AM, Richard Zhu wrote:
[...]
> > > [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.
> > 
> > How/what does have such pericom switch and can be attached to an MX6
> > sabresdp ? Where can I get it?
> 
> [Richard] You can apply the example from Pericom.

Which one? Please point me to a website or something here.

> > > 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.
> > 
> > Understood. Given that the iATU programming works on other CPUs
> > (confirmed on st spear and ti dra7xx), we might have some issues with
> > the iATU on MX6 . Is there anything special about the iATU on the MX6 ?
> 
> [Richard] As I know that there is no anything special about the iATU on
> MX6. Let me to make a double check with IC team later.

Oh this would be absolutelly _amazing_ if you could do that. Thank you very 
much!
[...]
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
Richard Zhu Dec. 5, 2013, 12:46 a.m. UTC | #17
Hi Marek:


> -----Original Message-----
> From: Marek Vasut [mailto:marex@denx.de]
> Sent: Wednesday, December 04, 2013 11:45 PM
> To: Zhu Richard-R65037
> Cc: Jingoo Han; 'Pratyush Anand'; 'Mohit KUMAR DCG'; 'Tim Harvey'; 'Kishon
> Vijay Abraham I'; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; 'Bjorn Helgaas'; 'Frank Li'; 'Sascha Hauer'; 'Sean
> Cross'; 'Shawn Guo'; 'Siva Reddy Kallam'; 'Srikanth T Shivanand'; 'Troy Kisky';
> 'Yinghai Lu'
> Subject: Re: [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping
> 
> On Wednesday, December 04, 2013 at 03:38:49 AM, Richard Zhu wrote:
> [...]
> > > > [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.
> > >
> > > How/what does have such pericom switch and can be attached to an MX6
> > > sabresdp ? Where can I get it?
> >
> > [Richard] You can apply the example from Pericom.
> 
> Which one? Please point me to a website or something here.
> 
[Richard] The model of the pcie switch used by me is PI7C9X2G303EL Ver3.0 provided by Pericom.
Here is the url of this switch.
http://www.pericom.com/products/pcie-switch/?part=PI7C9X2G303EL

> > > > 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.
> > >
> > > Understood. Given that the iATU programming works on other CPUs
> > > (confirmed on st spear and ti dra7xx), we might have some issues
> > > with the iATU on MX6 . Is there anything special about the iATU on the
> MX6 ?
> >
> > [Richard] As I know that there is no anything special about the iATU
> > on MX6. Let me to make a double check with IC team later.
> 
> Oh this would be absolutelly _amazing_ if you could do that. Thank you very
> much!
> [...]
> Best regards,
> Marek Vasut

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
diff mbox

Patch

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 5d1f268..6cce779 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)
@@ -502,8 +501,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);
@@ -513,38 +512,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,
@@ -560,11 +529,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;
@@ -583,11 +550,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;