Re: [RESEND PATCH v2 2/4] PCI: Add pci_check_platform_service_irqs

Bjorn Helgaas
2022-01-12T16:42:30Z

On Wed, Jan 12, 2022 at 10:42:49AM +0100, Stefan Roese wrote:
> From: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
>
> Adding method pci_check_platform_service_irqs to check if platform
> has registered method to proivde dedicated IRQ lines for PCIe services
> like AER.
>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Tested-by: Stefan Roese <sr@denx.de>
> Cc: Bjorn Helgaas <helgaas@kernel.org>
> Cc: Pali Rohár <pali@kernel.org>
> Cc: Michal Simek <michal.simek@xilinx.com>
> ---
> include/linux/pci.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 291eadade811..d6812d596ecc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2420,6 +2420,24 @@ static inline bool pci_ari_enabled(struct pci_bus *bus)
> return bus->self && bus->self->ari_enabled;
> }
>
> +/**
> + * pci_check_platform_service_irqs - check platform service irq's
> + * @pdev: PCI Express device to check
> + * @irqs: Array of irqs to populate
> + * @mask: Bitmask of capabilities
> + */
> +static inline void pci_check_platform_service_irqs(struct pci_dev *dev,
> + int *irqs, int mask)
> +{
> + struct pci_host_bridge *bridge;
> +
> + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
> + bridge = pci_find_host_bridge(dev->bus);
> + if (bridge && bridge->setup_platform_service_irq)
> + bridge->setup_platform_service_irq(bridge, irqs, mask);
> + }
> +}

I don't think this needs to be in include/linux/pci.h; I think it
should be in drivers/pci/pcie/portdrv_core.c where it is called.

The name and signature should be parallel to pcie_init_service_irqs()
since it is basically doing the same thing for platform-specific
interrupts.

These patches are split up a little bit *too* much. Each one should
add some piece of functionality. Currently they add declarations that
are not used, functions that are not called, etc. That makes it hard
to read one patch and get any idea of what it's for.

Bjorn