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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/12/22 11:23, Pali Rohár wrote:
Hello!

On Wednesday 12 January 2022 10:42:48 Stefan Roese wrote:
From: Bharat Kumar Gogada <bharat.kumar.gogada@xxxxxxxxxx>

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@xxxxxxxxxx>
Signed-off-by: Stefan Roese <sr@xxxxxxx>
Tested-by: Stefan Roese <sr@xxxxxxx>
Cc: Bjorn Helgaas <helgaas@xxxxxxxxxx>
Cc: Pali Rohár <pali@xxxxxxxxxx>
Cc: Michal Simek <michal.simek@xxxxxxxxxx>
---
  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.

I see your point, but I also don't want to make this function name too
long, making the code harder to read IMHO. In the next patchset version
I've changed this name to init_platform_service_irqs (added 's') to
better reflect the input parameters and its related function in
portdrv_core.c, which I now changed according to Bjorn's suggestion to
pcie_init_platform_service_irqs().

Please feel free to suggest a better matching name that is not too long
if one comes to your mind.

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.

Good idea. I'll integrate this in the next version.

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*.

Sounds like a plan.

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@xxxxxxxxxxxxx/

So I would be happy to see it!

Stay tuned...

Thanks,
Stefan



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux