[PATCH 1/6] irqchip: add the Alpine MSIX interrupt controller

Marc Zyngier
2016-02-08T09:44:49Z

Hi Antoine,

On 08/02/16 09:16, Antoine Tenart wrote:
> This patch adds the Alpine MSIX interrupt controller driver.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> Signed-off-by: Tsahee Zidenberg <tsahee@annapurnalabs.com>
> ---
> drivers/irqchip/Kconfig | 6 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-alpine-msi.c | 297 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
> create mode 100644 drivers/irqchip/irq-alpine-msi.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 715923d5236c..f20e5b28eb5f 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -60,6 +60,12 @@ config ARM_VIC_NR
> The maximum number of VICs available in the system, for
> power management.
>
> +config ALPINE_MSI
> + bool
> + depends on PCI && PCI_MSI
> + select GENERIC_IRQ_CHIP
> + select PCI_MSI_IRQ_DOMAIN
> +
> config ATMEL_AIC_IRQ
> bool
> select GENERIC_IRQ_CHIP
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 18caacb60d58..57f68e0eea74 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -1,5 +1,6 @@
> obj-$(CONFIG_IRQCHIP) += irqchip.o
>
> +obj-$(CONFIG_ALPINE_MSI) += irq-alpine-msi.o
> obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
> obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2836.o
> obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
> diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-alpine-msi.c
> new file mode 100644
> index 000000000000..435dd4ab3626
> --- /dev/null
> +++ b/drivers/irqchip/irq-alpine-msi.c
> @@ -0,0 +1,297 @@
> +/*
> + * Annapurna Labs MSIX support services
> + *
> + * Copyright (C) 2016, Amazon.com, Inc. or its affiliates. All Rights Reserved.
> + *
> + * Antoine Tenart <antoine.tenart@free-electrons.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.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/arm-gic.h>
> +#include <linux/msi.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
> +#include <linux/slab.h>
> +
> +#include <asm/irq.h>
> +#include <asm-generic/msi.h>
> +
> +/* MSIX message address format: local GIC target */
> +#define ALPINE_MSIX_SPI_TARGET_CLUSTER0 BIT(16)
> +
> +struct alpine_msix_data {
> + spinlock_t msi_map_lock;
> + u32 addr_high;
> + u32 addr_low;

As this looks to be a physical address, please consider using phys_addr_t.

> + u32 spi_first; /* The SGI number that MSIs start */
> + u32 num_spis; /* The number of SGIs for MSIs */
> + unsigned long *msi_map;
> +};
> +
> +static void alpine_msix_mask_msi_irq(struct irq_data *d)
> +{
> + pci_msi_mask_irq(d);
> + irq_chip_mask_parent(d);
> +}
> +
> +static void alpine_msix_unmask_msi_irq(struct irq_data *d)
> +{
> + pci_msi_unmask_irq(d);
> + irq_chip_unmask_parent(d);
> +}
> +
> +static int alpine_msix_set_affinity(struct irq_data *irq_data,
> + const struct cpumask *mask, bool force)
> +{
> + int ret;
> +
> + ret = irq_chip_set_affinity_parent(irq_data, mask, force);
> + return ret == IRQ_SET_MASK_OK ? IRQ_SET_MASK_OK_DONE : ret;
> +}
> +
> +static struct irq_chip alpine_msix_irq_chip = {
> + .name = "MSIx",
> + .irq_mask = alpine_msix_mask_msi_irq,
> + .irq_unmask = alpine_msix_unmask_msi_irq,
> + .irq_eoi = irq_chip_eoi_parent,
> + .irq_set_affinity = alpine_msix_set_affinity,
> +};
> +
> +static int alpine_msix_allocate_sgi(struct alpine_msix_data *priv, int num_req)
> +{
> + int first, i;
> +
> + spin_lock(&priv->msi_map_lock);
> +
> + first = bitmap_find_next_zero_area(priv->msi_map, priv->num_spis, 0,
> + num_req, 0);
> + if (first >= priv->num_spis) {
> + spin_unlock(&priv->msi_map_lock);
> + return -ENOSPC;
> + }
> +
> + for (i = 0; i < num_req; i++)
> + set_bit(first + i, priv->msi_map);
> +
> + spin_unlock(&priv->msi_map_lock);
> +
> + return priv->spi_first + first;
> +}
> +
> +static void alpine_msix_free_sgi(struct alpine_msix_data *priv, unsigned sgi,
> + int num_req)
> +{
> + int i, first;
> +
> + first = sgi - priv->spi_first;
> +
> + spin_lock(&priv->msi_map_lock);
> +
> + for (i = 0; i < num_req; i++)
> + clear_bit(first + i, priv->msi_map);
> +
> + spin_unlock(&priv->msi_map_lock);
> +}
> +
> +static void alpine_msix_compose_msi_msg(struct irq_data *data,
> + struct msi_msg *msg)
> +{
> + struct alpine_msix_data *priv = irq_data_get_irq_chip_data(data);
> +
> + msg->address_hi = priv->addr_high;
> + msg->address_lo = priv->addr_low + (data->hwirq << 3);
> + msg->data = 0;
> +}
> +
> +static struct msi_domain_info alpine_msix_domain_info = {
> + .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> + MSI_FLAG_PCI_MSIX,

You can probably add MSI_FLAG_PCI_MSI, it should work as well (MULTI_MSI
obviously won't).

> + .chip = &alpine_msix_irq_chip,
> +};
> +
> +static struct irq_chip middle_irq_chip = {
> + .name = "alpine_msix_middle",
> + .irq_mask = irq_chip_mask_parent,
> + .irq_unmask = irq_chip_unmask_parent,
> + .irq_eoi = irq_chip_eoi_parent,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> + .irq_compose_msi_msg = alpine_msix_compose_msi_msg,
> +};
> +
> +static int alpine_msix_gic_domain_alloc(struct irq_domain *domain,
> + unsigned int virq, int sgi)
> +{
> + struct irq_fwspec fwspec;
> + struct irq_data *d;
> + int ret;
> +
> + if (!is_of_node(domain->parent->fwnode))
> + return -EINVAL;
> +
> + fwspec.fwnode = domain->parent->fwnode;
> + fwspec.param_count = 3;
> + fwspec.param[0] = 0;
> + fwspec.param[1] = sgi;
> + fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> +
> + ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> + if (ret)
> + return ret;
> +
> + d = irq_domain_get_irq_data(domain->parent, virq);
> + d->chip->irq_set_type(d, IRQ_TYPE_EDGE_RISING);
> +
> + return 0;
> +}
> +
> +static int alpine_msix_middle_domain_alloc(struct irq_domain *domain,
> + unsigned int virq,
> + unsigned int nr_irqs, void *args)
> +{
> + struct alpine_msix_data *priv = domain->host_data;
> + int sgi, err, i;
> +
> + sgi = alpine_msix_allocate_sgi(priv, nr_irqs);
> + if (sgi < 0)
> + return sgi;
> +
> + for (i = 0; i < nr_irqs; i++) {
> + err = alpine_msix_gic_domain_alloc(domain, virq + i, sgi + i);
> + if (err)
> + goto err_sgi;
> +
> + irq_domain_set_hwirq_and_chip(domain, virq + i, sgi + i,
> + &middle_irq_chip, priv);
> + }
> +
> + return 0;
> +
> +err_sgi:
> + while (--i >= 0)
> + irq_domain_free_irqs_parent(domain, virq, i);
> + alpine_msix_free_sgi(priv, sgi, nr_irqs);
> + return err;
> +}
> +
> +static void alpine_msix_middle_domain_free(struct irq_domain *domain,
> + unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> + struct alpine_msix_data *priv = irq_data_get_irq_chip_data(d);
> +
> + irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> + alpine_msix_free_sgi(priv, d->hwirq, nr_irqs);
> +}
> +
> +static const struct irq_domain_ops alpine_msix_middle_domain_ops = {
> + .alloc = alpine_msix_middle_domain_alloc,
> + .free = alpine_msix_middle_domain_free,
> +};
> +
> +static int alpine_msix_init_domains(struct alpine_msix_data *priv,
> + struct device_node *node)
> +{
> + struct irq_domain *middle_domain, *msi_domain, *gic_domain;
> + struct device_node *gic_node;
> +
> + gic_node = of_irq_find_parent(node);
> + if (!gic_node) {
> + pr_err("Failed to find the GIC node\n");
> + return -ENODEV;
> + }
> +
> + gic_domain = irq_find_host(gic_node);
> + if (!gic_domain) {
> + pr_err("Failed to find the GIC domain\n");
> + return -ENXIO;
> + }
> +
> + middle_domain = irq_domain_add_tree(NULL,
> + &alpine_msix_middle_domain_ops,
> + priv);
> + if (!middle_domain) {
> + pr_err("Failed to create the MSIX middle domain\n");
> + return -ENOMEM;
> + }
> +
> + middle_domain->parent = gic_domain;
> +
> + msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(node),
> + &alpine_msix_domain_info,
> + middle_domain);
> + if (!msi_domain) {
> + pr_err("Failed to create MSI domain\n");
> + irq_domain_remove(msi_domain);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int alpine_msix_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + struct alpine_msix_data *priv;
> + struct resource res;
> + int ret;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + spin_lock_init(&priv->msi_map_lock);
> +
> + ret = of_address_to_resource(node, 0, &res);
> + if (ret) {
> + pr_err("Failed to allocate resource\n");
> + goto err_priv;
> + }
> +
> + priv->addr_high = upper_32_bits((u64)res.start);
> + priv->addr_low = lower_32_bits(res.start) + ALPINE_MSIX_SPI_TARGET_CLUSTER0;

This is a bit odd. If you always set bit 16, why isn't that reflected in
the base address coming from the DT?

> +
> + if (of_property_read_u32(node, "al,msi-base-spi", &priv->spi_first)) {
> + pr_err("Unable to parse MSI base\n");
> + ret = -EINVAL;
> + goto err_priv;
> + }
> +
> + if (of_property_read_u32(node, "al,msi-num-spis", &priv->num_spis)) {
> + pr_err("Unable to parse MSI numbers\n");
> + ret = -EINVAL;
> + goto err_priv;
> + }
> +
> + priv->msi_map = kzalloc(sizeof(*priv->msi_map) * BITS_TO_LONGS(priv->num_spis),
> + GFP_KERNEL);
> + if (!priv->msi_map) {
> + ret = -ENOMEM;
> + goto err_priv;
> + }
> +
> + pr_debug("Registering %d msixs, starting at %d\n",
> + priv->num_spis, priv->spi_first);
> +
> + ret = alpine_msix_init_domains(priv, node);
> + if (ret)
> + goto err_map;
> +
> + return 0;
> +
> +err_map:
> + kfree(priv->msi_map);
> +err_priv:
> + kfree(priv);
> + return ret;
> +}
> +IRQCHIP_DECLARE(alpine_msix, "al,alpine-msix", alpine_msix_init);
>

Otherwise, looks pretty good.

Thanks,

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