Re: [RESEND PATCH v2 1/4] PCI: Add setup_platform_service_irq hook to struct pci_host_bridge

["Pali Rohár" <pali@kernel.org>]

Hello!

On Wednesday 12 January 2022 10:42:48 Stefan Roese wrote:
> From: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> 
> As per section 6.2.4.1.2, 6.2.6 in PCIe r4.0 error interrupts can
> be delivered with platform specific interrupt lines.

My understanding of these sections is that they still have to be
deliverable via standard interrupts (INTx / MSI) if root port supports
standard interrupts:

"If a Root Port or Root Complex Event Collector is enabled for
level-triggered interrupt signaling using the INTx messages, the virtual
INTx wire must be asserted..."

"If a Root Port or Root Complex Event Collector is enabled for
edge-triggered interrupt signaling using MSI or MSI-X, an interrupt
message must be sent every time..."

> Add setup_platform_service_irq hook to struct pci_host_bridge.
> Some platforms have dedicated interrupt line from root complex to
> interrupt controller for PCIe services like AER.
> This hook is to register platform IRQ's to PCIe port services.
> 
> 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 | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 18a75c8e615c..291eadade811 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -554,6 +554,8 @@ struct pci_host_bridge {
>  	u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* Platform IRQ swizzler */
>  	int (*map_irq)(const struct pci_dev *, u8, u8);
>  	void (*release_fn)(struct pci_host_bridge *);
> +	void (*setup_platform_service_irq)(struct pci_host_bridge *, int *,
> +					   int);

This callback is used only for root port. So I would suggest to put
_root port_ into the name of the callback to indicate it. To distinguish
for which devices is callback designed because other callbacks (e.g.
map_irq) are used for any device.

This callback is root port specific and therefore struct pci_dev *
pointer should be passed as callback argument. Host bridge may have
multiple root ports, so passing only host bridge is not enough.

Maybe it would be better to pass struct pci_dev * instead of struct
pci_host_bridge * As from pci_dev can be easily derived host bridge.

Anyway, this callback looks to be very useful, I would like to use it in
pci-aardvark.c and pci-mvebu.c drivers for better mapping of PME, AER
and HP interrupts. And pci-mvebu.c is multi root port driver, so needs
pci_dev*.

And my guess is that this callback can be useful for adding AER support
also for pcie-uniphier.c driver, as replacement for this (rather ugly)
patches:
https://lore.kernel.org/linux-pci/1619111097-10232-1-git-send-email-hayashi.kunihiko@socionext.com/

So I would be happy to see it!

>  	void		*release_data;
>  	unsigned int	ignore_reset_delay:1;	/* For entire hierarchy */
>  	unsigned int	no_ext_tags:1;		/* No Extended Tags */
> -- 
> 2.34.1
>