All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bhelgaas@google.com>
To: Tim Harvey <tharvey@gateworks.com>
Cc: linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Marek Vasut <marex@denx.de>, Frank Li <lznuaa@gmail.com>,
	Jingoo Han <jg1.han@samsung.com>,
	Mohit KUMAR <Mohit.KUMAR@st.com>,
	Pratyush Anand <pratyush.anand@st.com>,
	Richard Zhu <r65037@freescale.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Sean Cross <xobs@kosagi.com>, Shawn Guo <shawn.guo@linaro.org>,
	Siva Reddy Kallam <siva.kallam@samsung.com>,
	Srikanth T Shivanand <ts.srikanth@samsung.com>,
	Troy Kisky <troy.kisky@boundarydevices.com>,
	Yinghai Lu <yinghai@kernel.org>
Subject: Re: [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping
Date: Mon, 25 Nov 2013 16:09:35 -0700	[thread overview]
Message-ID: <20131125230935.GA4034@google.com> (raw)
In-Reply-To: <1382504143-27915-1-git-send-email-tharvey@gateworks.com>

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
> 

WARNING: multiple messages have this Message-ID (diff)
From: bhelgaas@google.com (Bjorn Helgaas)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping
Date: Mon, 25 Nov 2013 16:09:35 -0700	[thread overview]
Message-ID: <20131125230935.GA4034@google.com> (raw)
In-Reply-To: <1382504143-27915-1-git-send-email-tharvey@gateworks.com>

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
> 

  parent reply	other threads:[~2013-11-25 23:09 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-23  4:55 [PATCH RFC] PCI: imx6: remove outbound io/mem ATU region mapping Tim Harvey
2013-10-23  4:55 ` Tim Harvey
2013-10-23  5:04 ` Marek Vasut
2013-10-23  5:04   ` Marek Vasut
2013-10-23  6:40 ` Pratyush Anand
2013-10-23  6:40   ` Pratyush Anand
2013-11-26 20:46   ` Marek Vasut
2013-11-26 20:46     ` Marek Vasut
2013-11-27  3:50     ` Pratyush Anand
2013-11-27  3:50       ` Pratyush Anand
2013-11-27  7:42       ` Richard Zhu
2013-11-27  7:42         ` Richard Zhu
2013-11-27  8:24       ` Marek Vasut
2013-11-27  8:24         ` Marek Vasut
2013-11-27  8:36         ` Pratyush Anand
2013-11-27  8:36           ` Pratyush Anand
2013-11-27  8:45           ` Marek Vasut
2013-11-27  8:45             ` Marek Vasut
2013-11-28  5:54           ` Jingoo Han
2013-11-28  5:54             ` Jingoo Han
2013-11-29  2:21             ` Marek Vasut
2013-11-29  2:21               ` Marek Vasut
2013-12-03  3:04               ` Richard Zhu
2013-12-03  3:04                 ` Richard Zhu
2013-12-03  9:19                 ` Marek Vasut
2013-12-03  9:19                   ` Marek Vasut
2013-12-04  2:38                   ` Richard Zhu
2013-12-04  2:38                     ` Richard Zhu
2013-12-04 15:45                     ` Marek Vasut
2013-12-04 15:45                       ` Marek Vasut
2013-12-05  0:46                       ` Richard Zhu
2013-12-05  0:46                         ` Richard Zhu
2013-11-25 23:09 ` Bjorn Helgaas [this message]
2013-11-25 23:09   ` Bjorn Helgaas
2013-11-26 20:47   ` Marek Vasut
2013-11-26 20:47     ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131125230935.GA4034@google.com \
    --to=bhelgaas@google.com \
    --cc=Mohit.KUMAR@st.com \
    --cc=jg1.han@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lznuaa@gmail.com \
    --cc=marex@denx.de \
    --cc=pratyush.anand@st.com \
    --cc=r65037@freescale.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawn.guo@linaro.org \
    --cc=siva.kallam@samsung.com \
    --cc=tharvey@gateworks.com \
    --cc=troy.kisky@boundarydevices.com \
    --cc=ts.srikanth@samsung.com \
    --cc=xobs@kosagi.com \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.