| Message ID | 20211007194409.3641467-1-danielwa@cisco.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-RFC] drivers: pci: pcieport: Allow AER service only on root ports | expand |
Please note "git log --oneline drivers/pci/pcie/portdrv_core.c" and make your patch follow the style. On Thu, Oct 07, 2021 at 12:44:09PM -0700, Daniel Walker wrote: > From: Suneel Garapati <sgarapati@marvell.com> > > Some AER interrupt capability registers may not be present on > non Root ports. Since there is no way to check presence of > ROOT_ERR_COMMAND and ROOT_ERR_STATUS registers. Allow AER > service only on root ports. > Otherwise AER interrupt message number is read incorrectly > causing MSIX vector registration to fail and fallback to legacy > unnecessarily. Wrap to fill 75 columns. Add blank lines between paragraphs. Use complete sentences ("Since there is ..." is not a sentence). Capitalize consistently ("Root ports" vs "root ports"). Use register names I can find in the spec or with grep ("ROOT_ERR_COMMAND" and "ROOT_ERR_STATUS" do not appear in the source). s/MSIX/MSI-X/ to match spec usage. Also applies to code comment below. > Signed-off-by: Suneel Garapati <sgarapati@marvell.com> Needs to include your (Daniel's) Signed-off-by; see: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v5.14#n365 > Reviewed-by: Chandrakala Chavva <cchavva@marvell.com> > Reviewed-by: Sunil Kovvuri Goutham <sgoutham@marvell.com> > --- > drivers/pci/pcie/portdrv_core.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c > index 3ee63968deaa..edc355971a32 100644 > --- a/drivers/pci/pcie/portdrv_core.c > +++ b/drivers/pci/pcie/portdrv_core.c > @@ -221,7 +221,16 @@ static int get_port_device_capability(struct pci_dev *dev) > } > > #ifdef CONFIG_PCIEAER > + /* > + * Some AER interrupt capability registers may not be present on > + * non Root ports. Since there is no way to check presence of > + * ROOT_ERR_COMMAND and ROOT_ERR_STATUS registers. Allow AER > + * service only on root ports. Refer PCIe rev5.0 spec v1.0 7.8.4. Sec 7.8.4 talks about Root Ports and Root Complex Event Collectors together, so I would think you would treat them the same here. > + * Otherwise AER interrupt message number is read incorrectly > + * causing MSIX vector registration to fail and fallback to legacy. > + */ > if (dev->aer_cap && pci_aer_available() && > + pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT && > (pcie_ports_native || host->native_aer)) { > services |= PCIE_PORT_SERVICE_AER; > > -- > 2.25.1 >
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 3ee63968deaa..edc355971a32 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -221,7 +221,16 @@ static int get_port_device_capability(struct pci_dev *dev) } #ifdef CONFIG_PCIEAER + /* + * Some AER interrupt capability registers may not be present on + * non Root ports. Since there is no way to check presence of + * ROOT_ERR_COMMAND and ROOT_ERR_STATUS registers. Allow AER + * service only on root ports. Refer PCIe rev5.0 spec v1.0 7.8.4. + * Otherwise AER interrupt message number is read incorrectly + * causing MSIX vector registration to fail and fallback to legacy. + */ if (dev->aer_cap && pci_aer_available() && + pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT && (pcie_ports_native || host->native_aer)) { services |= PCIE_PORT_SERVICE_AER;