On Tue, Jul 21, 2020 at 09:02:28AM -0700, Dave Jiang wrote: > From: Megha Dey <megha.dey@xxxxxxxxx> > > Add support for the creation of a new DEV_MSI irq domain. It creates a > new irq chip associated with the DEV_MSI domain and adds the necessary > domain operations to it. > > Add a new config option DEV_MSI which must be enabled by any > driver that wants to support device-specific message-signaled-interrupts > outside of PCI-MSI(-X). > > Lastly, add device specific mask/unmask callbacks in addition to a write > function to the platform_msi_ops. > > Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx> > Signed-off-by: Megha Dey <megha.dey@xxxxxxxxx> > Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx> > arch/x86/include/asm/hw_irq.h | 5 ++ > drivers/base/Kconfig | 7 +++ > drivers/base/Makefile | 1 > drivers/base/dev-msi.c | 95 +++++++++++++++++++++++++++++++++++++++++ > drivers/base/platform-msi.c | 45 +++++++++++++------ > drivers/base/platform-msi.h | 23 ++++++++++ > include/linux/msi.h | 8 +++ > 7 files changed, 168 insertions(+), 16 deletions(-) > create mode 100644 drivers/base/dev-msi.c > create mode 100644 drivers/base/platform-msi.h > > diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h > index 74c12437401e..8ecd7570589d 100644 > +++ b/arch/x86/include/asm/hw_irq.h > @@ -61,6 +61,11 @@ struct irq_alloc_info { > irq_hw_number_t msi_hwirq; > }; > #endif > +#ifdef CONFIG_DEV_MSI > + struct { > + irq_hw_number_t hwirq; > + }; > +#endif Why is this in this patch? I didn't see an obvious place where it is used? > > +static void __platform_msi_desc_mask_unmask_irq(struct msi_desc *desc, u32 mask) > +{ > + const struct platform_msi_ops *ops; > + > + ops = desc->platform.msi_priv_data->ops; > + if (!ops) > + return; > + > + if (mask) { > + if (ops->irq_mask) > + ops->irq_mask(desc); > + } else { > + if (ops->irq_unmask) > + ops->irq_unmask(desc); > + } > +} > + > +void platform_msi_mask_irq(struct irq_data *data) > +{ > + __platform_msi_desc_mask_unmask_irq(irq_data_get_msi_desc(data), 1); > +} > + > +void platform_msi_unmask_irq(struct irq_data *data) > +{ > + __platform_msi_desc_mask_unmask_irq(irq_data_get_msi_desc(data), 0); > +} This is a bit convoluted, just call the op directly: void platform_msi_unmask_irq(struct irq_data *data) { const struct platform_msi_ops *ops = desc->platform.msi_priv_data->ops; if (ops->irq_unmask) ops->irq_unmask(desc); } > diff --git a/include/linux/msi.h b/include/linux/msi.h > index 7f6a8eb51aca..1da97f905720 100644 > +++ b/include/linux/msi.h > @@ -323,9 +323,13 @@ enum { > > /* > * platform_msi_ops - Callbacks for platform MSI ops > + * @irq_mask: mask an interrupt source > + * @irq_unmask: unmask an interrupt source > * @write_msg: write message content > */ > struct platform_msi_ops { > + unsigned int (*irq_mask)(struct msi_desc *desc); > + unsigned int (*irq_unmask)(struct msi_desc *desc); Why do these functions return things if the only call site throws it away? Jason