From: "Pali Rohár" <pali@kernel.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Rob Herring <robh@kernel.org>, Bjorn Helgaas <bhelgaas@google.com> Cc: "Russell King" <rmk+kernel@armlinux.org.uk>, "Marek Behún" <kabel@kernel.org>, "Remi Pommarel" <repk@triplefau.lt>, Xogium <contact@xogium.me>, "Tomasz Maciej Nowak" <tmn505@gmail.com>, "Marc Zyngier" <maz@kernel.org>, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 07/42] PCI: aardvark: Fix link training Date: Thu, 6 May 2021 17:31:18 +0200 [thread overview] Message-ID: <20210506153153.30454-8-pali@kernel.org> (raw) In-Reply-To: <20210506153153.30454-1-pali@kernel.org> 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 revision 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 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 check when 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/ Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Marek Behún <kabel@kernel.org> Cc: stable@vger.kernel.org # f4c7d053d7f7 ("PCI: aardvark: Wait for endpoint to be ready before training link") Cc: stable@vger.kernel.org # 6964494582f5 ("PCI: aardvark: Train link immediately after enabling training") Cc: stable@vger.kernel.org # 43fc679ced18 ("PCI: aardvark: Improve link training") Cc: stable@vger.kernel.org # 5169a9851daa ("PCI: aardvark: Issue PERST via GPIO") Cc: stable@vger.kernel.org # 96be36dbffac ("PCI: aardvark: Replace custom macros by standard linux/pci_regs.h macros") Cc: stable@vger.kernel.org # d0c6a3475b03 ("PCI: aardvark: Move PCIe reset card code to advk_pcie_train_link()") Cc: stable@vger.kernel.org # 1d1cd163d0de ("PCI: aardvark: Update comment about disabling link training") --- drivers/pci/controller/pci-aardvark.c | 117 ++++++++------------------ 1 file changed, 34 insertions(+), 83 deletions(-) diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index e297ec9ec390..edeacd95297e 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -209,11 +209,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; @@ -251,23 +246,9 @@ static void advk_pcie_wait_for_retrain(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); @@ -275,53 +256,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. @@ -332,41 +306,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. - */ - 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. + * 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. */ - 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"); } static void advk_pcie_setup_hw(struct advk_pcie *pcie) -- 2.20.1
WARNING: multiple messages have this Message-ID (diff)
From: "Pali Rohár" <pali@kernel.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Rob Herring <robh@kernel.org>, Bjorn Helgaas <bhelgaas@google.com> Cc: "Russell King" <rmk+kernel@armlinux.org.uk>, "Marek Behún" <kabel@kernel.org>, "Remi Pommarel" <repk@triplefau.lt>, Xogium <contact@xogium.me>, "Tomasz Maciej Nowak" <tmn505@gmail.com>, "Marc Zyngier" <maz@kernel.org>, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 07/42] PCI: aardvark: Fix link training Date: Thu, 6 May 2021 17:31:18 +0200 [thread overview] Message-ID: <20210506153153.30454-8-pali@kernel.org> (raw) In-Reply-To: <20210506153153.30454-1-pali@kernel.org> 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 revision 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 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 check when 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/ Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Marek Behún <kabel@kernel.org> Cc: stable@vger.kernel.org # f4c7d053d7f7 ("PCI: aardvark: Wait for endpoint to be ready before training link") Cc: stable@vger.kernel.org # 6964494582f5 ("PCI: aardvark: Train link immediately after enabling training") Cc: stable@vger.kernel.org # 43fc679ced18 ("PCI: aardvark: Improve link training") Cc: stable@vger.kernel.org # 5169a9851daa ("PCI: aardvark: Issue PERST via GPIO") Cc: stable@vger.kernel.org # 96be36dbffac ("PCI: aardvark: Replace custom macros by standard linux/pci_regs.h macros") Cc: stable@vger.kernel.org # d0c6a3475b03 ("PCI: aardvark: Move PCIe reset card code to advk_pcie_train_link()") Cc: stable@vger.kernel.org # 1d1cd163d0de ("PCI: aardvark: Update comment about disabling link training") --- drivers/pci/controller/pci-aardvark.c | 117 ++++++++------------------ 1 file changed, 34 insertions(+), 83 deletions(-) diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index e297ec9ec390..edeacd95297e 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -209,11 +209,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; @@ -251,23 +246,9 @@ static void advk_pcie_wait_for_retrain(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); @@ -275,53 +256,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. @@ -332,41 +306,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. - */ - 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. + * 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. */ - 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"); } static void advk_pcie_setup_hw(struct advk_pcie *pcie) -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-06 15:34 UTC|newest] Thread overview: 146+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-06 15:31 [PATCH 00/42] PCI: aardvark: Various driver fixes Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 01/42] PCI: aardvark: Fix kernel panic during PIO transfer Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-19 8:06 ` Pali Rohár 2021-05-19 8:06 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 02/42] PCI: aardvark: Fix checking for PIO Non-posted Request Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 03/42] PCI: aardvark: Fix checking for PIO status Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 04/42] PCI: aardvark: Increase polling delay to 1.5s while waiting for PIO response Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 05/42] PCI: pci-bridge-emul: Add PCIe Root Capabilities Register Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 23:10 ` Bjorn Helgaas 2021-05-06 23:10 ` Bjorn Helgaas 2021-05-07 14:40 ` Pali Rohár 2021-05-07 14:40 ` Pali Rohár 2021-05-07 16:41 ` Bjorn Helgaas 2021-05-07 16:41 ` Bjorn Helgaas 2021-05-06 15:31 ` [PATCH 06/42] PCI: aardvark: Fix reporting CRS Software Visibility on emulated bridge Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 13:03 ` Bjorn Helgaas 2021-05-07 13:03 ` Bjorn Helgaas 2021-05-07 15:25 ` Pali Rohár 2021-05-07 15:25 ` Pali Rohár 2021-05-07 15:33 ` Pali Rohár 2021-05-07 15:33 ` Pali Rohár 2021-05-06 15:31 ` Pali Rohár [this message] 2021-05-06 15:31 ` [PATCH 07/42] PCI: aardvark: Fix link training Pali Rohár 2021-05-06 15:31 ` [PATCH 08/42] PCI: Add PCI_EXP_DEVCTL_PAYLOAD_* macros Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 09/42] PCI: aardvark: Fix PCIe Max Payload Size setting Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 10/42] PCI: aardvark: Implement workaround for the readback value of VEND_ID Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 11/42] PCI: aardvark: Do not touch status bits of masked interrupts in interrupt handler Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 12/42] PCI: aardvark: Check for virq mapping when processing INTx IRQ Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 9:15 ` Marc Zyngier 2021-05-07 9:15 ` Marc Zyngier 2021-06-04 16:24 ` Pali Rohár 2021-06-04 16:24 ` Pali Rohár 2021-06-04 16:29 ` Marc Zyngier 2021-06-04 16:29 ` Marc Zyngier 2021-05-06 15:31 ` [PATCH 13/42] PCI: aardvark: Remove irq_mask_ack callback for INTx interrupts Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 9:16 ` Marc Zyngier 2021-05-07 9:16 ` Marc Zyngier 2021-05-06 15:31 ` [PATCH 14/42] PCI: aardvark: Don't mask irq when mapping Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 9:20 ` Marc Zyngier 2021-05-07 9:20 ` Marc Zyngier 2021-05-07 9:27 ` Pali Rohár 2021-05-07 9:27 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 15/42] PCI: aardvark: Change name of INTx irq_chip to advk-INT Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 9:08 ` Marc Zyngier 2021-05-07 9:08 ` Marc Zyngier 2021-05-24 14:36 ` Marek Behún 2021-05-24 14:36 ` Marek Behún 2021-05-24 15:14 ` Marc Zyngier 2021-05-24 15:14 ` Marc Zyngier 2021-05-06 15:31 ` [PATCH 16/42] PCI: aardvark: Remove unneeded goto Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 17/42] PCI: aardvark: Fix support for MSI interrupts Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 10:16 ` Marc Zyngier 2021-05-07 10:16 ` Marc Zyngier 2021-05-07 14:44 ` Pali Rohár 2021-05-07 14:44 ` Pali Rohár 2021-05-07 16:24 ` Marc Zyngier 2021-05-07 16:24 ` Marc Zyngier 2021-06-04 16:02 ` Pali Rohár 2021-06-04 16:02 ` Pali Rohár 2021-06-04 16:22 ` Marc Zyngier 2021-06-04 16:22 ` Marc Zyngier 2021-05-06 15:31 ` [PATCH 18/42] PCI: aardvark: Correctly clear and unmask all " Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 10:19 ` Marc Zyngier 2021-05-07 10:19 ` Marc Zyngier 2021-05-07 10:21 ` Pali Rohár 2021-05-07 10:21 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 19/42] PCI: aardvark: Fix setting MSI address Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-07 10:25 ` Marc Zyngier 2021-05-07 10:25 ` Marc Zyngier 2021-05-06 15:31 ` [PATCH 20/42] PCI: aardvark: Add support for more than 32 MSI interrupts Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-07-02 21:35 ` Pali Rohár 2021-07-02 21:35 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 21/42] PCI: aardvark: Add support for masking " Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 22/42] PCI: aardvark: Enable MSI-X support Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 23/42] PCI: aardvark: Fix support for ERR interrupt on emulated bridge Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 24/42] PCI: aardvark: Fix support for PME " Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 25/42] PCI: aardvark: Fix support for PME requester " Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 26/42] PCI: aardvark: Fix support for bus mastering and PCI_COMMAND " Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 27/42] PCI: aardvark: Disable bus mastering and mask all interrupts when unbinding driver Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 28/42] PCI: aardvark: Free config space for emulated root bridge when unbinding driver to fix memory leak Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 29/42] PCI: aardvark: Reset PCIe card and disable PHY when unbinding driver Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 30/42] PCI: aardvark: Rewrite irq code to chained irq handler Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 31/42] PCI: aardvark: Use separate INTA interrupt for emulated root bridge Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 32/42] PCI: pci-bridge-emul: Add description for class_revision field Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 33/42] PCI: pci-bridge-emul: Add definitions for missing capabilities registers Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 34/42] PCI: aardvark: Add support for DEVCAP2, DEVCTL2, LNKCAP2 and LNKCTL2 registers on emulated bridge Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 35/42] PCI: aardvark: Add support for PCI_BRIDGE_CTL_BUS_RESET " Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 36/42] PCI: aardvark: Replace custom PCIE_CORE_ERR_CAPCTL_* macros by linux/pci_regs.h macros Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 37/42] PCI: aardvark: Replace custom PCIE_CORE_INT_* macros by linux PCI_INTERRUPT_* values Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 38/42] PCI: aardvark: Cleanup some register macros Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 39/42] PCI: aardvark: Add comments for OB_WIN_ENABLE and ADDR_WIN_DISABLE Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 40/42] PCI: pci-bridge-emul: re-arrange register tests Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 41/42] PCI: pci-bridge-emul: add support for PCIe extended capabilities Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-05-06 15:31 ` [PATCH 42/42] PCI: aardvark: Add support for Advanced Error Reporting registers on emulated bridge Pali Rohár 2021-05-06 15:31 ` Pali Rohár 2021-06-03 15:16 ` [PATCH 00/42] PCI: aardvark: Various driver fixes Lorenzo Pieralisi 2021-06-03 15:16 ` Lorenzo Pieralisi 2021-06-03 17:02 ` Pali Rohár 2021-06-03 17:02 ` Pali Rohár 2021-06-03 18:02 ` Simon Glass 2021-06-03 18:02 ` Simon Glass 2021-06-03 18:18 ` Pali Rohár 2021-06-03 18:18 ` Pali Rohár 2021-06-04 14:05 ` Lorenzo Pieralisi 2021-06-04 14:05 ` Lorenzo Pieralisi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210506153153.30454-8-pali@kernel.org \ --to=pali@kernel.org \ --cc=bhelgaas@google.com \ --cc=contact@xogium.me \ --cc=kabel@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=maz@kernel.org \ --cc=repk@triplefau.lt \ --cc=rmk+kernel@armlinux.org.uk \ --cc=robh@kernel.org \ --cc=thomas.petazzoni@bootlin.com \ --cc=tmn505@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.