From patchwork Tue Dec 3 20:14:47 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Harvey X-Patchwork-Id: 3278631 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 070CC9F377 for ; Tue, 3 Dec 2013 20:16:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EDBB120445 for ; Tue, 3 Dec 2013 20:16:28 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2509620414 for ; Tue, 3 Dec 2013 20:16:27 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VnwNj-0006o1-Ml; Tue, 03 Dec 2013 20:15:44 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VnwNP-0007yX-Sc; Tue, 03 Dec 2013 20:15:23 +0000 Received: from mail-we0-f170.google.com ([74.125.82.170]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VnwNM-0007vx-RK for linux-arm-kernel@lists.infradead.org; Tue, 03 Dec 2013 20:15:22 +0000 Received: by mail-we0-f170.google.com with SMTP id w61so14255603wes.29 for ; Tue, 03 Dec 2013 12:14:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=/VwunmK1TpkT7/N9kEZkXJpAGF8CxTMwAC97tp24b1Q=; b=hRjRQSjuZ9oqCEx5W4HvOtGEVOtOorx0V3vjNu5NaHy2jOWWCV8DvNHAgx1tb1bgO4 rpfSUMGwudR14zfjmg7SgY8t0dPqPsYViYe6DImy21CG317aocA/K84wtFQujRV3TIj0 Y0t34iflt2u2DSxnU3vKGnF6GbTPDZ2j2vbbCOBqWFvHtaGAYkGEAzQlptwYWVIjOqd2 A1OP+/xir5QHKaJku7SIAN2lGDnKKUnzj4p776Xa8DL/ui3APeT1V+Q/5sAhhfKGagS7 tpi8m8ytqZvMLdTA2mDuwxl0/7YpaoEk2MbtDqbX7WBBYV1QWy/kgAKH8crEWwtX4lY6 yUvw== X-Gm-Message-State: ALoCoQmQYoQsxNzF/Xf7EGsfEgNvuNtuz6Fyo8l59fseJhnWru9lsIXqMd6bHJzZZiZgRMxzA6PZ MIME-Version: 1.0 X-Received: by 10.180.13.142 with SMTP id h14mr4152357wic.3.1386101687351; Tue, 03 Dec 2013 12:14:47 -0800 (PST) Received: by 10.194.139.114 with HTTP; Tue, 3 Dec 2013 12:14:47 -0800 (PST) In-Reply-To: <201311270944.09383.marex@denx.de> References: <1385500248-6551-1-git-send-email-marex@denx.de> <1385500248-6551-2-git-send-email-marex@denx.de> <20131127043009.GB12773@pratyush-vbox> <201311270944.09383.marex@denx.de> Date: Tue, 3 Dec 2013 12:14:47 -0800 Message-ID: Subject: Re: [PATCH 2/7] [RFC] PCI: imx6: remove outbound io/mem ATU region mapping From: Tim Harvey To: Marek Vasut X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131203_151521_104104_A1FBF6F9 X-CRM114-Status: GOOD ( 34.03 ) X-Spam-Score: -2.6 (--) Cc: Richard Zhu , Pratyush Anand , Siva Reddy Kallam , "linux-pci@vger.kernel.org" , Sascha Hauer , Jingoo Han , Harro Haan , Troy Kisky , Frank Li , Mohit KUMAR DCG , Bjorn Helgaas , Sean Cross , Shawn Guo , Yinghai Lu , Srikanth T Shivanand , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Nov 27, 2013 at 12:44 AM, Marek Vasut wrote: > Dear Pratyush Anand, > >> Dear Marek Vasut, >> >> Please replace imx6 with designware in subject line of this patch. > > OK, this one has to be fixed anyway. We need to figure out why do we even need > this one. > >> On Wed, Nov 27, 2013 at 05:10:43AM +0800, Marek Vasut wrote: >> > From: Tim Harvey >> > >> > The IMX6 iATU is used for address translation between the AXI bus >> > address space and PCI address space. This is used for type0 and type1 >> > config cycles but is not necessary for outbound io/mem regions. >> > >> > This patch removes the calls that inappropriately re-configures the ATU >> > viewport for outbound memory and IO after config cycles and removes them >> > altogether as they are not necessary. >> >> Yes, they are not necessary for all the cases where translation is one >> to one. So so for sure all the platform till now introduced should >> work. >> But, what about a platform where memory translation is not one to one? >> >> Existing code should work with all sort of memory translation on a >> platform having atleast two viewports capable of programming any type >> of outbound transaction. > > Full ACK, that's why it's called RFC. > >> > This resolves issues with PCI devices behind switches and has been tested >> > with a Gige device behind a PLX PEX860x switch. More testing is needed >> > for other configurations. >> >> I do not understand if MX6 has 4 Outbound Viewport then how this patch >> helps? >> -- PCI devices behind switches would not have been working because >> CFG1 transaction would not have been correct. >> -- It works with this patch. This patch changes viewport for CFG1 from >> 1 to 0. >> -- Can it be possible that MX6 has some restriction on viewport >> programming capability. I mean,like only viewport0 can be programmed >> for CFG0/1? > > Tim ? > > Here is the MX6 datasheet [1], the section 48.3.9.1.1 and 48.3.9.1.2 describe > the iATU configuration on MX6. My understanding from this description is that > the MX6 has 4 inbound and 4 outbound iATU regions. Am I wrong ? > > [1] http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf > >> Otherwise I do not find any convincing reason that why this patch >> helps in resolving issues with PCI devices behind switches. In my configuration I have a PLX switch off the IMX root complex with several devices behind it. I find that the devices enumerate correctly but as soon as data is transferred to or from (not sure which) the device the system hangs (INFO: rcu_sched detected stalls on CPUs/tasks: { 0} (detected by 3, t=2135 jiffies, g=20, c=19, q=66)). My current test case is a GigE ethernet device and when I connect a network to the device I hange when a packet is responded to. I can't claim to fully understand PCI resource mappings or the iATU and I don't understand why dw_pcie_rd_other_conf/dw_pcie_wr_other_conf need to change the viewport then change it back instead of using multiple viewports (perhaps because some hardware may not have more than the 2 viewports currently being used?). The current driver uses viewport0 for cfg0 and mem and viewport1 for cfg1 and io. If I remove the call to dw_pcie_prog_viewport_io_outbound to reconfigure viewport1 for io after its altered for type1 cfg cycles, devices behind the bridge work for me: perhaps break devices that use IO resources? Thanks, Tim >> >> >> Regards >> Pratyush >> >> > From: Tim Harvey >> > Signed-off-by: Tim Harvey >> > Cc: Bjorn Helgaas >> > Cc: Frank Li >> > Cc: Harro Haan >> > Cc: Jingoo Han >> > Cc: Mohit KUMAR >> > Cc: Pratyush Anand >> > Cc: Richard Zhu >> > Cc: Sascha Hauer >> > Cc: Sean Cross >> > Cc: Shawn Guo >> > Cc: Siva Reddy Kallam >> > Cc: Srikanth T Shivanand >> > Cc: Tim Harvey >> > Cc: Troy Kisky >> > Cc: Yinghai Lu >> > --- >> > >> > 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; diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designwa index e33b68b..a064bc5 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -556,7 +556,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struc } else { dw_pcie_prog_viewport_cfg1(pp, busdev); ret = cfg_read(pp->va_cfg1_base + address, where, size, val); - dw_pcie_prog_viewport_io_outbound(pp); +// dw_pcie_prog_viewport_io_outbound(pp); } return ret; @@ -579,7 +579,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struc } else { dw_pcie_prog_viewport_cfg1(pp, busdev); ret = cfg_write(pp->va_cfg1_base + address, where, size, val); - dw_pcie_prog_viewport_io_outbound(pp); +// dw_pcie_prog_viewport_io_outbound(pp); } Can anyone explain whats going on here? I would think this would