Messages in this thread |  | | Date | Sat, 23 Apr 2022 12:52:42 +0530 | Subject | Re: [PATCH V1 09/10] PCI: Disable MSI for Tegra234 root ports | From | Vidya Sagar <> |
| |
On 2/7/2022 11:06 PM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > On Sat, Feb 05, 2022 at 09:51:43PM +0530, Vidya Sagar wrote: >> Tegra234 PCIe rootports don't generate MSI interrupts for PME and AER >> events. Since PCIe spec (Ref: r4.0 sec 7.7.1.2 and 7.7.2.2) doesn't support >> using a mix of INTx and MSI/MSI-X, MSI needs to be disabled to avoid root >> ports service drivers registering their respective ISRs with MSI interrupt >> and to let only INTx be used for all events. > > s/rootports/root ports/ to match other usage here. > > This argument matches that in 8c7e96d3fe75 ("PCI: Disable MSI for > Tegra root ports") [1], but that's not quite what sec 7.7.1.2 and > 7.7.2.2 say. Those sections talk about what happens when both MSI and > MSI-X are disabled: > > If MSI and MSI-X are both disabled, the Function requests servicing > using INTx interrupts (if supported). > > but they don't say anything about what happens when MSI or MSI-X is > *enabled*. > > I think a better citation is PCIe r6.0, sec 6.1.4.3, which says: > > While enabled for MSI or MSI-X operation, a Function is prohibited > from using INTx interrupts (if implemented) to request service (MSI, > MSI-X, and INTx are mutually exclusive). > > Can you please update the comment in the code and this commit log to > cite PCIe r6.0, sec 6.1.4.3 instead, and to clarify that these Tegra > devices always use INTx for PME and AER, even when MSI/MSI-X is > enabled?
Sure. I would fix this in the next patch set.
> > Why do these Tegra quirks use DECLARE_PCI_FIXUP_CLASS_EARLY() instead > of just DECLARE_PCI_FIXUP_EARLY()? quirk_al_msi_disable() uses the > _CLASS version because the same Device ID is used for non-Root Port > devices. Is the same true here, or could these use > DECLARE_PCI_FIXUP_EARLY()?
Tegra's PCIe controllers are also dual mode controllers and MSI works just fine when they operate in the endpoint mode configuration.
> > There are many quirks that disable MSI, and they're a mixture of EARLY > and FINAL. They should probably all be the same. > > [1] https://git.kernel.org/linus/8c7e96d3fe75 > >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> drivers/pci/quirks.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index d2dd6a6cda60..3ac5c45e61a1 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -2747,6 +2747,15 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_NVIDIA, 0x10e5, >> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_NVIDIA, 0x10e6, >> PCI_CLASS_BRIDGE_PCI, 8, >> pci_quirk_nvidia_tegra_disable_rp_msi); >> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_NVIDIA, 0x229a, >> + PCI_CLASS_BRIDGE_PCI, 8, >> + pci_quirk_nvidia_tegra_disable_rp_msi); >> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_NVIDIA, 0x229c, >> + PCI_CLASS_BRIDGE_PCI, 8, >> + pci_quirk_nvidia_tegra_disable_rp_msi); >> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_NVIDIA, 0x229e, >> + PCI_CLASS_BRIDGE_PCI, 8, >> + pci_quirk_nvidia_tegra_disable_rp_msi); >> >> /* >> * Some versions of the MCP55 bridge from Nvidia have a legacy IRQ routing >> -- >> 2.17.1 >>
|  |