[PATCH 2/7] [RFC] PCI: imx6: remove outbound io/mem ATU region mapping
Marek Vasut
marex at denx.de
Wed Nov 27 03:44:09 EST 2013
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 at 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 at gateworks.com>
> > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > Cc: Bjorn Helgaas <bhelgaas at google.com>
> > Cc: Frank Li <lznuaa at gmail.com>
> > Cc: Harro Haan <hrhaan at gmail.com>
> > Cc: Jingoo Han <jg1.han at samsung.com>
> > Cc: Mohit KUMAR <Mohit.KUMAR at st.com>
> > Cc: Pratyush Anand <pratyush.anand at st.com>
> > Cc: Richard Zhu <r65037 at freescale.com>
> > Cc: Sascha Hauer <s.hauer at pengutronix.de>
> > Cc: Sean Cross <xobs at kosagi.com>
> > Cc: Shawn Guo <shawn.guo at linaro.org>
> > Cc: Siva Reddy Kallam <siva.kallam at samsung.com>
> > Cc: Srikanth T Shivanand <ts.srikanth at samsung.com>
> > Cc: Tim Harvey <tharvey at gateworks.com>
> > Cc: Troy Kisky <troy.kisky at boundarydevices.com>
> > Cc: Yinghai Lu <yinghai at 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;
More information about the linux-arm-kernel
mailing list