kernel/linux.git/drivers/pci/controller, branch v5.15.7

PCI: aardvark: Fix link training

2021-12-01T08:04:44+00:00

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 Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún Signed-off-by: Marek Behún Signed-off-by: Greg Kroah-Hartman

PCI: aardvark: Simplify initialization of rootcap on virtual bridge

2021-12-01T08:04:44+00:00

commit 454c53271fc11f3aa5e44e41fd99ca181bd32c62 upstream. PCIe config space can be initialized also before pci_bridge_emul_init() call, so move rootcap initialization after PCI config space initialization. This simplifies the function a little since it removes one if (ret < 0) check. Link: https://lore.kernel.org/r/20211005180952.6812-11-kabel@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún Signed-off-by: Marek Behún Signed-off-by: Greg Kroah-Hartman

PCI: aardvark: Implement re-issuing config requests on CRS response

2021-12-01T08:04:44+00:00

commit 223dec14a05337a4155f1deed46d2becce4d00fd upstream. Commit 43f5c77bcbd2 ("PCI: aardvark: Fix reporting CRS value") fixed handling of CRS response and when CRSSVE flag was not enabled it marked CRS response as failed transaction (due to simplicity). But pci-aardvark.c driver is already waiting up to the PIO_RETRY_CNT count for PIO config response and so we can with a small change implement re-issuing of config requests as described in PCIe base specification. This change implements re-issuing of config requests when response is CRS. Set upper bound of wait cycles to around PIO_RETRY_CNT, afterwards the transaction is marked as failed and an all-ones value is returned as before. We do this by returning appropriate error codes from function advk_pcie_check_pio_status(). On CRS we return -EAGAIN and caller then reissues transaction. Link: https://lore.kernel.org/r/20211005180952.6812-10-kabel@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún Signed-off-by: Greg Kroah-Hartman

PCI: aardvark: Deduplicate code in advk_pcie_rd_conf()

2021-12-01T08:04:44+00:00

commit 67cb2a4c93499c2c22704998fd1fd2bc35194d8e upstream. Avoid code repetition in advk_pcie_rd_conf() by handling errors with goto jump, as is customary in kernel. Link: https://lore.kernel.org/r/20211005180952.6812-9-kabel@kernel.org Fixes: 43f5c77bcbd2 ("PCI: aardvark: Fix reporting CRS value") Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Signed-off-by: Greg Kroah-Hartman

PCI: aardvark: Fix PCIe Max Payload Size setting

2021-11-18T18:17:20+00:00

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 Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman

PCI: j721e: Fix j721e_pcie_probe() error path

2021-11-18T18:17:06+00:00

[ Upstream commit 496bb18483cc0474913e81e18a6b313aaea4c120 ] If an error occurs after a successful cdns_pcie_init_phy() call, it must be undone by a cdns_pcie_disable_phy() call, as already done above and below. Update the goto to branch at the correct place of the error handling path. Link: https://lore.kernel.org/r/db477b0cb444891a17c4bb424467667dc30d0bab.1624794264.git.christophe.jaillet@wanadoo.fr Fixes: 49e0efdce791 ("PCI: j721e: Add support to provide refclk to PCIe connector") Signed-off-by: Christophe JAILLET Signed-off-by: Lorenzo Pieralisi Signed-off-by: Bjorn Helgaas Reviewed-by: Krzysztof Wilczyński Signed-off-by: Sasha Levin

PCI: uniphier: Serialize INTx masking/unmasking and fix the bit operation

2021-11-18T18:17:01+00:00

[ Upstream commit 4caab28a6215da5f3c1b505ff08810bc6acfe365 ] The condition register PCI_RCV_INTX is used in irq_mask() and irq_unmask() callbacks. Accesses to register can occur at the same time without a lock. Add a lock into each callback to prevent the issue. And INTX mask and unmask fields in PCL_RCV_INTX register should only be set/reset for each bit. Clearing by PCL_RCV_INTX_ALL_MASK should be removed. INTX status fields in PCL_RCV_INTX register only indicates each INTX interrupt status, so the handler can't clear by writing 1 to the field. The status is expected to be cleared by the interrupt origin. The ack function has no meaning, so should remove it. Suggested-by: Pali Rohár Link: https://lore.kernel.org/r/1631924579-24567-1-git-send-email-hayashi.kunihiko@socionext.com Fixes: 7e6d5cd88a6f ("PCI: uniphier: Add UniPhier PCIe host controller support") Signed-off-by: Kunihiko Hayashi Signed-off-by: Lorenzo Pieralisi Acked-by: Pali Rohár Acked-by: Marc Zyngier Signed-off-by: Sasha Levin

PCI: aardvark: Fix preserving PCI_EXP_RTCTL_CRSSVE flag on emulated bridge

2021-11-18T18:17:00+00:00

[ Upstream commit d419052bc6c60fa4ab2b5a51d5f1e55a66e2b4ff ] Commit 43f5c77bcbd2 ("PCI: aardvark: Fix reporting CRS value") started using CRSSVE flag for handling CRS responses. PCI_EXP_RTCTL_CRSSVE flag is stored only in emulated config space buffer and there is handler for PCI_EXP_RTCTL register. So every read operation from config space automatically clears CRSSVE flag as it is not defined in PCI_EXP_RTCTL read handler. Fix this by reading current CRSSVE bit flag from emulated space buffer and appending it to PCI_EXP_RTCTL read response. Link: https://lore.kernel.org/r/20211005180952.6812-5-kabel@kernel.org Fixes: 43f5c77bcbd2 ("PCI: aardvark: Fix reporting CRS value") Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún Signed-off-by: Sasha Levin

PCI: aardvark: Don't spam about PIO Response Status

2021-11-18T18:17:00+00:00

[ Upstream commit 464de7e7fff767e87429cd7be09c4f2cb50a6ccb ] Use dev_dbg() instead of dev_err() in advk_pcie_check_pio_status(). For example CRS is not an error status, it just says that the request should be retried. Link: https://lore.kernel.org/r/20211005180952.6812-4-kabel@kernel.org Fixes: 8c39d710363c1 ("PCI: aardvark: Add Aardvark PCI host controller driver") Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Signed-off-by: Sasha Levin

PCI: aardvark: Fix support for PCI_ROM_ADDRESS1 on emulated bridge

2021-11-18T18:16:05+00:00

commit 239edf686c14a9ff926dec2f350289ed7adfefe2 upstream. This register is exported at address offset 0x30. Link: https://lore.kernel.org/r/20211028185659.20329-8-kabel@kernel.org Fixes: 8a3ebd8de328 ("PCI: aardvark: Implement emulated root PCI bridge config space") Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Signed-off-by: Lorenzo Pieralisi Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman