Re: [PATCH v2] PCI/portdrv: Use link bandwidth notification capability bit

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, May 13, 2021 at 03:03:14AM +0530, Stuart Hayes wrote:
> The pcieport driver can fail to attach to a downstream port that doesn't
> support bandwidth notification.  This can happen when, in
> pcie_port_device_register(), pci_init_service_irqs() tries (and fails) to
> set up a bandwidth notification IRQ for a device that doesn't support it.

I'm trying to figure out exactly how this fails.  The only failure
path in pcie_init_service_irqs() is when
pci_alloc_irq_vectors(PCI_IRQ_LEGACY) fails, which I guess means the
port's dev->irq was zero?

And to even attempt legacy IRQs, either we had pcie_pme_no_msi() or
pcie_port_enable_irq_vec() failed?

> This patch changes get_port_device_capability() to look at the link
> bandwidth notification capability bit in the link capabilities register of
> the port, instead of assuming that all downstream ports have that
> capability.

I think this needs:

  Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")

because even though b4c7d2076b4e ("PCI/LINK: Remove bandwidth
notification") removed *most* of e8303bb7a75c, it did not remove the
code in get_port_device_capability() that you're fixing here.

I can fix this up locally, no need to resend.  I think the patch
itself is fine, just trying to understand the commit log.

> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> ---
> 
> changes from v1:
> 	- corrected spelling errors in commit message
> 	- added Lukas' reviewed-by:
> 
> ---
>  drivers/pci/pcie/portdrv_core.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index e1fed6649c41..3ee63968deaa 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -257,8 +257,13 @@ static int get_port_device_capability(struct pci_dev *dev)
>  		services |= PCIE_PORT_SERVICE_DPC;
>  
>  	if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM ||
> -	    pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
> -		services |= PCIE_PORT_SERVICE_BWNOTIF;
> +	    pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
> +		u32 linkcap;
> +
> +		pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &linkcap);
> +		if (linkcap & PCI_EXP_LNKCAP_LBNC)
> +			services |= PCIE_PORT_SERVICE_BWNOTIF;
> +	}
>  
>  	return services;
>  }
> -- 
> 2.27.0
>