Re: [patch V2 33/40] irqchip/imx-mu-msi: Switch to MSI parent

From: Frank Li
Date: Mon Nov 28 2022 - 15:48:05 EST


From: Frank Li <Frank.li@xxxxxxx>

On Mon, Nov 21, 2022 at 03:40:09PM +0100, Thomas Gleixner wrote:
> All platform MSI users and the PCI/MSI code handle per device MSI domains
> when the irqdomain associated to the device provides MSI parent
> functionality.
>
> Remove the "global" platform domain related code and provide the MSI parent
> functionality by filling in msi_parent_ops.
>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Marc Zyngier <maz@xxxxxxxxxx>
> Cc: Shawn Guo <shawnguo@xxxxxxxxxx>
> Cc: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> Cc: Fabio Estevam <festevam@xxxxxxxxx>
> ---
> drivers/irqchip/Kconfig | 1
> drivers/irqchip/irq-gic-msi-lib.c | 2 +
> drivers/irqchip/irq-imx-mu-msi.c | 53 +++++++++++++++-----------------------
> 3 files changed, 25 insertions(+), 31 deletions(-)
>
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -494,6 +494,7 @@ config IMX_MU_MSI
> select IRQ_DOMAIN
> select IRQ_DOMAIN_HIERARCHY
> select GENERIC_MSI_IRQ
> + select ARM_GIC_MSI_LIB
> help
> Provide a driver for the i.MX Messaging Unit block used as a
> CPU-to-CPU MSI controller. This requires a specially crafted DT
> --- a/drivers/irqchip/irq-gic-msi-lib.c
> +++ b/drivers/irqchip/irq-gic-msi-lib.c
> @@ -90,6 +90,8 @@ bool gic_msi_lib_init_dev_msi_info(struc
> /* Chip updates for all child bus types */
> if (!info->chip->irq_eoi)
> info->chip->irq_eoi = irq_chip_eoi_parent;
> + if (!info->chip->irq_ack)
> + info->chip->irq_ack = irq_chip_ack_parent;
>
> /*
> * The device MSI domain can never have a set affinity callback it
> --- a/drivers/irqchip/irq-imx-mu-msi.c
> +++ b/drivers/irqchip/irq-imx-mu-msi.c
> @@ -24,6 +24,8 @@
> #include <linux/pm_domain.h>
> #include <linux/spinlock.h>
>
> +#include "irq-gic-msi-lib.h"
> +

I think irq-gic-msi-lib.h is not good name. Actually mu-msi is not arm gic.
irq-gic-msi-lib do common work, which not related arm gic at all.

> #define IMX_MU_CHANS 4
>
> enum imx_mu_xcr {
> @@ -114,20 +116,6 @@ static void imx_mu_msi_parent_ack_irq(st
> imx_mu_read(msi_data, msi_data->cfg->xRR + data->hwirq * 4);
> }
>
> -static struct irq_chip imx_mu_msi_irq_chip = {
> - .name = "MU-MSI",
> - .irq_ack = irq_chip_ack_parent,
> -};
> -
> -static struct msi_domain_ops imx_mu_msi_irq_ops = {
> -};
> -
> -static struct msi_domain_info imx_mu_msi_domain_info = {
> - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
> - .ops = &imx_mu_msi_irq_ops,
> - .chip = &imx_mu_msi_irq_chip,
> -};
> -
> static void imx_mu_msi_parent_compose_msg(struct irq_data *data,
> struct msi_msg *msg)
> {
> @@ -195,6 +183,7 @@ static void imx_mu_msi_domain_irq_free(s
> }
>
> static const struct irq_domain_ops imx_mu_msi_domain_ops = {
> + .select = gic_msi_lib_irq_domain_select,
> .alloc = imx_mu_msi_domain_irq_alloc,
> .free = imx_mu_msi_domain_irq_free,
> };
> @@ -216,35 +205,37 @@ static void imx_mu_msi_irq_handler(struc
> chained_irq_exit(chip, desc);
> }
>
> +#define IMX_MU_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
> + MSI_FLAG_USE_DEF_CHIP_OPS | \
> + MSI_FLAG_PARENT_PM_DEV)
> +
> +#define IMX_MU_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK)
> +
> +static const struct msi_parent_ops imx_mu_msi_parent_ops = {
> + .supported_flags = IMX_MU_MSI_FLAGS_SUPPORTED,
> + .required_flags = IMX_MU_MSI_FLAGS_REQUIRED,
> + .bus_select_token = DOMAIN_BUS_NEXUS,
> + .bus_select_mask = MATCH_PLATFORM_MSI,
> + .prefix = "MU-MSI-",
> + .init_dev_msi_info = gic_msi_lib_init_dev_msi_info,
> +};
> +
> static int imx_mu_msi_domains_init(struct imx_mu_msi *msi_data, struct device *dev)
> {
> struct fwnode_handle *fwnodes = dev_fwnode(dev);
> struct irq_domain *parent;
>
> /* Initialize MSI domain parent */
> - parent = irq_domain_create_linear(fwnodes,
> - IMX_MU_CHANS,
> - &imx_mu_msi_domain_ops,
> - msi_data);
> + parent = irq_domain_create_linear(fwnodes, IMX_MU_CHANS, &imx_mu_msi_domain_ops, msi_data);

coding style change should be in sperated patch.

> if (!parent) {
> dev_err(dev, "failed to create IRQ domain\n");
> return -ENOMEM;
> }
>
> irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
> -
> - msi_data->msi_domain = platform_msi_create_irq_domain(fwnodes,
> - &imx_mu_msi_domain_info,
> - parent);
> -
> - if (!msi_data->msi_domain) {
> - dev_err(dev, "failed to create MSI domain\n");
> - irq_domain_remove(parent);
> - return -ENOMEM;
> - }
> -
> - irq_domain_set_pm_device(msi_data->msi_domain, dev);
> -
> + parent->dev = parent->pm_dev = dev;
> + parent->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
> + parent->msi_parent_ops = &imx_mu_msi_parent_ops;
> return 0;
> }