Hello Greg, Sasha,
this series for 4.14-stable backports all the fixes (and their dependencies) for Armada 3720 PCIe driver. These include: - fixes (and their dependencies) for pci-aardvark controller - fixes (and their dependencies) for pinctrl-armada-37xx driver - device-tree fixes
Basically all fixes from upstream are taken, excluding those that need fix the emulated bridge, since that was introduced after 4.19. (Should we backport it? It concerns only mvebu and aardvark controllers...)
Marek
Evan Wang (1): PCI: aardvark: Remove PCIe outbound window configuration
Frederick Lawler (1): PCI: Add PCI_EXP_LNKCTL2_TLS* macros
Gregory CLEMENT (1): pinctrl: armada-37xx: add missing pin: PCIe1 Wakeup
Marek Behún (4): PCI: aardvark: Improve link training pinctrl: armada-37xx: Correct mpp definitions pinctrl: armada-37xx: Correct PWM pins definitions arm64: dts: marvell: armada-37xx: Set pcie_reset_pin to gpio function
Miquel Raynal (1): arm64: dts: marvell: armada-37xx: declare PCIe reset pin
Pali Rohár (12): PCI: aardvark: Train link immediately after enabling training PCI: aardvark: Issue PERST via GPIO PCI: aardvark: Replace custom macros by standard linux/pci_regs.h macros PCI: aardvark: Indicate error in 'val' when config read fails PCI: aardvark: Don't touch PCIe registers if no card connected PCI: aardvark: Fix compilation on s390 PCI: aardvark: Move PCIe reset card code to advk_pcie_train_link() PCI: aardvark: Update comment about disabling link training PCI: aardvark: Configure PCIe resources from 'ranges' DT property PCI: aardvark: Fix PCIe Max Payload Size setting PCI: aardvark: Fix link training PCI: aardvark: Fix checking for link up via LTSSM state
Remi Pommarel (1): PCI: aardvark: Wait for endpoint to be ready before training link
Sergei Shtylyov (1): PCI: aardvark: Fix I/O space page leak
Thomas Petazzoni (1): PCI: aardvark: Introduce an advk_pcie_valid_device() helper
Wen Yang (1): PCI: aardvark: Fix a leaked reference by adding missing of_node_put()
.../pinctrl/marvell,armada-37xx-pinctrl.txt | 26 +- .../arm64/boot/dts/marvell/armada-3720-db.dts | 3 + .../dts/marvell/armada-3720-espressobin.dts | 3 + arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 9 + drivers/pci/host/pci-aardvark.c | 463 ++++++++++++++---- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 28 +- include/uapi/linux/pci_regs.h | 5 + 7 files changed, 430 insertions(+), 107 deletions(-)
From: Sergei Shtylyov sergei.shtylyov@cogentembedded.com
commit 1df3e5b3feebf29a3ecfa0c0f06f79544ca573e4 upstream.
When testing the R-Car PCIe driver on the Condor board, if the PCIe PHY driver was left disabled, the kernel crashed with this BUG:
kernel BUG at lib/ioremap.c:72! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 39 Comm: kworker/0:1 Not tainted 4.17.0-dirty #1092 Hardware name: Renesas Condor board based on r8a77980 (DT) Workqueue: events deferred_probe_work_func pstate: 80000005 (Nzcv daif -PAN -UAO) pc : ioremap_page_range+0x370/0x3c8 lr : ioremap_page_range+0x40/0x3c8 sp : ffff000008da39e0 x29: ffff000008da39e0 x28: 00e8000000000f07 x27: ffff7dfffee00000 x26: 0140000000000000 x25: ffff7dfffef00000 x24: 00000000000fe100 x23: ffff80007b906000 x22: ffff000008ab8000 x21: ffff000008bb1d58 x20: ffff7dfffef00000 x19: ffff800009c30fb8 x18: 0000000000000001 x17: 00000000000152d0 x16: 00000000014012d0 x15: 0000000000000000 x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720 x11: 0720072007300730 x10: 00000000000000ae x9 : 0000000000000000 x8 : ffff7dffff000000 x7 : 0000000000000000 x6 : 0000000000000100 x5 : 0000000000000000 x4 : 000000007b906000 x3 : ffff80007c61a880 x2 : ffff7dfffeefffff x1 : 0000000040000000 x0 : 00e80000fe100f07 Process kworker/0:1 (pid: 39, stack limit = 0x (ptrval)) Call trace: ioremap_page_range+0x370/0x3c8 pci_remap_iospace+0x7c/0xac pci_parse_request_of_pci_ranges+0x13c/0x190 rcar_pcie_probe+0x4c/0xb04 platform_drv_probe+0x50/0xbc driver_probe_device+0x21c/0x308 __device_attach_driver+0x98/0xc8 bus_for_each_drv+0x54/0x94 __device_attach+0xc4/0x12c device_initial_probe+0x10/0x18 bus_probe_device+0x90/0x98 deferred_probe_work_func+0xb0/0x150 process_one_work+0x12c/0x29c worker_thread+0x200/0x3fc kthread+0x108/0x134 ret_from_fork+0x10/0x18 Code: f9004ba2 54000080 aa0003fb 17ffff48 (d4210000)
It turned out that pci_remap_iospace() wasn't undone when the driver's probe failed, and since devm_phy_optional_get() returned -EPROBE_DEFER, the probe was retried, finally causing the BUG due to trying to remap already remapped pages.
The Aardvark PCI controller driver has the same issue. Replace pci_remap_iospace() with its devm_ managed version to fix the bug.
Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller driver") Signed-off-by: Sergei Shtylyov sergei.shtylyov@cogentembedded.com [lorenzo.pieralisi@arm.com: updated the commit log] Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Bjorn Helgaas bhelgaas@google.com Reviewed-by: Thomas Petazzoni thomas.petazzoni@bootlin.com Reviewed-by: Linus Walleij linus.walleij@linaro.org Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index a572b2fb7af8..fd605796b011 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -932,7 +932,7 @@ static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie) 0, 0xF8000000, 0, lower_32_bits(res->start), OB_PCIE_IO); - err = pci_remap_iospace(res, iobase); + err = devm_pci_remap_iospace(dev, res, iobase); if (err) { dev_warn(dev, "error %d: failed to map resource %pR\n", err, res);
From: Wen Yang wen.yang99@zte.com.cn
commit 3842f5166bf1ef286fe7a39f262b5c9581308366 upstream.
The call to of_get_next_child() returns a node pointer with refcount incremented thus it must be explicitly decremented after the last usage.
irq_domain_add_linear() also calls of_node_get() to increase refcount, so irq_domain will not be affected when it is released.
Detected by coccinelle with the following warnings: ./drivers/pci/controller/pci-aardvark.c:826:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 798, but without a corresponding object release within this function.
Signed-off-by: Wen Yang wen.yang99@zte.com.cn Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Cc: Thomas Petazzoni thomas.petazzoni@bootlin.com Cc: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Cc: Bjorn Helgaas bhelgaas@google.com Cc: linux-pci@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index fd605796b011..79cd11b1c89a 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -789,6 +789,7 @@ static int advk_pcie_init_irq_domain(struct advk_pcie *pcie) struct device_node *node = dev->of_node; struct device_node *pcie_intc_node; struct irq_chip *irq_chip; + int ret = 0;
raw_spin_lock_init(&pcie->irq_lock);
@@ -803,8 +804,8 @@ static int advk_pcie_init_irq_domain(struct advk_pcie *pcie) irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-irq", dev_name(dev)); if (!irq_chip->name) { - of_node_put(pcie_intc_node); - return -ENOMEM; + ret = -ENOMEM; + goto out_put_node; }
irq_chip->irq_mask = advk_pcie_irq_mask; @@ -816,11 +817,13 @@ static int advk_pcie_init_irq_domain(struct advk_pcie *pcie) &advk_pcie_irq_domain_ops, pcie); if (!pcie->irq_domain) { dev_err(dev, "Failed to get a INTx IRQ domain\n"); - of_node_put(pcie_intc_node); - return -ENOMEM; + ret = -ENOMEM; + goto out_put_node; }
- return 0; +out_put_node: + of_node_put(pcie_intc_node); + return ret; }
static void advk_pcie_remove_irq_domain(struct advk_pcie *pcie)
From: Remi Pommarel repk@triplefau.lt
commit f4c7d053d7f77cd5c1a1ba7c7ce085ddba13d1d7 upstream.
When configuring pcie reset pin from gpio (e.g. initially set by u-boot) to pcie function this pin goes low for a brief moment asserting the PERST# signal. Thus connected device enters fundamental reset process and link configuration can only begin after a minimal 100ms delay (see [1]).
Because the pin configuration comes from the "default" pinctrl it is implicitly configured before the probe callback is called:
driver_probe_device() really_probe() ... pinctrl_bind_pins() /* Here pin goes from gpio to PCIE reset function and PERST# is asserted */ ... drv->probe()
[1] "PCI Express Base Specification", REV. 4.0 PCI Express, February 19 2014, 6.6.1 Conventional Reset
Signed-off-by: Remi Pommarel repk@triplefau.lt Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Acked-by: Thomas Petazzoni thomas.petazzoni@bootlin.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 79cd11b1c89a..7ee5a91e5f7f 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -362,6 +362,14 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg |= PIO_CTRL_ADDR_WIN_DISABLE; advk_writel(pcie, reg, PIO_CTRL);
+ /* + * PERST# signal could have been asserted by pinctrl subsystem before + * probe() callback has been called, making the endpoint going into + * fundamental reset. As required by PCI Express spec a delay for at + * least 100ms after such a reset before link training is needed. + */ + msleep(PCI_PM_D3COLD_WAIT); + /* Start link training */ reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); reg |= PCIE_CORE_LINK_TRAINING;
From: Pali Rohár pali@kernel.org
commit 6964494582f56a3882c2c53b0edbfe99eb32b2e1 upstream.
Adding even 100ms (PCI_PM_D3COLD_WAIT) delay between enabling link training and starting link training causes detection issues with some buggy cards (such as Compex WLE900VX).
Move the code which enables link training immediately before the one which starts link traning.
This fixes detection issues of Compex WLE900VX card on Turris MOX after cold boot.
Link: https://lore.kernel.org/r/20200430080625.26070-2-pali@kernel.org Fixes: f4c7d053d7f7 ("PCI: aardvark: Wait for endpoint to be ready...") Tested-by: Tomasz Maciej Nowak tmn505@gmail.com Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Acked-by: Rob Herring robh@kernel.org Acked-by: Thomas Petazzoni thomas.petazzoni@bootlin.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 7ee5a91e5f7f..8373f8cc4c52 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -324,11 +324,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg |= LANE_COUNT_1; advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
- /* Enable link training */ - reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); - reg |= LINK_TRAINING_EN; - advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); - /* Enable MSI */ reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG); reg |= PCIE_CORE_CTRL2_MSI_ENABLE; @@ -370,7 +365,15 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) */ msleep(PCI_PM_D3COLD_WAIT);
- /* Start link training */ + /* Enable link training */ + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); + reg |= LINK_TRAINING_EN; + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); + + /* + * Start link training immediately after enabling it. + * This solves problems for some buggy cards. + */ reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); reg |= PCIE_CORE_LINK_TRAINING; advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG);
From: Marek Behún marek.behun@nic.cz
commit 43fc679ced18006b12d918d7a8a4af392b7fbfe7 upstream.
Currently the aardvark driver trains link in PCIe gen2 mode. This may cause some buggy gen1 cards (such as Compex WLE900VX) to be unstable or even not detected. Moreover when ASPM code tries to retrain link second time, these cards may stop responding and link goes down. If gen1 is used this does not happen.
Unconditionally forcing gen1 is not a good solution since it may have performance impact on gen2 cards.
To overcome this, read 'max-link-speed' property (as defined in PCI device tree bindings) and use this as max gen mode. Then iteratively try link training at this mode or lower until successful. After successful link training choose final controller gen based on Negotiated Link Speed from Link Status register, which should match card speed.
Link: https://lore.kernel.org/r/20200430080625.26070-5-pali@kernel.org Tested-by: Tomasz Maciej Nowak tmn505@gmail.com Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún marek.behun@nic.cz Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Reviewed-by: Rob Herring robh@kernel.org Acked-by: Thomas Petazzoni thomas.petazzoni@bootlin.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 114 +++++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 25 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 8373f8cc4c52..714d81462297 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -36,6 +36,7 @@ #define PCIE_CORE_LINK_CTRL_STAT_REG 0xd0 #define PCIE_CORE_LINK_L0S_ENTRY BIT(0) #define PCIE_CORE_LINK_TRAINING BIT(5) +#define PCIE_CORE_LINK_SPEED_SHIFT 16 #define PCIE_CORE_LINK_WIDTH_SHIFT 20 #define PCIE_CORE_ERR_CAPCTL_REG 0x118 #define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX BIT(5) @@ -212,6 +213,7 @@ struct advk_pcie { struct mutex msi_used_lock; u16 msi_msg; int root_bus_nr; + int link_gen; };
static inline void advk_writel(struct advk_pcie *pcie, u32 val, u64 reg) @@ -235,20 +237,16 @@ static int advk_pcie_link_up(struct advk_pcie *pcie)
static int advk_pcie_wait_for_link(struct advk_pcie *pcie) { - struct device *dev = &pcie->pdev->dev; int retries;
/* check if the link is up or not */ for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) { - if (advk_pcie_link_up(pcie)) { - dev_info(dev, "link up\n"); + if (advk_pcie_link_up(pcie)) return 0; - }
usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX); }
- dev_err(dev, "link never came up\n"); return -ETIMEDOUT; }
@@ -272,6 +270,85 @@ static void advk_pcie_set_ob_win(struct advk_pcie *pcie, advk_writel(pcie, match_ls | BIT(0), OB_WIN_MATCH_LS(win_num)); }
+static int advk_pcie_train_at_gen(struct advk_pcie *pcie, int gen) +{ + int ret, neg_gen; + u32 reg; + + /* Setup link speed */ + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); + reg &= ~PCIE_GEN_SEL_MSK; + if (gen == 3) + reg |= SPEED_GEN_3; + else if (gen == 2) + reg |= SPEED_GEN_2; + else + reg |= SPEED_GEN_1; + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); + + /* + * Enable link training. This is not needed in every call to this + * function, just once suffices, but it does not break anything either. + */ + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); + reg |= LINK_TRAINING_EN; + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); + + /* + * Start link training immediately after enabling it. + * This solves problems for some buggy cards. + */ + reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); + reg |= PCIE_CORE_LINK_TRAINING; + advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG); + + ret = advk_pcie_wait_for_link(pcie); + if (ret) + return ret; + + reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); + neg_gen = (reg >> PCIE_CORE_LINK_SPEED_SHIFT) & 0xf; + + return neg_gen; +} + +static void advk_pcie_train_link(struct advk_pcie *pcie) +{ + struct device *dev = &pcie->pdev->dev; + int neg_gen = -1, gen; + + /* + * Try link training at link gen specified by device tree property + * 'max-link-speed'. If this fails, iteratively train at lower gen. + */ + for (gen = pcie->link_gen; gen > 0; --gen) { + neg_gen = advk_pcie_train_at_gen(pcie, gen); + if (neg_gen > 0) + break; + } + + if (neg_gen < 0) + goto err; + + /* + * After successful training if negotiated gen is lower than requested, + * train again on negotiated gen. This solves some stability issues for + * some buggy gen1 cards. + */ + if (neg_gen < gen) { + gen = neg_gen; + neg_gen = advk_pcie_train_at_gen(pcie, gen); + } + + if (neg_gen == gen) { + dev_info(dev, "link up at gen %i\n", gen); + return; + } + +err: + dev_err(dev, "link never came up\n"); +} + static void advk_pcie_setup_hw(struct advk_pcie *pcie) { u32 reg; @@ -312,12 +389,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) PCIE_CORE_CTRL2_TD_ENABLE; advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
- /* Set GEN2 */ - reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); - reg &= ~PCIE_GEN_SEL_MSK; - reg |= SPEED_GEN_2; - advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); - /* Set lane X1 */ reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); reg &= ~LANE_CNT_MSK; @@ -365,20 +436,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) */ msleep(PCI_PM_D3COLD_WAIT);
- /* Enable link training */ - reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); - reg |= LINK_TRAINING_EN; - advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); - - /* - * Start link training immediately after enabling it. - * This solves problems for some buggy cards. - */ - reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); - reg |= PCIE_CORE_LINK_TRAINING; - advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG); - - advk_pcie_wait_for_link(pcie); + advk_pcie_train_link(pcie);
reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG); reg |= PCIE_CORE_CMD_MEM_ACCESS_EN | @@ -1017,6 +1075,12 @@ static int advk_pcie_probe(struct platform_device *pdev) return ret; }
+ ret = of_pci_get_max_link_speed(dev->of_node); + if (ret <= 0 || ret > 3) + pcie->link_gen = 3; + else + pcie->link_gen = ret; + advk_pcie_setup_hw(pcie);
ret = advk_pcie_init_irq_domain(pcie);
From: Pali Rohár pali@kernel.org
commit 5169a9851daaa2782a7bd2bb83d5b1bd224b2879 upstream.
Add support for issuing PERST via GPIO specified in 'reset-gpios' property (as described in PCI device tree bindings).
Some buggy cards (e.g. Compex WLE900VX or WLE1216) are not detected after reboot when PERST is not issued during driver initialization.
If bootloader already enabled link training then issuing PERST has no effect for some buggy cards (e.g. Compex WLE900VX) and these cards are not detected. We therefore clear the LINK_TRAINING_EN register before.
It was observed that Compex WLE900VX card needs to be in PERST reset for at least 10ms if bootloader enabled link training.
Tested on Turris MOX.
Link: https://lore.kernel.org/r/20200430080625.26070-6-pali@kernel.org Tested-by: Tomasz Maciej Nowak tmn505@gmail.com Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Acked-by: Thomas Petazzoni thomas.petazzoni@bootlin.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 44 ++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 714d81462297..0cfec5660c69 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -12,6 +12,7 @@ */
#include <linux/delay.h> +#include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/irq.h> #include <linux/irqdomain.h> @@ -20,6 +21,7 @@ #include <linux/init.h> #include <linux/platform_device.h> #include <linux/of_address.h> +#include <linux/of_gpio.h> #include <linux/of_pci.h>
/* PCIe core registers */ @@ -214,6 +216,7 @@ struct advk_pcie { u16 msi_msg; int root_bus_nr; int link_gen; + struct gpio_desc *reset_gpio; };
static inline void advk_writel(struct advk_pcie *pcie, u32 val, u64 reg) @@ -349,6 +352,25 @@ static void advk_pcie_train_link(struct advk_pcie *pcie) dev_err(dev, "link never came up\n"); }
+static void advk_pcie_issue_perst(struct advk_pcie *pcie) +{ + u32 reg; + + if (!pcie->reset_gpio) + return; + + /* PERST does not work for some cards when link training is enabled */ + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); + reg &= ~LINK_TRAINING_EN; + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); + + /* 10ms delay is needed for some cards */ + dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n"); + gpiod_set_value_cansleep(pcie->reset_gpio, 1); + usleep_range(10000, 11000); + gpiod_set_value_cansleep(pcie->reset_gpio, 0); +} + static void advk_pcie_setup_hw(struct advk_pcie *pcie) { u32 reg; @@ -358,6 +380,8 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) for (i = 0; i < 8; i++) advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);
+ advk_pcie_issue_perst(pcie); + /* Set to Direct mode */ reg = advk_readl(pcie, CTRL_CONFIG_REG); reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT); @@ -430,7 +454,8 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
/* * PERST# signal could have been asserted by pinctrl subsystem before - * probe() callback has been called, making the endpoint going into + * probe() callback has been called or issued explicitly by reset gpio + * function advk_pcie_issue_perst(), making the endpoint going into * fundamental reset. As required by PCI Express spec a delay for at * least 100ms after such a reset before link training is needed. */ @@ -1075,6 +1100,23 @@ static int advk_pcie_probe(struct platform_device *pdev) return ret; }
+ pcie->reset_gpio = devm_fwnode_get_index_gpiod_from_child(dev, "reset", + 0, + dev_fwnode(dev), + GPIOD_OUT_LOW, + "pcie1-reset"); + ret = PTR_ERR_OR_ZERO(pcie->reset_gpio); + if (ret) { + if (ret == -ENOENT) { + pcie->reset_gpio = NULL; + } else { + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get reset-gpio: %i\n", + ret); + return ret; + } + } + ret = of_pci_get_max_link_speed(dev->of_node); if (ret <= 0 || ret > 3) pcie->link_gen = 3;
From: Pali Rohár pali@kernel.org
commit 96be36dbffacea0aa9e6ec4839583e79faa141a1 upstream.
PCI-E capability macros are already defined in linux/pci_regs.h. Remove their reimplementation in pcie-aardvark.
Link: https://lore.kernel.org/r/20200430080625.26070-9-pali@kernel.org Tested-by: Tomasz Maciej Nowak tmn505@gmail.com Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Reviewed-by: Rob Herring robh@kernel.org Acked-by: Thomas Petazzoni thomas.petazzoni@bootlin.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 42 +++++++++++++++------------------ 1 file changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 0cfec5660c69..3ad6b6245f94 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -29,17 +29,7 @@ #define PCIE_CORE_CMD_IO_ACCESS_EN BIT(0) #define PCIE_CORE_CMD_MEM_ACCESS_EN BIT(1) #define PCIE_CORE_CMD_MEM_IO_REQ_EN BIT(2) -#define PCIE_CORE_DEV_CTRL_STATS_REG 0xc8 -#define PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE (0 << 4) -#define PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT 5 -#define PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE (0 << 11) -#define PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT 12 -#define PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SZ 0x2 -#define PCIE_CORE_LINK_CTRL_STAT_REG 0xd0 -#define PCIE_CORE_LINK_L0S_ENTRY BIT(0) -#define PCIE_CORE_LINK_TRAINING BIT(5) -#define PCIE_CORE_LINK_SPEED_SHIFT 16 -#define PCIE_CORE_LINK_WIDTH_SHIFT 20 +#define PCIE_CORE_PCIEXP_CAP 0xc0 #define PCIE_CORE_ERR_CAPCTL_REG 0x118 #define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX BIT(5) #define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN BIT(6) @@ -229,6 +219,11 @@ static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg) return readl(pcie->base + reg); }
+static inline u16 advk_read16(struct advk_pcie *pcie, u64 reg) +{ + return advk_readl(pcie, (reg & ~0x3)) >> ((reg & 0x3) * 8); +} + static int advk_pcie_link_up(struct advk_pcie *pcie) { u32 val, ltssm_state; @@ -301,16 +296,16 @@ static int advk_pcie_train_at_gen(struct advk_pcie *pcie, int gen) * Start link training immediately after enabling it. * This solves problems for some buggy cards. */ - reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); - reg |= PCIE_CORE_LINK_TRAINING; - advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG); + reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL); + reg |= PCI_EXP_LNKCTL_RL; + advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL);
ret = advk_pcie_wait_for_link(pcie); if (ret) return ret;
- reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); - neg_gen = (reg >> PCIE_CORE_LINK_SPEED_SHIFT) & 0xf; + reg = advk_read16(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKSTA); + neg_gen = reg & PCI_EXP_LNKSTA_CLS;
return neg_gen; } @@ -400,13 +395,14 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) PCIE_CORE_ERR_CAPCTL_ECRC_CHCK_RCV; advk_writel(pcie, reg, PCIE_CORE_ERR_CAPCTL_REG);
- /* Set PCIe Device Control and Status 1 PF0 register */ - reg = PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE | - (7 << PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT) | - PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE | - (PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SZ << - PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT); - advk_writel(pcie, reg, PCIE_CORE_DEV_CTRL_STATS_REG); + /* Set PCIe Device Control register */ + reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL); + reg &= ~PCI_EXP_DEVCTL_RELAX_EN; + reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN; + reg &= ~PCI_EXP_DEVCTL_READRQ; + reg |= PCI_EXP_DEVCTL_PAYLOAD; /* Set max payload size */ + reg |= PCI_EXP_DEVCTL_READRQ_512B; + advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
/* Program PCIe Control 2 to disable strict ordering */ reg = PCIE_CORE_CTRL2_RESERVED |
From: Pali Rohár pali@kernel.org
commit b1bd5714472cc72e14409f5659b154c765a76c65 upstream.
Most callers of config read do not check for return value. But most of the ones that do, checks for error indication in 'val' variable.
This patch updates error handling in advk_pcie_rd_conf() function. If PIO transfer fails then 'val' variable is set to 0xffffffff which indicates failture.
Link: https://lore.kernel.org/r/20200528162604.GA323482@bjorn-Precision-5520 Link: https://lore.kernel.org/r/20200601130315.18895-1-pali@kernel.org Reported-by: Bjorn Helgaas helgaas@kernel.org Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 3ad6b6245f94..ae67e5c3fe70 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -631,8 +631,10 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, advk_writel(pcie, 1, PIO_START);
ret = advk_pcie_wait_pio(pcie); - if (ret < 0) + if (ret < 0) { + *val = 0xffffffff; return PCIBIOS_SET_FAILED; + }
/* Check PIO status and get the read result */ ret = advk_pcie_check_pio_status(pcie, val);
From: Thomas Petazzoni thomas.petazzoni@bootlin.com
commit 248d4e59616c632f37f04c233eec6d5008384926 upstream.
In other to mimic other PCIe host controller drivers, introduce an advk_pcie_valid_device() helper, used in the configuration read/write functions.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@bootlin.com [lorenzo.pieralisi@arm.com: updated host->controller dir move] Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index ae67e5c3fe70..9426715316b2 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -592,6 +592,15 @@ static bool advk_pcie_pio_is_running(struct advk_pcie *pcie) return false; }
+static bool advk_pcie_valid_device(struct advk_pcie *pcie, struct pci_bus *bus, + int devfn) +{ + if ((bus->number == pcie->root_bus_nr) && PCI_SLOT(devfn) != 0) + return false; + + return true; +} + static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where, int size, u32 *val) { @@ -599,7 +608,7 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, u32 reg; int ret;
- if ((bus->number == pcie->root_bus_nr) && PCI_SLOT(devfn) != 0) { + if (!advk_pcie_valid_device(pcie, bus, devfn)) { *val = 0xffffffff; return PCIBIOS_DEVICE_NOT_FOUND; } @@ -660,7 +669,7 @@ static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn, int offset; int ret;
- if ((bus->number == pcie->root_bus_nr) && PCI_SLOT(devfn) != 0) + if (!advk_pcie_valid_device(pcie, bus, devfn)) return PCIBIOS_DEVICE_NOT_FOUND;
if (where % size)
From: Pali Rohár pali@kernel.org
commit 70e380250c3621c55ff218cbaf2272830d9dbb1d upstream.
When there is no PCIe card connected and advk_pcie_rd_conf() or advk_pcie_wr_conf() is called for PCI bus which doesn't belong to emulated root bridge, the aardvark driver throws the following error message:
advk-pcie d0070000.pcie: config read/write timed out
Obviously accessing PCIe registers of disconnected card is not possible.
Extend check in advk_pcie_valid_device() function for validating availability of PCIe bus. If PCIe link is down, then the device is marked as Not Found and the driver does not try to access these registers.
This is just an optimization to prevent accessing PCIe registers when card is disconnected. Trying to access PCIe registers of disconnected card does not cause any crash, kernel just needs to wait for a timeout. So if card disappear immediately after checking for PCIe link (before accessing PCIe registers), it does not cause any problems.
Link: https://lore.kernel.org/r/20200702083036.12230-1-pali@kernel.org Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 9426715316b2..acc04c4a37df 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -598,6 +598,13 @@ static bool advk_pcie_valid_device(struct advk_pcie *pcie, struct pci_bus *bus, if ((bus->number == pcie->root_bus_nr) && PCI_SLOT(devfn) != 0) return false;
+ /* + * If the link goes down after we check for link-up, nothing bad + * happens but the config access times out. + */ + if (bus->number != pcie->root_bus_nr && !advk_pcie_link_up(pcie)) + return false; + return true; }
From: Pali Rohár pali@kernel.org
commit b32c012e4b98f0126aa327be2d1f409963057643 upstream.
Include linux/gpio/consumer.h instead of linux/gpio.h, as is said in the latter file.
This was reported by kernel test bot when compiling for s390.
drivers/pci/controller/pci-aardvark.c:350:2: error: implicit declaration of function 'gpiod_set_value_cansleep' [-Werror,-Wimplicit-function-declaration] drivers/pci/controller/pci-aardvark.c:1074:21: error: implicit declaration of function 'devm_gpiod_get_from_of_node' [-Werror,-Wimplicit-function-declaration] drivers/pci/controller/pci-aardvark.c:1076:14: error: use of undeclared identifier 'GPIOD_OUT_LOW'
Link: https://lore.kernel.org/r/202006211118.LxtENQfl%25lkp@intel.com Link: https://lore.kernel.org/r/20200907111038.5811-2-pali@kernel.org Fixes: 5169a9851daa ("PCI: aardvark: Issue PERST via GPIO") Reported-by: kernel test robot lkp@intel.com Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Reviewed-by: Marek Behún marek.behun@nic.cz Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index acc04c4a37df..b3bce506d995 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -12,7 +12,7 @@ */
#include <linux/delay.h> -#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/interrupt.h> #include <linux/irq.h> #include <linux/irqdomain.h>
From: Pali Rohár pali@kernel.org
commit d0c6a3475b033960e85ae2bf176b14cab0a627d2 upstream.
Move code which belongs to link training (delays and resets) into advk_pcie_train_link() function, so everything related to link training, including timings is at one place.
After experiments it can be observed that link training in aardvark hardware is very sensitive to timings and delays, so it is a good idea to have this code at the same place as link training calls.
This patch does not change behavior of aardvark initialization.
Link: https://lore.kernel.org/r/20200907111038.5811-6-pali@kernel.org Tested-by: Marek Behún marek.behun@nic.cz Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 64 +++++++++++++++++---------------- 1 file changed, 34 insertions(+), 30 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index b3bce506d995..68ab3aef0e12 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -268,6 +268,25 @@ static void advk_pcie_set_ob_win(struct advk_pcie *pcie, advk_writel(pcie, match_ls | BIT(0), OB_WIN_MATCH_LS(win_num)); }
+static void advk_pcie_issue_perst(struct advk_pcie *pcie) +{ + u32 reg; + + if (!pcie->reset_gpio) + return; + + /* PERST does not work for some cards when link training is enabled */ + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); + reg &= ~LINK_TRAINING_EN; + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); + + /* 10ms delay is needed for some cards */ + dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n"); + gpiod_set_value_cansleep(pcie->reset_gpio, 1); + usleep_range(10000, 11000); + gpiod_set_value_cansleep(pcie->reset_gpio, 0); +} + static int advk_pcie_train_at_gen(struct advk_pcie *pcie, int gen) { int ret, neg_gen; @@ -315,6 +334,21 @@ static void advk_pcie_train_link(struct advk_pcie *pcie) struct device *dev = &pcie->pdev->dev; int neg_gen = -1, gen;
+ /* + * Reset PCIe card via PERST# signal. Some cards are not detected + * during link training when they are in some non-initial state. + */ + advk_pcie_issue_perst(pcie); + + /* + * PERST# signal could have been asserted by pinctrl subsystem before + * probe() callback has been called or issued explicitly by reset gpio + * function advk_pcie_issue_perst(), making the endpoint going into + * fundamental reset. As required by PCI Express spec a delay for at + * least 100ms after such a reset before link training is needed. + */ + msleep(PCI_PM_D3COLD_WAIT); + /* * Try link training at link gen specified by device tree property * 'max-link-speed'. If this fails, iteratively train at lower gen. @@ -347,25 +381,6 @@ static void advk_pcie_train_link(struct advk_pcie *pcie) dev_err(dev, "link never came up\n"); }
-static void advk_pcie_issue_perst(struct advk_pcie *pcie) -{ - u32 reg; - - if (!pcie->reset_gpio) - return; - - /* PERST does not work for some cards when link training is enabled */ - reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); - reg &= ~LINK_TRAINING_EN; - advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); - - /* 10ms delay is needed for some cards */ - dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n"); - gpiod_set_value_cansleep(pcie->reset_gpio, 1); - usleep_range(10000, 11000); - gpiod_set_value_cansleep(pcie->reset_gpio, 0); -} - static void advk_pcie_setup_hw(struct advk_pcie *pcie) { u32 reg; @@ -375,8 +390,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) for (i = 0; i < 8; i++) advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);
- advk_pcie_issue_perst(pcie); - /* Set to Direct mode */ reg = advk_readl(pcie, CTRL_CONFIG_REG); reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT); @@ -448,15 +461,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg |= PIO_CTRL_ADDR_WIN_DISABLE; advk_writel(pcie, reg, PIO_CTRL);
- /* - * PERST# signal could have been asserted by pinctrl subsystem before - * probe() callback has been called or issued explicitly by reset gpio - * function advk_pcie_issue_perst(), making the endpoint going into - * fundamental reset. As required by PCI Express spec a delay for at - * least 100ms after such a reset before link training is needed. - */ - msleep(PCI_PM_D3COLD_WAIT); - advk_pcie_train_link(pcie);
reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
From: Pali Rohár pali@kernel.org
commit 1d1cd163d0de22a4041a6f1aeabcf78f80076539 upstream.
According to PCI Express Base Specifications (rev 4.0, 6.6.1 "Conventional reset"), after fundamental reset a 100ms delay is needed prior to enabling link training.
Update comment in code to reflect this requirement.
Link: https://lore.kernel.org/r/20201202184659.3795-1-pali@kernel.org Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 68ab3aef0e12..66c4b9282420 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -275,7 +275,14 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie) if (!pcie->reset_gpio) return;
- /* PERST does not work for some cards when link training is enabled */ + /* + * As required by PCI Express spec (PCI Express Base Specification, REV. + * 4.0 PCI Express, February 19 2014, 6.6.1 Conventional Reset) a delay + * for at least 100ms after de-asserting PERST# signal is needed before + * link training is enabled. So ensure that link training is disabled + * prior de-asserting PERST# signal to fulfill that PCI Express spec + * requirement. + */ reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); reg &= ~LINK_TRAINING_EN; advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
From: Evan Wang xswang@marvell.com
commit 6df6ba974a55678a2c7d9a0c06eb15cde0c4b184 upstream.
Outbound window is used to translate CPU space addresses to PCIe space addresses when the CPU initiates PCIe transactions.
According to the suggestion of the HW designers, the recommended solution is to use the default outbound parameters, even though the current outbound window setting does not cause any known functional issue.
This patch doesn't address any known functional issue, but aligns to HW design guidelines, and removes code that isn't needed.
Signed-off-by: Evan Wang xswang@marvell.com [Thomas: tweak commit log.] Signed-off-by: Thomas Petazzoni thomas.petazzoni@bootlin.com [lorenzo.pieralisi@arm.com: handled host->controller dir move] Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Reviewed-by: Victor Gu xigu@marvell.com Reviewed-by: Nadav Haklai nadavh@marvell.com Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 55 --------------------------------- 1 file changed, 55 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 66c4b9282420..4021c4d1c6e8 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -107,24 +107,6 @@ #define PCIE_MSI_PAYLOAD_REG (CONTROL_BASE_ADDR + 0x9C) #define PCIE_MSI_DATA_MASK GENMASK(15, 0)
-/* PCIe window configuration */ -#define OB_WIN_BASE_ADDR 0x4c00 -#define OB_WIN_BLOCK_SIZE 0x20 -#define OB_WIN_REG_ADDR(win, offset) (OB_WIN_BASE_ADDR + \ - OB_WIN_BLOCK_SIZE * (win) + \ - (offset)) -#define OB_WIN_MATCH_LS(win) OB_WIN_REG_ADDR(win, 0x00) -#define OB_WIN_MATCH_MS(win) OB_WIN_REG_ADDR(win, 0x04) -#define OB_WIN_REMAP_LS(win) OB_WIN_REG_ADDR(win, 0x08) -#define OB_WIN_REMAP_MS(win) OB_WIN_REG_ADDR(win, 0x0c) -#define OB_WIN_MASK_LS(win) OB_WIN_REG_ADDR(win, 0x10) -#define OB_WIN_MASK_MS(win) OB_WIN_REG_ADDR(win, 0x14) -#define OB_WIN_ACTIONS(win) OB_WIN_REG_ADDR(win, 0x18) - -/* PCIe window types */ -#define OB_PCIE_MEM 0x0 -#define OB_PCIE_IO 0x4 - /* LMI registers base address and register offsets */ #define LMI_BASE_ADDR 0x6000 #define CFG_REG (LMI_BASE_ADDR + 0x0) @@ -248,26 +230,6 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie) return -ETIMEDOUT; }
-/* - * Set PCIe address window register which could be used for memory - * mapping. - */ -static void advk_pcie_set_ob_win(struct advk_pcie *pcie, - u32 win_num, u32 match_ms, - u32 match_ls, u32 mask_ms, - u32 mask_ls, u32 remap_ms, - u32 remap_ls, u32 action) -{ - advk_writel(pcie, match_ls, OB_WIN_MATCH_LS(win_num)); - advk_writel(pcie, match_ms, OB_WIN_MATCH_MS(win_num)); - advk_writel(pcie, mask_ms, OB_WIN_MASK_MS(win_num)); - advk_writel(pcie, mask_ls, OB_WIN_MASK_LS(win_num)); - advk_writel(pcie, remap_ms, OB_WIN_REMAP_MS(win_num)); - advk_writel(pcie, remap_ls, OB_WIN_REMAP_LS(win_num)); - advk_writel(pcie, action, OB_WIN_ACTIONS(win_num)); - advk_writel(pcie, match_ls | BIT(0), OB_WIN_MATCH_LS(win_num)); -} - static void advk_pcie_issue_perst(struct advk_pcie *pcie) { u32 reg; @@ -391,11 +353,6 @@ static void advk_pcie_train_link(struct advk_pcie *pcie) static void advk_pcie_setup_hw(struct advk_pcie *pcie) { u32 reg; - int i; - - /* Point PCIe unit MBUS decode windows to DRAM space */ - for (i = 0; i < 8; i++) - advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);
/* Set to Direct mode */ reg = advk_readl(pcie, CTRL_CONFIG_REG); @@ -1048,12 +1005,6 @@ static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie)
switch (resource_type(res)) { case IORESOURCE_IO: - advk_pcie_set_ob_win(pcie, 1, - upper_32_bits(res->start), - lower_32_bits(res->start), - 0, 0xF8000000, 0, - lower_32_bits(res->start), - OB_PCIE_IO); err = devm_pci_remap_iospace(dev, res, iobase); if (err) { dev_warn(dev, "error %d: failed to map resource %pR\n", @@ -1062,12 +1013,6 @@ static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie) } break; case IORESOURCE_MEM: - advk_pcie_set_ob_win(pcie, 0, - upper_32_bits(res->start), - lower_32_bits(res->start), - 0x0, 0xF8000000, 0, - lower_32_bits(res->start), - (2 << 20) | OB_PCIE_MEM); res_valid |= !(res->flags & IORESOURCE_PREFETCH); break; case IORESOURCE_BUS:
From: Pali Rohár pali@kernel.org
commit 64f160e19e9264a7f6d89c516baae1473b6f8359 upstream.
In commit 6df6ba974a55 ("PCI: aardvark: Remove PCIe outbound window configuration") was removed aardvark PCIe outbound window configuration and commit description said that was recommended solution by HW designers.
But that commit completely removed support for configuring PCIe IO resources without removing PCIe IO 'ranges' from DTS files. After that commit PCIe IO space started to be treated as PCIe MEM space and accessing it just caused kernel crash.
Moreover implementation of PCIe outbound windows prior that commit was incorrect. It completely ignored offset between CPU address and PCIe bus address and expected that in DTS is CPU address always same as PCIe bus address without doing any checks. Also it completely ignored size of every PCIe resource specified in 'ranges' DTS property and expected that every PCIe resource has size 128 MB (also for PCIe IO range). Again without any check. Apparently none of PCIe resource has in DTS specified size of 128 MB. So it was completely broken and thanks to how aardvark mask works, configuration was completely ignored.
This patch reverts back support for PCIe outbound window configuration but implementation is a new without issues mentioned above. PCIe outbound window is required when DTS specify in 'ranges' property non-zero offset between CPU and PCIe address space. To address recommendation by HW designers as specified in commit description of 6df6ba974a55, set default outbound parameters as PCIe MEM access without translation and therefore for this PCIe 'ranges' it is not needed to configure PCIe outbound window. For PCIe IO space is needed to configure aardvark PCIe outbound window.
This patch fixes kernel crash when trying to access PCIe IO space.
Link: https://lore.kernel.org/r/20210624215546.4015-2-pali@kernel.org Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Cc: stable@vger.kernel.org # 6df6ba974a55 ("PCI: aardvark: Remove PCIe outbound window configuration") Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 190 +++++++++++++++++++++++++++++++- 1 file changed, 189 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 4021c4d1c6e8..fd5155b10c1d 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -107,6 +107,46 @@ #define PCIE_MSI_PAYLOAD_REG (CONTROL_BASE_ADDR + 0x9C) #define PCIE_MSI_DATA_MASK GENMASK(15, 0)
+/* PCIe window configuration */ +#define OB_WIN_BASE_ADDR 0x4c00 +#define OB_WIN_BLOCK_SIZE 0x20 +#define OB_WIN_COUNT 8 +#define OB_WIN_REG_ADDR(win, offset) (OB_WIN_BASE_ADDR + \ + OB_WIN_BLOCK_SIZE * (win) + \ + (offset)) +#define OB_WIN_MATCH_LS(win) OB_WIN_REG_ADDR(win, 0x00) +#define OB_WIN_ENABLE BIT(0) +#define OB_WIN_MATCH_MS(win) OB_WIN_REG_ADDR(win, 0x04) +#define OB_WIN_REMAP_LS(win) OB_WIN_REG_ADDR(win, 0x08) +#define OB_WIN_REMAP_MS(win) OB_WIN_REG_ADDR(win, 0x0c) +#define OB_WIN_MASK_LS(win) OB_WIN_REG_ADDR(win, 0x10) +#define OB_WIN_MASK_MS(win) OB_WIN_REG_ADDR(win, 0x14) +#define OB_WIN_ACTIONS(win) OB_WIN_REG_ADDR(win, 0x18) +#define OB_WIN_DEFAULT_ACTIONS (OB_WIN_ACTIONS(OB_WIN_COUNT-1) + 0x4) +#define OB_WIN_FUNC_NUM_MASK GENMASK(31, 24) +#define OB_WIN_FUNC_NUM_SHIFT 24 +#define OB_WIN_FUNC_NUM_ENABLE BIT(23) +#define OB_WIN_BUS_NUM_BITS_MASK GENMASK(22, 20) +#define OB_WIN_BUS_NUM_BITS_SHIFT 20 +#define OB_WIN_MSG_CODE_ENABLE BIT(22) +#define OB_WIN_MSG_CODE_MASK GENMASK(21, 14) +#define OB_WIN_MSG_CODE_SHIFT 14 +#define OB_WIN_MSG_PAYLOAD_LEN BIT(12) +#define OB_WIN_ATTR_ENABLE BIT(11) +#define OB_WIN_ATTR_TC_MASK GENMASK(10, 8) +#define OB_WIN_ATTR_TC_SHIFT 8 +#define OB_WIN_ATTR_RELAXED BIT(7) +#define OB_WIN_ATTR_NOSNOOP BIT(6) +#define OB_WIN_ATTR_POISON BIT(5) +#define OB_WIN_ATTR_IDO BIT(4) +#define OB_WIN_TYPE_MASK GENMASK(3, 0) +#define OB_WIN_TYPE_SHIFT 0 +#define OB_WIN_TYPE_MEM 0x0 +#define OB_WIN_TYPE_IO 0x4 +#define OB_WIN_TYPE_CONFIG_TYPE0 0x8 +#define OB_WIN_TYPE_CONFIG_TYPE1 0x9 +#define OB_WIN_TYPE_MSG 0xc + /* LMI registers base address and register offsets */ #define LMI_BASE_ADDR 0x6000 #define CFG_REG (LMI_BASE_ADDR + 0x0) @@ -175,6 +215,13 @@ struct advk_pcie { struct platform_device *pdev; void __iomem *base; struct list_head resources; + struct { + phys_addr_t match; + phys_addr_t remap; + phys_addr_t mask; + u32 actions; + } wins[OB_WIN_COUNT]; + u8 wins_count; struct irq_domain *irq_domain; struct irq_chip irq_chip; raw_spinlock_t irq_lock; @@ -350,9 +397,39 @@ static void advk_pcie_train_link(struct advk_pcie *pcie) dev_err(dev, "link never came up\n"); }
+/* + * Set PCIe address window register which could be used for memory + * mapping. + */ +static void advk_pcie_set_ob_win(struct advk_pcie *pcie, u8 win_num, + phys_addr_t match, phys_addr_t remap, + phys_addr_t mask, u32 actions) +{ + advk_writel(pcie, OB_WIN_ENABLE | + lower_32_bits(match), OB_WIN_MATCH_LS(win_num)); + advk_writel(pcie, upper_32_bits(match), OB_WIN_MATCH_MS(win_num)); + advk_writel(pcie, lower_32_bits(remap), OB_WIN_REMAP_LS(win_num)); + advk_writel(pcie, upper_32_bits(remap), OB_WIN_REMAP_MS(win_num)); + advk_writel(pcie, lower_32_bits(mask), OB_WIN_MASK_LS(win_num)); + advk_writel(pcie, upper_32_bits(mask), OB_WIN_MASK_MS(win_num)); + advk_writel(pcie, actions, OB_WIN_ACTIONS(win_num)); +} + +static void advk_pcie_disable_ob_win(struct advk_pcie *pcie, u8 win_num) +{ + advk_writel(pcie, 0, OB_WIN_MATCH_LS(win_num)); + advk_writel(pcie, 0, OB_WIN_MATCH_MS(win_num)); + advk_writel(pcie, 0, OB_WIN_REMAP_LS(win_num)); + advk_writel(pcie, 0, OB_WIN_REMAP_MS(win_num)); + advk_writel(pcie, 0, OB_WIN_MASK_LS(win_num)); + advk_writel(pcie, 0, OB_WIN_MASK_MS(win_num)); + advk_writel(pcie, 0, OB_WIN_ACTIONS(win_num)); +} + static void advk_pcie_setup_hw(struct advk_pcie *pcie) { u32 reg; + int i;
/* Set to Direct mode */ reg = advk_readl(pcie, CTRL_CONFIG_REG); @@ -416,15 +493,51 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg = PCIE_IRQ_ALL_MASK & (~PCIE_IRQ_ENABLE_INTS_MASK); advk_writel(pcie, reg, HOST_CTRL_INT_MASK_REG);
+ /* + * Enable AXI address window location generation: + * When it is enabled, the default outbound window + * configurations (Default User Field: 0xD0074CFC) + * are used to transparent address translation for + * the outbound transactions. Thus, PCIe address + * windows are not required for transparent memory + * access when default outbound window configuration + * is set for memory access. + */ reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG); reg |= PCIE_CORE_CTRL2_OB_WIN_ENABLE; advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
- /* Bypass the address window mapping for PIO */ + /* + * Set memory access in Default User Field so it + * is not required to configure PCIe address for + * transparent memory access. + */ + advk_writel(pcie, OB_WIN_TYPE_MEM, OB_WIN_DEFAULT_ACTIONS); + + /* + * Bypass the address window mapping for PIO: + * Since PIO access already contains all required + * info over AXI interface by PIO registers, the + * address window is not required. + */ reg = advk_readl(pcie, PIO_CTRL); reg |= PIO_CTRL_ADDR_WIN_DISABLE; advk_writel(pcie, reg, PIO_CTRL);
+ /* + * Configure PCIe address windows for non-memory or + * non-transparent access as by default PCIe uses + * transparent memory access. + */ + for (i = 0; i < pcie->wins_count; i++) + advk_pcie_set_ob_win(pcie, i, + pcie->wins[i].match, pcie->wins[i].remap, + pcie->wins[i].mask, pcie->wins[i].actions); + + /* Disable remaining PCIe outbound windows */ + for (i = pcie->wins_count; i < OB_WIN_COUNT; i++) + advk_pcie_disable_ob_win(pcie, i); + advk_pcie_train_link(pcie);
reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG); @@ -1041,6 +1154,7 @@ static int advk_pcie_probe(struct platform_device *pdev) struct resource *res; struct pci_bus *bus, *child; struct pci_host_bridge *bridge; + struct resource_entry *entry; int ret, irq;
bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct advk_pcie)); @@ -1070,6 +1184,80 @@ static int advk_pcie_probe(struct platform_device *pdev) return ret; }
+ resource_list_for_each_entry(entry, &pcie->resources) { + resource_size_t start = entry->res->start; + resource_size_t size = resource_size(entry->res); + unsigned long type = resource_type(entry->res); + u64 win_size; + + /* + * Aardvark hardware allows to configure also PCIe window + * for config type 0 and type 1 mapping, but driver uses + * only PIO for issuing configuration transfers which does + * not use PCIe window configuration. + */ + if (type != IORESOURCE_MEM && type != IORESOURCE_MEM_64 && + type != IORESOURCE_IO) + continue; + + /* + * Skip transparent memory resources. Default outbound access + * configuration is set to transparent memory access so it + * does not need window configuration. + */ + if ((type == IORESOURCE_MEM || type == IORESOURCE_MEM_64) && + entry->offset == 0) + continue; + + /* + * The n-th PCIe window is configured by tuple (match, remap, mask) + * and an access to address A uses this window if A matches the + * match with given mask. + * So every PCIe window size must be a power of two and every start + * address must be aligned to window size. Minimal size is 64 KiB + * because lower 16 bits of mask must be zero. Remapped address + * may have set only bits from the mask. + */ + while (pcie->wins_count < OB_WIN_COUNT && size > 0) { + /* Calculate the largest aligned window size */ + win_size = (1ULL << (fls64(size)-1)) | + (start ? (1ULL << __ffs64(start)) : 0); + win_size = 1ULL << __ffs64(win_size); + if (win_size < 0x10000) + break; + + dev_dbg(dev, + "Configuring PCIe window %d: [0x%llx-0x%llx] as %lu\n", + pcie->wins_count, (unsigned long long)start, + (unsigned long long)start + win_size, type); + + if (type == IORESOURCE_IO) { + pcie->wins[pcie->wins_count].actions = OB_WIN_TYPE_IO; + pcie->wins[pcie->wins_count].match = pci_pio_to_address(start); + } else { + pcie->wins[pcie->wins_count].actions = OB_WIN_TYPE_MEM; + pcie->wins[pcie->wins_count].match = start; + } + pcie->wins[pcie->wins_count].remap = start - entry->offset; + pcie->wins[pcie->wins_count].mask = ~(win_size - 1); + + if (pcie->wins[pcie->wins_count].remap & (win_size - 1)) + break; + + start += win_size; + size -= win_size; + pcie->wins_count++; + } + + if (size > 0) { + dev_err(&pcie->pdev->dev, + "Invalid PCIe region [0x%llx-0x%llx]\n", + (unsigned long long)entry->res->start, + (unsigned long long)entry->res->end + 1); + return -EINVAL; + } + } + pcie->reset_gpio = devm_fwnode_get_index_gpiod_from_child(dev, "reset", 0, dev_fwnode(dev),
From: Pali Rohár pali@kernel.org
commit a4e17d65dafdd3513042d8f00404c9b6068a825c upstream.
Change PCIe Max Payload Size setting in PCIe Device Control register to 512 bytes to align with PCIe Link Initialization sequence as defined in Marvell Armada 3700 Functional Specification. According to the specification, maximal Max Payload Size supported by this device is 512 bytes.
Without this kernel prints suspicious line:
pci 0000:01:00.0: Upstream bridge's Max Payload Size set to 256 (was 16384, max 512)
With this change it changes to:
pci 0000:01:00.0: Upstream bridge's Max Payload Size set to 256 (was 512, max 512)
Link: https://lore.kernel.org/r/20211005180952.6812-3-kabel@kernel.org Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller driver") Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Reviewed-by: Marek Behún kabel@kernel.org Cc: stable@vger.kernel.org Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index fd5155b10c1d..fc198e6e7fa4 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -453,8 +453,9 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL); reg &= ~PCI_EXP_DEVCTL_RELAX_EN; reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN; + reg &= ~PCI_EXP_DEVCTL_PAYLOAD; reg &= ~PCI_EXP_DEVCTL_READRQ; - reg |= PCI_EXP_DEVCTL_PAYLOAD; /* Set max payload size */ + reg |= PCI_EXP_DEVCTL_PAYLOAD_512B; reg |= PCI_EXP_DEVCTL_READRQ_512B; advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
From: Frederick Lawler fred@fredlawl.com
commit c80851f6ce63a6e313f8c7b4b6eb82c67aa4497b upstream.
The Link Control 2 register is missing macros for Target Link Speeds. Add those in.
Signed-off-by: Frederick Lawler fred@fredlawl.com [bhelgaas: use "GT" instead of "GB"] Signed-off-by: Bjorn Helgaas bhelgaas@google.com Signed-off-by: Marek Behún kabel@kernel.org --- include/uapi/linux/pci_regs.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index c843929e0c4f..5db024aa00e9 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -654,6 +654,11 @@ #define PCI_EXP_LNKCAP2_SLS_8_0GB 0x00000008 /* Supported Speed 8.0GT/s */ #define PCI_EXP_LNKCAP2_CROSSLINK 0x00000100 /* Crosslink supported */ #define PCI_EXP_LNKCTL2 48 /* Link Control 2 */ +#define PCI_EXP_LNKCTL2_TLS 0x000f +#define PCI_EXP_LNKCTL2_TLS_2_5GT 0x0001 /* Supported Speed 2.5GT/s */ +#define PCI_EXP_LNKCTL2_TLS_5_0GT 0x0002 /* Supported Speed 5GT/s */ +#define PCI_EXP_LNKCTL2_TLS_8_0GT 0x0003 /* Supported Speed 8GT/s */ +#define PCI_EXP_LNKCTL2_TLS_16_0GT 0x0004 /* Supported Speed 16GT/s */ #define PCI_EXP_LNKSTA2 50 /* Link Status 2 */ #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 52 /* v2 endpoints with link end here */ #define PCI_EXP_SLTCAP2 52 /* Slot Capabilities 2 */
From: Pali Rohár pali@kernel.org
commit f76b36d40beee0a13aa8f6aa011df0d7cbbb8a7f upstream.
Fix multiple link training issues in aardvark driver. The main reason of these issues was misunderstanding of what certain registers do, since their names and comments were misleading: before commit 96be36dbffac ("PCI: aardvark: Replace custom macros by standard linux/pci_regs.h macros"), the pci-aardvark.c driver used custom macros for accessing standard PCIe Root Bridge registers, and misleading comments did not help to understand what the code was really doing.
After doing more tests and experiments I've come to the conclusion that the SPEED_GEN register in aardvark sets the PCIe revision / generation compliance and forces maximal link speed. Both GEN3 and GEN2 values set the read-only PCI_EXP_FLAGS_VERS bits (PCIe capabilities version of Root Bridge) to value 2, while GEN1 value sets PCI_EXP_FLAGS_VERS to 1, which matches with PCI Express specifications revisions 3, 2 and 1 respectively. Changing SPEED_GEN also sets the read-only bits PCI_EXP_LNKCAP_SLS and PCI_EXP_LNKCAP2_SLS to corresponding speed.
(Note that PCI Express rev 1 specification does not define PCI_EXP_LNKCAP2 and PCI_EXP_LNKCTL2 registers and when SPEED_GEN is set to GEN1 (which also sets PCI_EXP_FLAGS_VERS set to 1), lspci cannot access PCI_EXP_LNKCAP2 and PCI_EXP_LNKCTL2 registers.)
Changing PCIe link speed can be done via PCI_EXP_LNKCTL2_TLS bits of PCI_EXP_LNKCTL2 register. Armada 3700 Functional Specifications says that the default value of PCI_EXP_LNKCTL2_TLS is based on SPEED_GEN value, but tests showed that the default value is always 8.0 GT/s, independently of speed set by SPEED_GEN. So after setting SPEED_GEN, we must also set value in PCI_EXP_LNKCTL2 register via PCI_EXP_LNKCTL2_TLS bits.
Triggering PCI_EXP_LNKCTL_RL bit immediately after setting LINK_TRAINING_EN bit actually doesn't do anything. Tests have shown that a delay is needed after enabling LINK_TRAINING_EN bit. As triggering PCI_EXP_LNKCTL_RL currently does nothing, remove it.
Commit 43fc679ced18 ("PCI: aardvark: Improve link training") introduced code which sets SPEED_GEN register based on negotiated link speed from PCI_EXP_LNKSTA_CLS bits of PCI_EXP_LNKSTA register. This code was added to fix detection of Compex WLE900VX (Atheros QCA9880) WiFi GEN1 PCIe cards, as otherwise these cards were "invisible" on PCIe bus (probably because they crashed). But apparently more people reported the same issues with these cards also with other PCIe controllers [1] and I was able to reproduce this issue also with other "noname" WiFi cards based on Atheros QCA9890 chip (with the same PCI vendor/device ids as Atheros QCA9880). So this is not an issue in aardvark but rather an issue in Atheros QCA98xx chips. Also, this issue only exists if the kernel is compiled with PCIe ASPM support, and a generic workaround for this is to change PCIe Bridge to 2.5 GT/s link speed via PCI_EXP_LNKCTL2_TLS_2_5GT bits in PCI_EXP_LNKCTL2 register [2], before triggering PCI_EXP_LNKCTL_RL bit. This workaround also works when SPEED_GEN is set to value GEN2 (5 GT/s). So remove this hack completely in the aardvark driver and always set SPEED_GEN to value from 'max-link-speed' DT property. Fix for Atheros QCA98xx chips is handled separately by patch [2].
These two things (code for triggering PCI_EXP_LNKCTL_RL bit and changing SPEED_GEN value) also explain why commit 6964494582f5 ("PCI: aardvark: Train link immediately after enabling training") somehow fixed detection of those problematic Compex cards with Atheros chips: if triggering link retraining (via PCI_EXP_LNKCTL_RL bit) was done immediately after enabling link training (via LINK_TRAINING_EN), it did nothing. If there was a specific delay, aardvark HW already initialized PCIe link and therefore triggering link retraining caused the above issue. Compex cards triggered link down event and disappeared from the PCIe bus.
Commit f4c7d053d7f7 ("PCI: aardvark: Wait for endpoint to be ready before training link") added 100ms sleep before calling 'Start link training' command and explained that it is a requirement of PCI Express specification. But the code after this 100ms sleep was not doing 'Start link training', rather it triggered PCI_EXP_LNKCTL_RL bit via PCIe Root Bridge to put link into Recovery state.
The required delay after fundamental reset is already done in function advk_pcie_wait_for_link() which also checks whether PCIe link is up. So after removing the code which triggers PCI_EXP_LNKCTL_RL bit on PCIe Root Bridge, there is no need to wait 100ms again. Remove the extra msleep() call and update comment about the delay required by the PCI Express specification.
According to Marvell Armada 3700 Functional Specifications, Link training should be enabled via aardvark register LINK_TRAINING_EN after selecting PCIe generation and x1 lane. There is no need to disable it prior resetting card via PERST# signal. This disabling code was introduced in commit 5169a9851daa ("PCI: aardvark: Issue PERST via GPIO") as a workaround for some Atheros cards. It turns out that this also is Atheros specific issue and affects any PCIe controller, not only aardvark. Moreover this Atheros issue was triggered by juggling with PCI_EXP_LNKCTL_RL, LINK_TRAINING_EN and SPEED_GEN bits interleaved with sleeps. Now, after removing triggering PCI_EXP_LNKCTL_RL, there is no need to explicitly disable LINK_TRAINING_EN bit. So remove this code too. The problematic Compex cards described in previous git commits are correctly detected in advk_pcie_train_link() function even after applying all these changes.
Note that with this patch, and also prior this patch, some NVMe disks which support PCIe GEN3 with 8 GT/s speed are negotiated only at the lowest link speed 2.5 GT/s, independently of SPEED_GEN value. After manually triggering PCI_EXP_LNKCTL_RL bit (e.g. from userspace via setpci), these NVMe disks change link speed to 5 GT/s when SPEED_GEN was configured to GEN2. This issue first needs to be properly investigated. I will send a fix in the future.
On the other hand, some other GEN2 PCIe cards with 5 GT/s speed are autonomously by HW autonegotiated at full 5 GT/s speed without need of any software interaction.
Armada 3700 Functional Specifications describes the following steps for link training: set SPEED_GEN to GEN2, enable LINK_TRAINING_EN, poll until link training is complete, trigger PCI_EXP_LNKCTL_RL, poll until signal rate is 5 GT/s, poll until link training is complete, enable ASPM L0s.
The requirement for triggering PCI_EXP_LNKCTL_RL can be explained by the need to achieve 5 GT/s speed (as changing link speed is done by throw to recovery state entered by PCI_EXP_LNKCTL_RL) or maybe as a part of enabling ASPM L0s (but in this case ASPM L0s should have been enabled prior PCI_EXP_LNKCTL_RL).
It is unknown why the original pci-aardvark.c driver was triggering PCI_EXP_LNKCTL_RL bit before waiting for the link to be up. This does not align with neither PCIe base specifications nor with Armada 3700 Functional Specification. (Note that in older versions of aardvark, this bit was called incorrectly PCIE_CORE_LINK_TRAINING, so this may be the reason.)
It is also unknown why Armada 3700 Functional Specification says that it is needed to trigger PCI_EXP_LNKCTL_RL for GEN2 mode, as according to PCIe base specification 5 GT/s speed negotiation is supposed to be entirely autonomous, even if initial speed is 2.5 GT/s.
[1] - https://lore.kernel.org/linux-pci/87h7l8axqp.fsf@toke.dk/ [2] - https://lore.kernel.org/linux-pci/20210326124326.21163-1-pali@kernel.org/
Link: https://lore.kernel.org/r/20211005180952.6812-12-kabel@kernel.org Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Reviewed-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 117 ++++++++++---------------------- 1 file changed, 34 insertions(+), 83 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index fc198e6e7fa4..7c3369726c87 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -248,11 +248,6 @@ static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg) return readl(pcie->base + reg); }
-static inline u16 advk_read16(struct advk_pcie *pcie, u64 reg) -{ - return advk_readl(pcie, (reg & ~0x3)) >> ((reg & 0x3) * 8); -} - static int advk_pcie_link_up(struct advk_pcie *pcie) { u32 val, ltssm_state; @@ -279,23 +274,9 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
static void advk_pcie_issue_perst(struct advk_pcie *pcie) { - u32 reg; - if (!pcie->reset_gpio) return;
- /* - * As required by PCI Express spec (PCI Express Base Specification, REV. - * 4.0 PCI Express, February 19 2014, 6.6.1 Conventional Reset) a delay - * for at least 100ms after de-asserting PERST# signal is needed before - * link training is enabled. So ensure that link training is disabled - * prior de-asserting PERST# signal to fulfill that PCI Express spec - * requirement. - */ - reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); - reg &= ~LINK_TRAINING_EN; - advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); - /* 10ms delay is needed for some cards */ dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n"); gpiod_set_value_cansleep(pcie->reset_gpio, 1); @@ -303,53 +284,46 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie) gpiod_set_value_cansleep(pcie->reset_gpio, 0); }
-static int advk_pcie_train_at_gen(struct advk_pcie *pcie, int gen) +static void advk_pcie_train_link(struct advk_pcie *pcie) { - int ret, neg_gen; + struct device *dev = &pcie->pdev->dev; u32 reg; + int ret;
- /* Setup link speed */ + /* + * Setup PCIe rev / gen compliance based on device tree property + * 'max-link-speed' which also forces maximal link speed. + */ reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); reg &= ~PCIE_GEN_SEL_MSK; - if (gen == 3) + if (pcie->link_gen == 3) reg |= SPEED_GEN_3; - else if (gen == 2) + else if (pcie->link_gen == 2) reg |= SPEED_GEN_2; else reg |= SPEED_GEN_1; advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
/* - * Enable link training. This is not needed in every call to this - * function, just once suffices, but it does not break anything either. + * Set maximal link speed value also into PCIe Link Control 2 register. + * Armada 3700 Functional Specification says that default value is based + * on SPEED_GEN but tests showed that default value is always 8.0 GT/s. */ + reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2); + reg &= ~PCI_EXP_LNKCTL2_TLS; + if (pcie->link_gen == 3) + reg |= PCI_EXP_LNKCTL2_TLS_8_0GT; + else if (pcie->link_gen == 2) + reg |= PCI_EXP_LNKCTL2_TLS_5_0GT; + else + reg |= PCI_EXP_LNKCTL2_TLS_2_5GT; + advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2); + + /* Enable link training after selecting PCIe generation */ reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); reg |= LINK_TRAINING_EN; advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
- /* - * Start link training immediately after enabling it. - * This solves problems for some buggy cards. - */ - reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL); - reg |= PCI_EXP_LNKCTL_RL; - advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL); - - ret = advk_pcie_wait_for_link(pcie); - if (ret) - return ret; - - reg = advk_read16(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKSTA); - neg_gen = reg & PCI_EXP_LNKSTA_CLS; - - return neg_gen; -} - -static void advk_pcie_train_link(struct advk_pcie *pcie) -{ - struct device *dev = &pcie->pdev->dev; - int neg_gen = -1, gen; - /* * Reset PCIe card via PERST# signal. Some cards are not detected * during link training when they are in some non-initial state. @@ -360,41 +334,18 @@ static void advk_pcie_train_link(struct advk_pcie *pcie) * PERST# signal could have been asserted by pinctrl subsystem before * probe() callback has been called or issued explicitly by reset gpio * function advk_pcie_issue_perst(), making the endpoint going into - * fundamental reset. As required by PCI Express spec a delay for at - * least 100ms after such a reset before link training is needed. + * fundamental reset. As required by PCI Express spec (PCI Express + * Base Specification, REV. 4.0 PCI Express, February 19 2014, 6.6.1 + * Conventional Reset) a delay for at least 100ms after such a reset + * before sending a Configuration Request to the device is needed. + * So wait until PCIe link is up. Function advk_pcie_wait_for_link() + * waits for link at least 900ms. */ - msleep(PCI_PM_D3COLD_WAIT); - - /* - * Try link training at link gen specified by device tree property - * 'max-link-speed'. If this fails, iteratively train at lower gen. - */ - for (gen = pcie->link_gen; gen > 0; --gen) { - neg_gen = advk_pcie_train_at_gen(pcie, gen); - if (neg_gen > 0) - break; - } - - if (neg_gen < 0) - goto err; - - /* - * After successful training if negotiated gen is lower than requested, - * train again on negotiated gen. This solves some stability issues for - * some buggy gen1 cards. - */ - if (neg_gen < gen) { - gen = neg_gen; - neg_gen = advk_pcie_train_at_gen(pcie, gen); - } - - if (neg_gen == gen) { - dev_info(dev, "link up at gen %i\n", gen); - return; - } - -err: - dev_err(dev, "link never came up\n"); + ret = advk_pcie_wait_for_link(pcie); + if (ret < 0) + dev_err(dev, "link never came up\n"); + else + dev_info(dev, "link up\n"); }
/*
From: Pali Rohár pali@kernel.org
commit 661c399a651c11aaf83c45cbfe0b4a1fb7bc3179 upstream.
Current implementation of advk_pcie_link_up() is wrong as it marks also link disabled or hot reset states as link up.
Fix it by marking link up only to those states which are defined in PCIe Base specification 3.0, Table 4-14: Link Status Mapped to the LTSSM.
To simplify implementation, Define macros for every LTSSM state which aardvark hardware can return in CFG_REG register.
Fix also checking for link training according to the same Table 4-14. Define a new function advk_pcie_link_training() for this purpose.
Link: https://lore.kernel.org/r/20211005180952.6812-13-kabel@kernel.org Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller driver") Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Reviewed-by: Marek Behún kabel@kernel.org Cc: stable@vger.kernel.org Cc: Remi Pommarel repk@triplefau.lt Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pci/host/pci-aardvark.c | 71 +++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c index 7c3369726c87..9ae544e113dc 100644 --- a/drivers/pci/host/pci-aardvark.c +++ b/drivers/pci/host/pci-aardvark.c @@ -152,9 +152,50 @@ #define CFG_REG (LMI_BASE_ADDR + 0x0) #define LTSSM_SHIFT 24 #define LTSSM_MASK 0x3f -#define LTSSM_L0 0x10 #define RC_BAR_CONFIG 0x300
+/* LTSSM values in CFG_REG */ +enum { + LTSSM_DETECT_QUIET = 0x0, + LTSSM_DETECT_ACTIVE = 0x1, + LTSSM_POLLING_ACTIVE = 0x2, + LTSSM_POLLING_COMPLIANCE = 0x3, + LTSSM_POLLING_CONFIGURATION = 0x4, + LTSSM_CONFIG_LINKWIDTH_START = 0x5, + LTSSM_CONFIG_LINKWIDTH_ACCEPT = 0x6, + LTSSM_CONFIG_LANENUM_ACCEPT = 0x7, + LTSSM_CONFIG_LANENUM_WAIT = 0x8, + LTSSM_CONFIG_COMPLETE = 0x9, + LTSSM_CONFIG_IDLE = 0xa, + LTSSM_RECOVERY_RCVR_LOCK = 0xb, + LTSSM_RECOVERY_SPEED = 0xc, + LTSSM_RECOVERY_RCVR_CFG = 0xd, + LTSSM_RECOVERY_IDLE = 0xe, + LTSSM_L0 = 0x10, + LTSSM_RX_L0S_ENTRY = 0x11, + LTSSM_RX_L0S_IDLE = 0x12, + LTSSM_RX_L0S_FTS = 0x13, + LTSSM_TX_L0S_ENTRY = 0x14, + LTSSM_TX_L0S_IDLE = 0x15, + LTSSM_TX_L0S_FTS = 0x16, + LTSSM_L1_ENTRY = 0x17, + LTSSM_L1_IDLE = 0x18, + LTSSM_L2_IDLE = 0x19, + LTSSM_L2_TRANSMIT_WAKE = 0x1a, + LTSSM_DISABLED = 0x20, + LTSSM_LOOPBACK_ENTRY_MASTER = 0x21, + LTSSM_LOOPBACK_ACTIVE_MASTER = 0x22, + LTSSM_LOOPBACK_EXIT_MASTER = 0x23, + LTSSM_LOOPBACK_ENTRY_SLAVE = 0x24, + LTSSM_LOOPBACK_ACTIVE_SLAVE = 0x25, + LTSSM_LOOPBACK_EXIT_SLAVE = 0x26, + LTSSM_HOT_RESET = 0x27, + LTSSM_RECOVERY_EQUALIZATION_PHASE0 = 0x28, + LTSSM_RECOVERY_EQUALIZATION_PHASE1 = 0x29, + LTSSM_RECOVERY_EQUALIZATION_PHASE2 = 0x2a, + LTSSM_RECOVERY_EQUALIZATION_PHASE3 = 0x2b, +}; + /* PCIe core controller registers */ #define CTRL_CORE_BASE_ADDR 0x18000 #define CTRL_CONFIG_REG (CTRL_CORE_BASE_ADDR + 0x0) @@ -248,13 +289,35 @@ static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg) return readl(pcie->base + reg); }
-static int advk_pcie_link_up(struct advk_pcie *pcie) +static u8 advk_pcie_ltssm_state(struct advk_pcie *pcie) { - u32 val, ltssm_state; + u32 val; + u8 ltssm_state;
val = advk_readl(pcie, CFG_REG); ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK; - return ltssm_state >= LTSSM_L0; + return ltssm_state; +} + +static inline bool advk_pcie_link_up(struct advk_pcie *pcie) +{ + /* check if LTSSM is in normal operation - some L* state */ + u8 ltssm_state = advk_pcie_ltssm_state(pcie); + return ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED; +} + +static inline bool advk_pcie_link_training(struct advk_pcie *pcie) +{ + /* + * According to PCIe Base specification 3.0, Table 4-14: Link + * Status Mapped to the LTSSM is Link Training mapped to LTSSM + * Configuration and Recovery states. + */ + u8 ltssm_state = advk_pcie_ltssm_state(pcie); + return ((ltssm_state >= LTSSM_CONFIG_LINKWIDTH_START && + ltssm_state < LTSSM_L0) || + (ltssm_state >= LTSSM_RECOVERY_EQUALIZATION_PHASE0 && + ltssm_state <= LTSSM_RECOVERY_EQUALIZATION_PHASE3)); }
static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
From: Marek Behún marek.behun@nic.cz
commit 823868fceae3bac07cf5eccb128d6916e7a5ae9d upstream.
This is a cleanup and fix of the patch by Ken Ma make@marvell.com.
Fix the mpp definitions according to newest revision of the specification: - northbridge: fix pmic1 gpio number to 7 fix pmic0 gpio number to 6 - southbridge split pcie1 group bit mask to BIT(5) and BIT(9) fix ptp group bit mask to BIT(11) | BIT(12) | BIT(13) add smi group with bit mask BIT(4)
[gregory: split the pcie group in 2, as at hardware level they can be configured separately] Signed-off-by: Marek Behún marek.behun@nic.cz Signed-off-by: Gregory CLEMENT gregory.clement@bootlin.com Tested-by: Miquel Raynal miquel.raynal@bootlin.com Signed-off-by: Linus Walleij linus.walleij@linaro.org Signed-off-by: Marek Behún kabel@kernel.org --- .../pinctrl/marvell,armada-37xx-pinctrl.txt | 18 +++++++++++++----- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 10 ++++++---- 2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt index f64060908d5a..79dc09ca40ed 100644 --- a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt @@ -58,11 +58,11 @@ group pwm3 - functions pwm, gpio
group pmic1 - - pin 17 + - pin 7 - functions pmic, gpio
group pmic0 - - pin 16 + - pin 6 - functions pmic, gpio
group i2c2 @@ -112,17 +112,25 @@ group usb2_drvvbus1 - functions drvbus, gpio
group sdio_sb - - pins 60-64 + - pins 60-65 - functions sdio, gpio
group rgmii - - pins 42-55 + - pins 42-53 - functions mii, gpio
group pcie1 - - pins 39-40 + - pins 39 + - functions pcie, gpio + +group pcie1_clkreq + - pins 40 - functions pcie, gpio
+group smi + - pins 54-55 + - functions smi, gpio + group ptp - pins 56-58 - functions ptp, gpio diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c index 262f591ad8a6..4b4fd4661268 100644 --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c @@ -157,8 +157,8 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { PIN_GRP_GPIO("pwm1", 12, 1, BIT(4), "pwm"), PIN_GRP_GPIO("pwm2", 13, 1, BIT(5), "pwm"), PIN_GRP_GPIO("pwm3", 14, 1, BIT(6), "pwm"), - PIN_GRP_GPIO("pmic1", 17, 1, BIT(7), "pmic"), - PIN_GRP_GPIO("pmic0", 16, 1, BIT(8), "pmic"), + PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"), + PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"), PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"), PIN_GRP_GPIO("i2c1", 0, 2, BIT(10), "i2c"), PIN_GRP_GPIO("spi_cs1", 17, 1, BIT(12), "spi"), @@ -182,8 +182,10 @@ static struct armada_37xx_pin_group armada_37xx_sb_groups[] = { PIN_GRP_GPIO("usb2_drvvbus1", 1, 1, BIT(1), "drvbus"), PIN_GRP_GPIO("sdio_sb", 24, 6, BIT(2), "sdio"), PIN_GRP_GPIO("rgmii", 6, 12, BIT(3), "mii"), - PIN_GRP_GPIO("pcie1", 3, 2, BIT(4), "pcie"), - PIN_GRP_GPIO("ptp", 20, 3, BIT(5), "ptp"), + PIN_GRP_GPIO("smi", 18, 2, BIT(4), "smi"), + PIN_GRP_GPIO("pcie1", 3, 1, BIT(5), "pcie"), + PIN_GRP_GPIO("pcie1_clkreq", 4, 1, BIT(9), "pcie"), + PIN_GRP_GPIO("ptp", 20, 3, BIT(11) | BIT(12) | BIT(13), "ptp"), PIN_GRP("ptp_clk", 21, 1, BIT(6), "ptp", "mii"), PIN_GRP("ptp_trig", 22, 1, BIT(7), "ptp", "mii"), PIN_GRP_GPIO_3("mii_col", 23, 1, BIT(8) | BIT(14), 0, BIT(8), BIT(14),
From: Gregory CLEMENT gregory.clement@bootlin.com
commit 4d98fbaacd79a82f408febb66a9c42fe42361b16 upstream.
Declare the PCIe1 Wakeup which was initially missing.
Signed-off-by: Gregory CLEMENT gregory.clement@bootlin.com Tested-by: Miquel Raynal miquel.raynal@bootlin.com Signed-off-by: Linus Walleij linus.walleij@linaro.org Signed-off-by: Marek Behún kabel@kernel.org --- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c index 4b4fd4661268..a58367909455 100644 --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c @@ -185,6 +185,7 @@ static struct armada_37xx_pin_group armada_37xx_sb_groups[] = { PIN_GRP_GPIO("smi", 18, 2, BIT(4), "smi"), PIN_GRP_GPIO("pcie1", 3, 1, BIT(5), "pcie"), PIN_GRP_GPIO("pcie1_clkreq", 4, 1, BIT(9), "pcie"), + PIN_GRP_GPIO("pcie1_wakeup", 5, 1, BIT(10), "pcie"), PIN_GRP_GPIO("ptp", 20, 3, BIT(11) | BIT(12) | BIT(13), "ptp"), PIN_GRP("ptp_clk", 21, 1, BIT(6), "ptp", "mii"), PIN_GRP("ptp_trig", 22, 1, BIT(7), "ptp", "mii"),
commit baf8d6899b1e8906dc076ef26cc633e96a8bb0c3 upstream.
The PWM pins on North Bridge on Armada 37xx can be configured into PWM or GPIO functions. When in PWM function, each pin can also be configured to drive low on 0 and tri-state on 1 (LED mode).
The current definitions handle this by declaring two pin groups for each pin: - group "pwmN" with functions "pwm" and "gpio" - group "ledN_od" ("od" for open drain) with functions "led" and "gpio"
This is semantically incorrect. The correct definition for each pin should be one group with three functions: "pwm", "led" and "gpio".
Change the "pwmN" groups to support "led" function.
Remove "ledN_od" groups. This cannot break backwards compatibility with older device trees: no device tree uses it since there is no PWM driver for this SOC yet. Also "ledN_od" groups are not even documented.
Fixes: b835d6953009 ("pinctrl: armada-37xx: swap polarity on LED group") Signed-off-by: Marek Behún kabel@kernel.org Acked-by: Rob Herring robh@kernel.org Link: https://lore.kernel.org/r/20210719112938.27594-1-kabel@kernel.org Signed-off-by: Linus Walleij linus.walleij@linaro.org Signed-off-by: Marek Behún kabel@kernel.org --- .../pinctrl/marvell,armada-37xx-pinctrl.txt | 8 ++++---- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 17 ++++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt index 79dc09ca40ed..35f469c4f675 100644 --- a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt @@ -43,19 +43,19 @@ group emmc_nb
group pwm0 - pin 11 (GPIO1-11) - - functions pwm, gpio + - functions pwm, led, gpio
group pwm1 - pin 12 - - functions pwm, gpio + - functions pwm, led, gpio
group pwm2 - pin 13 - - functions pwm, gpio + - functions pwm, led, gpio
group pwm3 - pin 14 - - functions pwm, gpio + - functions pwm, led, gpio
group pmic1 - pin 7 diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c index a58367909455..9f7d917458c7 100644 --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c @@ -153,10 +153,14 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { PIN_GRP_GPIO("jtag", 20, 5, BIT(0), "jtag"), PIN_GRP_GPIO("sdio0", 8, 3, BIT(1), "sdio"), PIN_GRP_GPIO("emmc_nb", 27, 9, BIT(2), "emmc"), - PIN_GRP_GPIO("pwm0", 11, 1, BIT(3), "pwm"), - PIN_GRP_GPIO("pwm1", 12, 1, BIT(4), "pwm"), - PIN_GRP_GPIO("pwm2", 13, 1, BIT(5), "pwm"), - PIN_GRP_GPIO("pwm3", 14, 1, BIT(6), "pwm"), + PIN_GRP_GPIO_3("pwm0", 11, 1, BIT(3) | BIT(20), 0, BIT(20), BIT(3), + "pwm", "led"), + PIN_GRP_GPIO_3("pwm1", 12, 1, BIT(4) | BIT(21), 0, BIT(21), BIT(4), + "pwm", "led"), + PIN_GRP_GPIO_3("pwm2", 13, 1, BIT(5) | BIT(22), 0, BIT(22), BIT(5), + "pwm", "led"), + PIN_GRP_GPIO_3("pwm3", 14, 1, BIT(6) | BIT(23), 0, BIT(23), BIT(6), + "pwm", "led"), PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"), PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"), PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"), @@ -170,11 +174,6 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { PIN_GRP_EXTRA("uart2", 9, 2, BIT(1) | BIT(13) | BIT(14) | BIT(19), BIT(1) | BIT(13) | BIT(14), BIT(1) | BIT(19), 18, 2, "gpio", "uart"), - PIN_GRP_GPIO_2("led0_od", 11, 1, BIT(20), BIT(20), 0, "led"), - PIN_GRP_GPIO_2("led1_od", 12, 1, BIT(21), BIT(21), 0, "led"), - PIN_GRP_GPIO_2("led2_od", 13, 1, BIT(22), BIT(22), 0, "led"), - PIN_GRP_GPIO_2("led3_od", 14, 1, BIT(23), BIT(23), 0, "led"), - };
static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
From: Miquel Raynal miquel.raynal@bootlin.com
commit a5470af981a0cc14a650af8da5186668971a4fc8 upstream.
One pin can be muxed as PCIe endpoint card reset.
Signed-off-by: Miquel Raynal miquel.raynal@bootlin.com Signed-off-by: Gregory CLEMENT gregory.clement@bootlin.com Signed-off-by: Marek Behún kabel@kernel.org --- arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi index 257bfa811fe8..a4c81a1c77b7 100644 --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi @@ -239,6 +239,15 @@ function = "mii"; };
+ pcie_reset_pins: pcie-reset-pins { + groups = "pcie1"; + function = "pcie"; + }; + + pcie_clkreq_pins: pcie-clkreq-pins { + groups = "pcie1_clkreq"; + function = "pcie"; + }; };
eth0: ethernet@30000 {
From: Marek Behún marek.behun@nic.cz
commit 715878016984b2617f6c1f177c50039e12e7bd5b upstream.
We found out that we are unable to control the PERST# signal via the default pin dedicated to be PERST# pin (GPIO2[3] pin) on A3700 SOC when this pin is in EP_PCIE1_Resetn mode. There is a register in the PCIe register space called PERSTN_GPIO_EN (D0088004[3]), but changing the value of this register does not change the pin output when measuring with voltmeter.
We do not know if this is a bug in the SOC, or if it works only when PCIe controller is in a certain state.
Commit f4c7d053d7f7 ("PCI: aardvark: Wait for endpoint to be ready before training link") says that when this pin changes pinctrl mode from EP_PCIE1_Resetn to GPIO, the PERST# signal is asserted for a brief moment.
So currently the situation is that on A3700 boards the PERST# signal is asserted in U-Boot (because the code in U-Boot issues reset via this pin via GPIO mode), and then in Linux by the obscure and undocumented mechanism described by the above mentioned commit.
We want to issue PERST# signal in a known way, therefore this patch changes the pcie_reset_pin function from "pcie" to "gpio" and adds the reset-gpios property to the PCIe node in device tree files of EspressoBin and Armada 3720 Dev Board (Turris Mox device tree already has this property and uDPU does not have a PCIe port).
Signed-off-by: Marek Behún marek.behun@nic.cz Cc: Remi Pommarel repk@triplefau.lt Tested-by: Tomasz Maciej Nowak tmn505@gmail.com Acked-by: Thomas Petazzoni thomas.petazzoni@bootlin.com Signed-off-by: Gregory CLEMENT gregory.clement@bootlin.com Signed-off-by: Marek Behún kabel@kernel.org --- arch/arm64/boot/dts/marvell/armada-3720-db.dts | 3 +++ arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts | 3 +++ arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts index 9df0f06ce607..0a0e9e19e879 100644 --- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts +++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts @@ -155,6 +155,9 @@
/* CON15(V2.0)/CON17(V1.4) : PCIe / CON15(V2.0)/CON12(V1.4) :mini-PCIe */ &pcie0 { + pinctrl-names = "default"; + pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>; + reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>; status = "okay"; };
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts index 13c0ec053ada..89c7385390f4 100644 --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts @@ -82,6 +82,9 @@
/* J9 */ &pcie0 { + pinctrl-names = "default"; + pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>; + reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>; status = "okay"; };
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi index a4c81a1c77b7..b369356e7419 100644 --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi @@ -241,7 +241,7 @@
pcie_reset_pins: pcie-reset-pins { groups = "pcie1"; - function = "pcie"; + function = "gpio"; };
pcie_clkreq_pins: pcie-clkreq-pins {
linux-stable-mirror@lists.linaro.org