On 7/3/2019 10:09 PM, Lorenzo Pieralisi wrote:
On Mon, Jul 01, 2019 at 04:39:42PM +0530, Vidya Sagar wrote:
Currently Relaxed Ordering bit in the configuration space is enabled for
all devices, but, as per the Technical Reference Manual of Tegra20 which is
What devices ?
All PCIe devices, because, current quirk uses PCI_ANY_ID for both Vendor-ID
and Device-ID and that makes it applicable for all PCIe devices.
available at https://developer.nvidia.com/embedded/downloads#?search=tegra%202
in Sec 34.1, it is mentioned that Relexed Ordering bit needs to be enabled in
its root ports to avoid deadlock in hardware. The same is applicable for
Tegra30 as well though it is not explicitly mentioned in Tegra30 TRM document,
but the same should not be extended to root ports of other Tegra SoCs or
other hosts.
Should not or must not ? What does this sentence mean ?
I think it is 'must not' here. Since the hardware deadlock issue is not present
in any other root ports other than Tegra20 and Tegra30, this quirk must not be
extended to any other root ports.
Can we try to be more precise please ?
As I said before the commit log must be clear to anyone reading it
even if he has no background information.
Would the following work?
Currently Relaxed Ordering bit in the configuration space is enabled for
all PCIe devices as the quirk uses PCI_ANY_ID for both Vendor-ID and
Device-ID, but, as per the Technical Reference Manual of Tegra20 which is
available at https://developer.nvidia.com/embedded/downloads#?search=tegra%202
in Sec 34.1, it is mentioned that Relexed Ordering bit needs to be enabled in
its root ports to avoid deadlock in hardware. The same is applicable for
Tegra30 as well though it is not explicitly mentioned in Tegra30 TRM document,
but the same must not be extended to root ports of other Tegra SoCs or
other hosts as the same issue doesn't exist there.
Lorenzo
Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
---
V2:
* Modified commit message to include reference to Tegra20 TRM document.
drivers/pci/controller/pci-tegra.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 9cc03a2549c0..241760aa15bd 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -787,12 +787,15 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class);
-/* Tegra PCIE requires relaxed ordering */
+/* Tegra20 and Tegra30 PCIE requires relaxed ordering */
static void tegra_pcie_relax_enable(struct pci_dev *dev)
{
pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN);
}
-DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_relax_enable);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_relax_enable);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_relax_enable);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_relax_enable);
static int tegra_pcie_request_resources(struct tegra_pcie *pcie)
{
--
2.17.1
