Anup Patel <apatel@xxxxxxxxxxxxxxxx> writes: > On Tue, Oct 24, 2023 at 6:39 PM Björn Töpel <bjorn@xxxxxxxxxx> wrote: >> >> Anup Patel <apatel@xxxxxxxxxxxxxxxx> writes: >> >> > The Linux PCI framework requires it's own dedicated MSI irqdomain so >> > let us create PCI MSI irqdomain as child of the IMSIC base irqdomain. >> > >> > Signed-off-by: Anup Patel <apatel@xxxxxxxxxxxxxxxx> >> > --- >> > drivers/irqchip/Kconfig | 7 +++ >> > drivers/irqchip/irq-riscv-imsic-platform.c | 51 ++++++++++++++++++++++ >> > drivers/irqchip/irq-riscv-imsic-state.h | 1 + >> > 3 files changed, 59 insertions(+) >> > >> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig >> > index bdd80716114d..c1d69b418dfb 100644 >> > --- a/drivers/irqchip/Kconfig >> > +++ b/drivers/irqchip/Kconfig >> > @@ -552,6 +552,13 @@ config RISCV_IMSIC >> > select IRQ_DOMAIN_HIERARCHY >> > select GENERIC_MSI_IRQ >> > >> > +config RISCV_IMSIC_PCI >> > + bool >> > + depends on RISCV_IMSIC >> > + depends on PCI >> > + depends on PCI_MSI >> > + default RISCV_IMSIC >> > + >> > config EXYNOS_IRQ_COMBINER >> > bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST >> > depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST >> > diff --git a/drivers/irqchip/irq-riscv-imsic-platform.c b/drivers/irqchip/irq-riscv-imsic-platform.c >> > index 23d286cb017e..cdb659401199 100644 >> > --- a/drivers/irqchip/irq-riscv-imsic-platform.c >> > +++ b/drivers/irqchip/irq-riscv-imsic-platform.c >> > @@ -13,6 +13,7 @@ >> > #include <linux/irqdomain.h> >> > #include <linux/module.h> >> > #include <linux/msi.h> >> > +#include <linux/pci.h> >> > #include <linux/platform_device.h> >> > #include <linux/spinlock.h> >> > #include <linux/smp.h> >> > @@ -215,6 +216,42 @@ static const struct irq_domain_ops imsic_base_domain_ops = { >> > #endif >> > }; >> > >> > +#ifdef CONFIG_RISCV_IMSIC_PCI >> > + >> > +static void imsic_pci_mask_irq(struct irq_data *d) >> > +{ >> > + pci_msi_mask_irq(d); >> > + irq_chip_mask_parent(d); >> >> I've asked this before, but I still don't get why you need to propagate >> to the parent? Why isn't masking on PCI enough? >> > > We are using hierarchical IRQ domains where IMSIC-BASE is > the root domain whereas IMSIC-PLAT domain (MSI irq domain > for platform devices) and IMSIC-PCI domain (MSI irq domain > for PCI devices). For hierarchical IRQ domains, if irq domain X > does not implement irq_mask/unmask then the parent irq > domain irq_mask/unmask is called with parent irq descriptor. > > Now for IMSIC-PCI domain, the PCI framework expects the > pci_msi_mask/unmask_irq() functions to be called but if > we directly point pci_msi_mask/unmask_irq() in the IMSIC-PCI > irqchip then IMSIC-BASE (parent domain) irq_mask/umask > won't be called hence the IRQ won't be masked/unmask. > Due to this, we call both pci_msi_mask/unmask_irq() and > irq_chip_mask/unmask_parent() for IMSIC-PCI domain. Ok. I wont dig more into it for now! If the interrupt is disabled at PCI, it seems a bit overkill to *also* mask it at the IMSIC level... Björn