On Tue, May 21, 2019 at 10:17:26PM +0530, Vidya Sagar wrote:Yes. There is a plan to boot kernel through UEFI (using ACPI) on this system.
On 5/21/2019 3:57 PM, Thierry Reding wrote:
On Fri, May 17, 2019 at 06:08:33PM +0530, Vidya Sagar wrote:
Tegra194 rootports don't generate MSI interrupts for PME events and hence
MSI needs to be disabled for them to avoid root ports service drivers
registering their respective ISRs with MSI interrupt.
The service drivers (AER, hotplug, etc) don't know whether the
interrupt is INTx or MSI; they just register their ISRs with
pcie_device.irq.
The point of this patch is that the PCI core doesn't support devices
that use MSI and INTx at the same time, and since this device can't
generate MSI for PME, we have to use INTx for *all* its interrupts.
Ok. I'll move it to DWC driver along with name change for the quirk API.Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
---
Changes since [v6]:
* This is a new patch
drivers/pci/quirks.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0f16acc323c6..28f9a0380df5 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2592,6 +2592,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA,
PCI_DEVICE_ID_NVIDIA_NVENET_15,
nvenet_msi_disable);
+/*
+ * Tegra194's PCIe root ports don't generate MSI interrupts for PME events
+ * instead legacy interrupts are generated. Hence, to avoid service drivers
+ * registering their respective ISRs for MSIs, need to disable MSI interrupts
+ * for root ports.
+ */
+static void disable_tegra194_rp_msi(struct pci_dev *dev)
+{
+ dev->no_msi = 1;
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad0, disable_tegra194_rp_msi);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad1, disable_tegra194_rp_msi);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad2, disable_tegra194_rp_msi);
+
Later functions in this file seem to use a more consistent naming
pattern, according to which the name for this would become:
pci_quirk_nvidia_tegra194_disable_rp_msi
Might be worth considering making this consistent.
This could also be moved to the DWC driver to restrict this to where it
is needed. In either case, this seems like a good solution, so:
Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>
Is there any possibility this hardware would be used in an ACPI
system? If so, the quirk should probably stay in drivers/pci/quirks.c
because the DWC driver would not be present.
Ok. I'll take care of it in the next patch series.
Either way, please also add some PCIe spec references about this in
the changelog and a comment in the code about working around this
issue. I think the MSI/MSI-X sections that prohibit a device from
using both INTx and MSI/MSI-X are probably the most pertinent.
Ok. I'll take it up with our hardware engineers to have only MSI/MSI-X interrupts
The reason I want a comment about this is to discourage future
hardware from following this example because every device that *does*
follow this example requires a kernel update that would be otherwise
unnecessary.
Bjorn