Messages in this thread | | | Subject | Re: [PATCH 1/6] irqchip: add the Alpine MSIX interrupt controller | From | Marc Zyngier <> | Date | Mon, 8 Feb 2016 09:44:49 +0000 |
| |
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...
| |