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

From: Marc Zyngier
Date: Wed Dec 09 2015 - 10:34:42 EST


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@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx>
> + *
> + * 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...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/