From patchwork Mon Aug 13 17:40:03 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thierry Reding X-Patchwork-Id: 1314381 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id D4C26E0000 for ; Mon, 13 Aug 2012 17:43:34 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1T0ycz-0003qn-2V; Mon, 13 Aug 2012 17:40:33 +0000 Received: from moutng.kundenserver.de ([212.227.17.10]) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1T0ycu-0003nT-QT for linux-arm-kernel@lists.infradead.org; Mon, 13 Aug 2012 17:40:29 +0000 Received: from mailbox.adnet.avionic-design.de (mailbox.avionic-design.de [109.75.18.3]) by mrelayeu.kundenserver.de (node=mreu0) with ESMTP (Nemesis) id 0Lv6gg-1Tjh0d1B34-010OqD; Mon, 13 Aug 2012 19:40:09 +0200 Received: from localhost (localhost [127.0.0.1]) by mailbox.adnet.avionic-design.de (Postfix) with ESMTP id A7EC22A282F5; Mon, 13 Aug 2012 19:40:08 +0200 (CEST) X-Virus-Scanned: amavisd-new at avionic-design.de Received: from mailbox.adnet.avionic-design.de ([127.0.0.1]) by localhost (mailbox.avionic-design.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mEp4t18i4jij; Mon, 13 Aug 2012 19:40:06 +0200 (CEST) Received: from localhost (avionic-0098.adnet.avionic-design.de [172.20.31.233]) (Authenticated sender: thierry.reding) by mailbox.adnet.avionic-design.de (Postfix) with ESMTPA id 77DF92A282FE; Mon, 13 Aug 2012 19:40:04 +0200 (CEST) Date: Mon, 13 Aug 2012 19:40:03 +0200 From: Thierry Reding To: Stephen Warren Subject: Re: [PATCH v3 00/10] ARM: tegra: Add PCIe device tree support Message-ID: <20120813174003.GA2527@avionic-0098.mockup.avionic-design.de> References: <1343332512-28762-1-git-send-email-thierry.reding@avionic-design.de> <50201E1D.5060200@wwwdotorg.org> MIME-Version: 1.0 In-Reply-To: <50201E1D.5060200@wwwdotorg.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Provags-ID: V02:K0:tizJK8fKadzhVyzBc9rJpiU8WSIsmh4svkZi5dZvTv8 OuqET5H7Lym8HCoktPf/CuDaautm5y4cJusrhnuEowQNaHGhSa niXatm91/fK2/mgqMxzqScAt4LV+YXyWdhaTAUBndBiUsTF4OB HKjfAtimI85L2aUfzt4aa3LvRF9dZVk91aymJUnEYzj4+kq+ef AqOnKIC8VkNMxmcABYwJfKRsY5KjBiYxT8xElaSBykoqwjMvP3 6zGzDkkgmFLYRdKnBLwzwkm++ZUpZaMf6P3WwMrepJWGvOw1TN JnEE9HqQgRlyTp8cQboIctNWLs17i1/m282SFetiRoWCJBn4NV Yv2G0f9kJ9wQycoFtuLtJ4E2DZ9y9TZxcTauzOoj9Boj3H1cED n2cjA45DIzza1De5UR5oDh4BxGerG63b0E= X-Spam-Note: CRM114 invocation failed X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [212.227.17.10 listed in list.dnswl.org] -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Mitch Bradley , Russell King , Arnd Bergmann , linux-pci@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, Olof Johansson , Rob Herring , Grant Likely , Bjorn Helgaas , Colin Cross , linux-tegra@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Mon, Aug 06, 2012 at 01:42:21PM -0600, Stephen Warren wrote: > On 07/26/2012 01:55 PM, Thierry Reding wrote: > > This patch series adds support for device tree based probing of the PCIe > > controller found on Tegra SoCs. > > Thierry, I just tested all Tegra boards in v3.6-rc1, and noticed that > PCIe doesn't work on TrimSlice when booting use device tree. I think I > found the cause, and I can't see why the same problem doesn't affect > this series. Perhaps you can enlighten me? > > When booting TrimSlice (or Harmony) using board files, Tegra's PCIe is > initialized using a subsys_initcall to tegra_pcie_init() directly (or > for Harmony to harmony_pcie_init() which then calls tegra_pcie_init()). > > The final thing tegra_pcie_init() does is call pci_common_init(). This > calls pcibios_init_hw() which calls hw->scan() which calls > pci_scan_root_bus() which adds a device object for each device on the > PCIe bus. However, since this happens very early in the boot sequence, I > believe the enumerated PCIe devices don't immediately get probed. > Instead, control gets returned to pci_common_init() which I believe then > calls pci_bus_assign_resources() which actually sets up the resources > for those devices. Later, the PCIe devices actually get probed, and > everything works. > > However, when booting using device tree, with the code currently in > v3.6-rc1, tegra_pcie_init() is called late in the boot sequence, and so > in the sequence described above, as soon as pci_scan_root_bus() adds a > device, it gets probed, before the device object's resources have been > set up, which results in the following failure: > > PCI: Device 0000:01:00.0 not available because of resource collisions > > ... because of the following code in pcibios_enable_device(): > > > for (idx = 0; idx < 6; idx++) { > > /* Only set up the requested stuff */ > > if (!(mask & (1 << idx))) > > continue; > > > > r = dev->resource + idx; > > if (!r->start && r->end) { > > printk(KERN_ERR "PCI: Device %s not available because" > > " of resource collisions\n", pci_name(dev)); > > Doesn't this same problem exist when instantiating the PCIe device > itself from device tree as in your patch series? If not, can you explain > why? > > Now, the obvious solution in v3.6 would be to simply have > tegra_pcie_init() be called at the same early stage in the boot process > when booting using device tree as it is when booting using board files. > This works for TrimSlice. > > However, on Harmony, it doesn't work, because PCIe on Harmony depends on > regulators, and the regulators are accessed using an I2C bus that is > instantiated from DT, and the instantiation of the I2C bus happens > fairly late in the boot process so can't be found early during the boot > sequence. See harmony_regulator_init() for the failing code. > > Does anyone have any good ideas (small, self-contained patches) for > solving this in v3.6 in such a way that PCIe works on both TrimSlice and > Harmony? > > Thanks. I've looked into this a bit, and it seems like ARM is using an open- coded version of the pci_enable_resources() function here, with the only difference being the unconditional enabling of both I/O and memory- mapped access for bridges. On Tegra there is already a PCI fixup to do this, so pci_enable_resources() can be used as-is. I came up with the attached patch but haven't been able to test it yet. Thierry From ebd69ae0a3d076e574da74d963cb3834b71dc6ad Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 13 Aug 2012 18:49:28 +0200 Subject: [PATCH] ARM: PCI: refactor pcibios_enable_device() The implementation is an open-coded version on pci_enable_resources() with a special case to enable I/O and memory-mapped functionality on bridges. This commit reuses the existing PCI core implementation of the pci_enable_resources() function. This also means that bridges no longer enable I/O and memory-mapped functionality unconditionally. Platforms where this is really required can add a corresponding fixup. Signed-off-by: Thierry Reding --- arch/arm/kernel/bios32.c | 36 +----------------------------------- 1 file changed, 1 insertion(+), 35 deletions(-) diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c index 13fd97b..dfe25f7 100644 --- a/arch/arm/kernel/bios32.c +++ b/arch/arm/kernel/bios32.c @@ -601,41 +601,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res, */ int pcibios_enable_device(struct pci_dev *dev, int mask) { - u16 cmd, old_cmd; - int idx; - struct resource *r; - - pci_read_config_word(dev, PCI_COMMAND, &cmd); - old_cmd = cmd; - for (idx = 0; idx < 6; idx++) { - /* Only set up the requested stuff */ - if (!(mask & (1 << idx))) - continue; - - r = dev->resource + idx; - if (!r->start && r->end) { - printk(KERN_ERR "PCI: Device %s not available because" - " of resource collisions\n", pci_name(dev)); - return -EINVAL; - } - if (r->flags & IORESOURCE_IO) - cmd |= PCI_COMMAND_IO; - if (r->flags & IORESOURCE_MEM) - cmd |= PCI_COMMAND_MEMORY; - } - - /* - * Bridges (eg, cardbus bridges) need to be fully enabled - */ - if ((dev->class >> 16) == PCI_BASE_CLASS_BRIDGE) - cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY; - - if (cmd != old_cmd) { - printk("PCI: enabling device %s (%04x -> %04x)\n", - pci_name(dev), old_cmd, cmd); - pci_write_config_word(dev, PCI_COMMAND, cmd); - } - return 0; + return pci_enable_resources(dev, mask); } int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,