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

[Frank Li]

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;
>  }