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 <[email protected]>
> Signed-off-by: Tsahee Zidenberg <[email protected]>
> ---
>  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 <[email protected]>
> + *
> + * 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...

Reply via email to