From patchwork Fri Apr 11 14:32:28 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Petazzoni X-Patchwork-Id: 338534 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id D94FD14008B for ; Sat, 12 Apr 2014 00:32:39 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755784AbaDKOci (ORCPT ); Fri, 11 Apr 2014 10:32:38 -0400 Received: from top.free-electrons.com ([176.31.233.9]:46503 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756093AbaDKOcf (ORCPT ); Fri, 11 Apr 2014 10:32:35 -0400 Received: by mail.free-electrons.com (Postfix, from userid 106) id 918A7912; Fri, 11 Apr 2014 16:32:38 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on mail.free-electrons.com X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT shortcircuit=ham autolearn=disabled version=3.3.2 Received: from skate (col31-4-88-188-83-94.fbx.proxad.net [88.188.83.94]) by mail.free-electrons.com (Postfix) with ESMTPSA id 22F8A839; Fri, 11 Apr 2014 16:32:37 +0200 (CEST) Date: Fri, 11 Apr 2014 16:32:28 +0200 From: Thomas Petazzoni To: Jason Gunthorpe , Neil Greatorex , Willy Tarreau , Matthew Minter , Gerlando Falauto Cc: Lior Amsalem , Andrew Lunn , Jason Cooper , Tawfik Bayouk , linux-pci@vger.kernel.org, Ezequiel Garcia , Gregory =?UTF-8?B?Q2zDqW1lbnQ=?= , linux-arm-kernel@lists.infradead.org Subject: Re: Fixing PCIe issues on Armada XP Message-ID: <20140411163228.4214645c@skate> In-Reply-To: <20140410181953.50ccfcc3@skate> References: <20140410181953.50ccfcc3@skate> Organization: Free Electrons X-Mailer: Claws Mail 3.9.1 (GTK+ 2.24.20; x86_64-pc-linux-gnu) Mime-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Hello all, On Thu, 10 Apr 2014 18:19:53 +0200, Thomas Petazzoni wrote: > This is an e-mail that attempts to summarize the situation in terms of > Armada XP PCIe issues. Attached is a v2 of the patches to fix the various pci-mvebu issues. Changes since the version posted yesterday: * Include a fix for the timing issue of the PCIe interface that gets its clock disabled. I've chosen a different approach than the one suggested by Jason Gunthorpe, which does not involve resetting the PHY. I've tested my fix on the Mirabox, and the Armada 385 DB board on which Gregory originally reported the problem (I finally managed to reproduce the problem, it was due to the fact that only one of the PCIe interfaces is actually affected by the problem, because only the clock of the first PCIe interface is used by the mvebu-soc-id stuff). * Invert the order of Willy's and Jason's patches around MBus addresses. I've also: * Pushed the patches at https://github.com/MISL-EBU-System-SW/mainline-public/tree/3.14/pci-debug * Included a single combined patch, because I know one of you needs that to test easily. Can everybody test these patches, and confirm that they solve all the outstanding problems? Thanks! Thomas Tested-by: Neil Greatorex diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi index 74b5964..2188ce6 100644 --- a/arch/arm/boot/dts/armada-370-xp.dtsi +++ b/arch/arm/boot/dts/armada-370-xp.dtsi @@ -44,8 +44,8 @@ #size-cells = <1>; controller = <&mbusc>; interrupt-parent = <&mpic>; - pcie-mem-aperture = <0xe0000000 0x8000000>; - pcie-io-aperture = <0xe8000000 0x100000>; + pcie-mem-aperture = <0xf8000000 0x7e00000>; + pcie-io-aperture = <0xffe00000 0x100000>; devbus-bootcs { compatible = "marvell,mvebu-devbus"; diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts index bcf6d79..448373c 100644 --- a/arch/arm/boot/dts/armada-xp-db.dts +++ b/arch/arm/boot/dts/armada-xp-db.dts @@ -2,7 +2,7 @@ * Device Tree file for Marvell Armada XP evaluation board * (DB-78460-BP) * - * Copyright (C) 2012 Marvell + * Copyright (C) 2012-2014 Marvell * * Lior Amsalem * Gregory CLEMENT @@ -11,6 +11,15 @@ * This file is licensed under the terms of the GNU General Public * License version 2. This program is licensed "as is" without any * warranty of any kind, whether express or implied. + * + * Note: this Device Tree assumes that the bootloader has remapped the + * internal registers to 0xf1000000 (instead of the default + * 0xd0000000). The 0xf1000000 is the default used by the recent, + * DT-capable, U-Boot bootloaders provided by Marvell. Some earlier + * boards were delivered with an older version of the bootloader that + * left internal registers mapped at 0xd0000000. If you are in this + * situation, you should either update your bootloader (preferred + * solution) or the below Device Tree should be adjusted. */ /dts-v1/; @@ -30,7 +39,7 @@ }; soc { - ranges = ; diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts index 274e2ad..61bda68 100644 --- a/arch/arm/boot/dts/armada-xp-gp.dts +++ b/arch/arm/boot/dts/armada-xp-gp.dts @@ -2,7 +2,7 @@ * Device Tree file for Marvell Armada XP development board * (DB-MV784MP-GP) * - * Copyright (C) 2013 Marvell + * Copyright (C) 2013-2014 Marvell * * Lior Amsalem * Gregory CLEMENT @@ -11,6 +11,15 @@ * This file is licensed under the terms of the GNU General Public * License version 2. This program is licensed "as is" without any * warranty of any kind, whether express or implied. + * + * Note: this Device Tree assumes that the bootloader has remapped the + * internal registers to 0xf1000000 (instead of the default + * 0xd0000000). The 0xf1000000 is the default used by the recent, + * DT-capable, U-Boot bootloaders provided by Marvell. Some earlier + * boards were delivered with an older version of the bootloader that + * left internal registers mapped at 0xd0000000. If you are in this + * situation, you should either update your bootloader (preferred + * solution) or the below Device Tree should be adjusted. */ /dts-v1/; @@ -30,16 +39,17 @@ * 8 GB of plug-in RAM modules by default.The amount * of memory available can be changed by the * bootloader according the size of the module - * actually plugged. Only 7GB are usable because - * addresses from 0xC0000000 to 0xffffffff are used by - * the internal registers of the SoC. + * actually plugged. However, memory between + * 0xF0000000 to 0xFFFFFFFF cannot be used, as it is + * the address range used for I/O (internal registers, + * MBus windows). */ - reg = <0x00000000 0x00000000 0x00000000 0xC0000000>, + reg = <0x00000000 0x00000000 0x00000000 0xf0000000>, <0x00000001 0x00000000 0x00000001 0x00000000>; }; soc { - ranges = ; diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c index 725c461..54d339e 100644 --- a/drivers/bus/mvebu-mbus.c +++ b/drivers/bus/mvebu-mbus.c @@ -56,6 +56,7 @@ #include #include #include +#include /* * DDR target is the same on all platforms. @@ -222,12 +223,6 @@ static int mvebu_mbus_window_conflicts(struct mvebu_mbus_state *mbus, */ if ((u64)base < wend && end > wbase) return 0; - - /* - * Check if target/attribute conflicts - */ - if (target == wtarget && attr == wattr) - return 0; } return 1; @@ -266,6 +261,17 @@ static int mvebu_mbus_setup_window(struct mvebu_mbus_state *mbus, mbus->soc->win_cfg_offset(win); u32 ctrl, remap_addr; + if (!is_power_of_2(size)) { + WARN(true, "Invalid MBus window size: 0x%zx\n", size); + return -EINVAL; + } + + if ((base & (phys_addr_t)(size - 1)) != 0) { + WARN(true, "Invalid MBus base/size: %pa len 0x%zx\n", &base, + size); + return -EINVAL; + } + ctrl = ((size - 1) & WIN_CTRL_SIZE_MASK) | (attr << WIN_CTRL_ATTR_SHIFT) | (target << WIN_CTRL_TGT_SHIFT) | @@ -413,6 +419,10 @@ static int mvebu_devs_debug_show(struct seq_file *seq, void *v) win, (unsigned long long)wbase, (unsigned long long)(wbase + wsize), wtarget, wattr); + if (!is_power_of_2(wsize) || + ((wbase & (u64)(wsize - 1)) != 0)) + seq_puts(seq, " (Invalid base/size!!)"); + if (win < mbus->soc->num_remappable_wins) { seq_printf(seq, " (remap %016llx)\n", (unsigned long long)wremap); diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c index 5409564..939eb0d 100644 --- a/drivers/irqchip/irq-armada-370-xp.c +++ b/drivers/irqchip/irq-armada-370-xp.c @@ -130,8 +130,7 @@ static int armada_370_xp_setup_msi_irq(struct msi_chip *chip, struct msi_desc *desc) { struct msi_msg msg; - irq_hw_number_t hwirq; - int virq; + int virq, hwirq; hwirq = armada_370_xp_alloc_msi(); if (hwirq < 0) @@ -157,8 +156,19 @@ static void armada_370_xp_teardown_msi_irq(struct msi_chip *chip, unsigned int irq) { struct irq_data *d = irq_get_irq_data(irq); + unsigned long hwirq = d->hwirq; + irq_dispose_mapping(irq); - armada_370_xp_free_msi(d->hwirq); + armada_370_xp_free_msi(hwirq); +} + +static int armada_370_xp_check_msi_device(struct msi_chip *chip, struct pci_dev *dev, + int nvec, int type) +{ + /* We support MSI, but not MSI-X */ + if (type == PCI_CAP_ID_MSI) + return 0; + return -EINVAL; } static struct irq_chip armada_370_xp_msi_irq_chip = { @@ -199,6 +209,7 @@ static int armada_370_xp_msi_init(struct device_node *node, msi_chip->setup_irq = armada_370_xp_setup_msi_irq; msi_chip->teardown_irq = armada_370_xp_teardown_msi_irq; + msi_chip->check_device = armada_370_xp_check_msi_device; msi_chip->of_node = node; armada_370_xp_msi_domain = diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 46d31a4..d9c7eb2 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -1014,6 +1014,12 @@ static void igb_reset_q_vector(struct igb_adapter *adapter, int v_idx) { struct igb_q_vector *q_vector = adapter->q_vector[v_idx]; + /* Coming from igb_set_interrupt_capability, the vectors are not yet + * allocated. So, q_vector is NULL so we should stop here. + */ + if (!q_vector) + return; + if (q_vector->tx.ring) adapter->tx_ring[q_vector->tx.ring->queue_index] = NULL; @@ -1121,6 +1127,7 @@ static void igb_set_interrupt_capability(struct igb_adapter *adapter, bool msix) /* If we can't do MSI-X, try MSI */ msi_only: + adapter->flags &= ~IGB_FLAG_HAS_MSIX; #ifdef CONFIG_PCI_IOV /* disable SR-IOV for non MSI-X configurations */ if (adapter->vf_data) { diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index 0e79665..008d718 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -21,6 +21,7 @@ #include #include #include +#include /* * PCIe unit register offsets. @@ -162,6 +163,14 @@ static void mvebu_pcie_set_local_bus_nr(struct mvebu_pcie_port *port, int nr) mvebu_writel(port, stat, PCIE_STAT_OFF); } +static u32 mvebu_pcie_get_local_bus_nr(struct mvebu_pcie_port *port) +{ + u32 stat; + + stat = mvebu_readl(port, PCIE_STAT_OFF); + return (stat & PCIE_STAT_BUS) >> 8; +} + static void mvebu_pcie_set_local_dev_nr(struct mvebu_pcie_port *port, int nr) { u32 stat; @@ -172,6 +181,30 @@ static void mvebu_pcie_set_local_dev_nr(struct mvebu_pcie_port *port, int nr) mvebu_writel(port, stat, PCIE_STAT_OFF); } +static void mvebu_pcie_wait_dev(struct mvebu_pcie_port *port) +{ + int tries; + + for (tries = 0; tries < 1000; tries++) { + u32 vpid; + + mvebu_writel(port, + PCIE_CONF_ADDR(mvebu_pcie_get_local_bus_nr(port), + PCI_DEVFN(0, 0), PCI_VENDOR_ID), + PCIE_CONF_ADDR_OFF); + vpid = mvebu_readl(port, PCIE_CONF_DATA_OFF); + + if (vpid != 0xffffffff) + break; + + udelay(100); + } + + if (tries >= 1000) + dev_warn(&port->pcie->pdev->dev, + "timeout when looking for the PCIe device\n"); +} + /* * Setup PCIE BARs and Address Decode Wins: * BAR[0,2] -> disabled, BAR[1] -> covers all DRAM banks @@ -291,6 +324,58 @@ static int mvebu_pcie_hw_wr_conf(struct mvebu_pcie_port *port, return PCIBIOS_SUCCESSFUL; } +/* + * Remove windows, starting from the largest ones to the smallest + * ones. + */ +static void mvebu_pcie_del_windows(struct mvebu_pcie_port *port, + phys_addr_t base, size_t size) +{ + while (size) { + size_t sz = 1 << (fls(size) - 1); + + mvebu_mbus_del_window(base, sz); + base += sz; + size -= sz; + } +} + +/* + * MBus windows can only have a power of two size, but PCI BARs do not + * have this constraint. Therefore, we have to split the PCI BAR into + * areas each having a power of two size. We start from the largest + * one (i.e highest order bit set in the size). + */ +static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, + unsigned int target, unsigned int attribute, + phys_addr_t base, size_t size, + phys_addr_t remap) +{ + size_t size_mapped = 0; + + while (size) { + size_t sz = 1 << (fls(size) - 1); + int ret; + + ret = mvebu_mbus_add_window_remap_by_id(target, attribute, base, + sz, remap); + if (ret) { + dev_err(&port->pcie->pdev->dev, + "Could not create MBus window at 0x%x, size 0x%x: %d\n", + base, sz, ret); + mvebu_pcie_del_windows(port, base - size_mapped, + size_mapped); + return; + } + + size -= sz; + size_mapped += sz; + base += sz; + if (remap != MVEBU_MBUS_NO_REMAP) + remap += sz; + } +} + static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) { phys_addr_t iobase; @@ -302,8 +387,8 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) /* If a window was configured, remove it */ if (port->iowin_base) { - mvebu_mbus_del_window(port->iowin_base, - port->iowin_size); + mvebu_pcie_del_windows(port, port->iowin_base, + port->iowin_size); port->iowin_base = 0; port->iowin_size = 0; } @@ -329,11 +414,11 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) port->iowin_base = port->pcie->io.start + iobase; port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) | (port->bridge.iolimitupper << 16)) - - iobase); + iobase) + 1; - mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr, - port->iowin_base, port->iowin_size, - iobase); + mvebu_pcie_add_windows(port, port->io_target, port->io_attr, + port->iowin_base, port->iowin_size, + iobase); } static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) @@ -344,8 +429,8 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) /* If a window was configured, remove it */ if (port->memwin_base) { - mvebu_mbus_del_window(port->memwin_base, - port->memwin_size); + mvebu_pcie_del_windows(port, port->memwin_base, + port->memwin_size); port->memwin_base = 0; port->memwin_size = 0; } @@ -362,10 +447,11 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) port->memwin_base = ((port->bridge.membase & 0xFFF0) << 16); port->memwin_size = (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - - port->memwin_base; + port->memwin_base + 1; - mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr, - port->memwin_base, port->memwin_size); + mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr, + port->memwin_base, port->memwin_size, + MVEBU_MBUS_NO_REMAP); } /* @@ -721,14 +807,21 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, /* * On the PCI-to-PCI bridge side, the I/O windows must have at - * least a 64 KB size and be aligned on their size, and the - * memory windows must have at least a 1 MB size and be - * aligned on their size + * least a 64 KB size and the memory windows must have at + * least a 1 MB size. Moreover, MBus windows need to have a + * base address aligned on their size, and their size must be + * a power of two. This means that if the BAR doesn't have a + * power of two size, several MBus windows will actually be + * created. We need to ensure that the biggest MBus window + * (which will be the first one) is aligned on its size, which + * explains the rounddown_pow_of_two() being done here. */ if (res->flags & IORESOURCE_IO) - return round_up(start, max_t(resource_size_t, SZ_64K, size)); + return round_up(start, max_t(resource_size_t, SZ_64K, + rounddown_pow_of_two(size))); else if (res->flags & IORESOURCE_MEM) - return round_up(start, max_t(resource_size_t, SZ_1M, size)); + return round_up(start, max_t(resource_size_t, SZ_1M, + rounddown_pow_of_two(size))); else return start; } @@ -891,6 +984,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) for_each_child_of_node(pdev->dev.of_node, child) { struct mvebu_pcie_port *port = &pcie->ports[i]; enum of_gpio_flags flags; + int linkup; if (!of_device_is_available(child)) continue; @@ -975,8 +1069,13 @@ static int mvebu_pcie_probe(struct platform_device *pdev) continue; } + linkup = mvebu_pcie_link_up(port); + mvebu_pcie_set_local_dev_nr(port, 1); + if (linkup) + mvebu_pcie_wait_dev(port); + port->dn = child; spin_lock_init(&port->conf_lock); mvebu_sw_pci_bridge_init(port);