Re: [PATCH RESEND v3 04/11] staging: fsl-mc: Added GICv3-ITS support for FSL-MC MSIs

[Marc Zyngier <marc.zyngier@arm.com>]

On 24/11/15 22:31, J. German Rivera wrote:
> Added platform-specific MSI support layer for FSL-MC devices.
> 
> Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
> ---
> CHANGE HISTORY
> 
> Changes in v3: none
> 
> Changes in v2: none
> 
>  drivers/staging/fsl-mc/bus/Makefile                |   1 +
>  .../staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 127 +++++++++++++++++++++
>  drivers/staging/fsl-mc/include/mc-private.h        |   4 +
>  3 files changed, 132 insertions(+)
>  create mode 100644 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> 
> diff --git a/drivers/staging/fsl-mc/bus/Makefile b/drivers/staging/fsl-mc/bus/Makefile
> index a5f2ba4..e731517 100644
> --- a/drivers/staging/fsl-mc/bus/Makefile
> +++ b/drivers/staging/fsl-mc/bus/Makefile
> @@ -14,5 +14,6 @@ mc-bus-driver-objs := mc-bus.o \
>  		      dprc-driver.o \
>  		      mc-allocator.o \
>  		      mc-msi.o \
> +		      irq-gic-v3-its-fsl-mc-msi.o \
>  		      dpmcp.o \
>  		      dpbp.o
> diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> new file mode 100644
> index 0000000..5319afa
> --- /dev/null
> +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> @@ -0,0 +1,127 @@
> +/*
> + * Freescale Management Complex (MC) bus driver MSI support
> + *
> + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> + * Author: German Rivera <German.Rivera@freescale.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include "../include/mc-private.h"
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/irqchip/arm-gic-v3.h>
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include "../include/mc-sys.h"
> +#include "dprc-cmd.h"
> +
> +static struct irq_chip its_msi_irq_chip = {
> +	.name = "fsl-mc-bus-msi",
> +	.irq_mask = irq_chip_mask_parent,
> +	.irq_unmask = irq_chip_unmask_parent,
> +	.irq_eoi = irq_chip_eoi_parent,
> +	.irq_set_affinity = msi_domain_set_affinity
> +};
> +
> +static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
> +				  struct device *dev,
> +				  int nvec, msi_alloc_info_t *info)
> +{
> +	u32 its_dev_id;
> +	struct fsl_mc_device *mc_bus_dev = to_fsl_mc_device(dev);
> +	struct msi_domain_info *msi_info;
> +
> +	if (WARN_ON(dev->bus != &fsl_mc_bus_type))
> +		return -EINVAL;

Isn't that a bit late? You've already called to_fsl_mc_device()...

> +
> +	if (WARN_ON(!(mc_bus_dev->flags & FSL_MC_IS_DPRC)))
> +		return -EINVAL;
> +
> +	/*
> +	 * Set the device Id to be passed to the GIC-ITS:
> +	 *
> +	 * NOTE: This device id corresponds to the IOMMU stream ID
> +	 * associated with the DPRC object (ICID).
> +	 */
> +	its_dev_id = mc_bus_dev->icid;
> +	info->scratchpad[0].ul = its_dev_id;

You can probably get rid of the its_dev_id variable.

> +	msi_info = msi_get_domain_info(msi_domain->parent);
> +	return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
> +}
> +
> +static struct msi_domain_ops its_fsl_mc_msi_ops = {
> +	.msi_prepare = its_fsl_mc_msi_prepare,
> +};
> +
> +static struct msi_domain_info its_fsl_mc_msi_domain_info = {
> +	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
> +	.ops	= &its_fsl_mc_msi_ops,
> +	.chip	= &its_msi_irq_chip,
> +};
> +
> +static const struct of_device_id its_device_id[] = {
> +	{	.compatible	= "arm,gic-v3-its",	},
> +	{},
> +};
> +
> +int __init its_fsl_mc_msi_init(void)
> +{
> +	struct device_node *np;
> +	struct irq_domain *parent;
> +	struct irq_domain *mc_msi_domain;
> +
> +	for (np = of_find_matching_node(NULL, its_device_id); np;
> +	     np = of_find_matching_node(np, its_device_id)) {
> +		if (!of_property_read_bool(np, "msi-controller"))
> +			continue;
> +
> +		parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
> +		if (!parent || !msi_get_domain_info(parent)) {
> +			pr_err("%s: unable to locate ITS domain\n",
> +			       np->full_name);
> +			continue;
> +		}
> +
> +		mc_msi_domain =
> +		    fsl_mc_msi_create_irq_domain(of_node_to_fwnode(np),
> +						 &its_fsl_mc_msi_domain_info,
> +						 parent);

Please keep both sides of the assignment on the same line.

> +		if (!mc_msi_domain) {
> +			pr_err("%s: unable to create fsl-mc domain\n",
> +			       np->full_name);
> +			continue;
> +		}
> +
> +		WARN_ON(mc_msi_domain->host_data !=
> +				&its_fsl_mc_msi_domain_info);

Same here.

> +
> +		pr_info("fsl-mc MSI: %s domain created\n", np->full_name);
> +	}
> +
> +	return 0;
> +}
> +
> +void its_fsl_mc_msi_cleanup(void)
> +{
> +	struct device_node *np;
> +
> +	for (np = of_find_matching_node(NULL, its_device_id); np;
> +	     np = of_find_matching_node(np, its_device_id)) {
> +		struct irq_domain *mc_msi_domain =
> +			irq_find_matching_host(np, DOMAIN_BUS_FSL_MC_MSI);

Same here.

> +
> +		if (!of_property_read_bool(np, "msi-controller"))
> +			continue;
> +
> +		mc_msi_domain =
> +			irq_find_matching_host(np, DOMAIN_BUS_FSL_MC_MSI);

And here.

> +		if (mc_msi_domain &&
> +		    mc_msi_domain->host_data == &its_fsl_mc_msi_domain_info)
> +			irq_domain_remove(mc_msi_domain);
> +	}
> +}
> diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h
> index 0b294d4..3423993 100644
> --- a/drivers/staging/fsl-mc/include/mc-private.h
> +++ b/drivers/staging/fsl-mc/include/mc-private.h
> @@ -133,4 +133,8 @@ int fsl_mc_msi_domain_alloc_irqs(struct device *dev,
> 
>  void fsl_mc_msi_domain_free_irqs(struct device *dev);
> 
> +int __init its_fsl_mc_msi_init(void);
> +
> +void its_fsl_mc_msi_cleanup(void);
> +
>  #endif /* _FSL_MC_PRIVATE_H_ */
> --
> 2.3.3
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...