Hi Jason, On 7/21/2020 9:13 AM, Jason Gunthorpe wrote:
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; + }; +#endifWhy is this in this patch? I didn't see an obvious place where it is used?
Since I have introduced the DEV-MSI domain and related ops, this is required in the dev_msi_set_hwirq and dev_msi_set_desc in this patch.
+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); }
Sure, I will update this.
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?
Hmmm, fair enough, I will change it to void.
Jason
