Messages in this thread | | | Subject | Re: [PATCH 2/2 v3] irqchip/Layerscape: Add SCFG MSI controller support | From | Marc Zyngier <> | Date | Wed, 17 Feb 2016 17:29:59 +0000 |
| |
Minguan,
Please CC LKML for any irqchip related patch - see the MAINTAINER file.
On 02/02/16 09:00, Minghuan Lian wrote: > Some kind of NXP Layerscape SoC provides a MSI > implementation which uses two SCFG registers MSIIR and > MSIR to support 32 MSI interrupts for each PCIe controller. > The patch is to support it. > > Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com> > --- > > Change log > v3: > 1. call of_node_to_fwnode() > v2: > 1. rename ls1-msi to ls-scfg-msi > 2. remove reg-names MSIIR MSIR > 3. remove calling set_irq_flags() > > drivers/irqchip/Kconfig | 5 + > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-ls-scfg-msi.c | 244 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 250 insertions(+) > create mode 100644 drivers/irqchip/irq-ls-scfg-msi.c > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index fb50911..0f2a3c3 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -218,3 +218,8 @@ config IRQ_MXS > def_bool y if MACH_ASM9260 || ARCH_MXS > select IRQ_DOMAIN > select STMP_DEVICE > + > +config LS_SCFG_MSI > + def_bool y if SOC_LS1021A || ARCH_LAYERSCAPE > + depends on PCI && PCI_MSI > + select PCI_MSI_IRQ_DOMAIN > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index 18caacb..37e12de 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -59,3 +59,4 @@ obj-$(CONFIG_ARCH_SA1100) += irq-sa11x0.o > obj-$(CONFIG_INGENIC_IRQ) += irq-ingenic.o > obj-$(CONFIG_IMX_GPCV2) += irq-imx-gpcv2.o > obj-$(CONFIG_PIC32_EVIC) += irq-pic32-evic.o > +obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o > diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c > new file mode 100644 > index 0000000..f57a72c > --- /dev/null > +++ b/drivers/irqchip/irq-ls-scfg-msi.c > @@ -0,0 +1,244 @@ > +/* > + * NXP SCFG MSI(-X) support > + * > + * Copyright (C) 2016 NXP Semiconductor. > + * > + * Author: Minghuan Lian <Minghuan.Lian@nxp.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/msi.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/irqchip/chained_irq.h> > +#include <linux/irqdomain.h> > +#include <linux/of_pci.h> > +#include <linux/of_platform.h> > +#include <linux/spinlock.h> > + > +#define MSI_MAX_IRQS 32 > +#define MSI_IBS_SHIFT 3 > +#define MSIR 4 > + > +struct ls_scfg_msi { > + spinlock_t lock; > + struct platform_device *pdev; > + struct irq_domain *parent; > + struct irq_domain *msi_domain; > + void __iomem *regs; > + phys_addr_t msiir_addr; > + u32 nr_irqs; > + int irq; > + DECLARE_BITMAP(used, MSI_MAX_IRQS); > +}; > + > +static struct irq_chip ls_scfg_msi_irq_chip = { > + .name = "MSI", > + .irq_enable = pci_msi_unmask_irq, > + .irq_disable = pci_msi_mask_irq, > + .irq_mask = pci_msi_mask_irq, > + .irq_unmask = pci_msi_unmask_irq, > +};
You don't need all of this. mask/unmask are enough.
> + > +static struct msi_domain_info ls_scfg_msi_domain_info = { > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | > + MSI_FLAG_USE_DEF_CHIP_OPS | > + MSI_FLAG_PCI_MSIX), > + .chip = &ls_scfg_msi_irq_chip, > +}; > + > +static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg) > +{ > + struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(data); > + > + msg->address_hi = upper_32_bits(msi_data->msiir_addr); > + msg->address_lo = lower_32_bits(msi_data->msiir_addr); > + msg->data = data->hwirq << MSI_IBS_SHIFT; > +} > + > +static int ls_scfg_msi_set_affinity(struct irq_data *irq_data, > + const struct cpumask *mask, bool force) > +{ > + return -EINVAL; > +} > + > +static struct irq_chip ls_scfg_msi_parent_chip = { > + .name = "LS SCFG MSI",
Worth making this shorter. "SCFG" should be enough.
> + .irq_compose_msi_msg = ls_scfg_msi_compose_msg, > + .irq_set_affinity = ls_scfg_msi_set_affinity, > +}; > + > +static int ls_scfg_msi_domain_irq_alloc(struct irq_domain *domain, > + unsigned int virq, > + unsigned int nr_irqs, > + void *args) > +{ > + struct ls_scfg_msi *msi_data = domain->host_data; > + int pos, err = 0; > + > + WARN_ON(nr_irqs != 1); > + > + spin_lock(&msi_data->lock); > + pos = find_first_zero_bit(msi_data->used, msi_data->nr_irqs); > + if (pos < msi_data->nr_irqs) > + __set_bit(pos, msi_data->used); > + else > + err = -ENOSPC; > + spin_unlock(&msi_data->lock); > + > + if (err) > + return err; > + > + irq_domain_set_info(domain, virq, pos, > + &ls_scfg_msi_parent_chip, msi_data, > + handle_simple_irq, NULL, NULL); > + > + return 0; > +} > + > +static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain, > + unsigned int virq, unsigned int nr_irqs) > +{ > + struct irq_data *d = irq_domain_get_irq_data(domain, virq); > + struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(d); > + int pos; > + > + pos = d->hwirq; > + if (pos < 0 || pos >= msi_data->nr_irqs) { > + pr_err("failed to teardown msi. Invalid hwirq %d\n", pos); > + return; > + } > + > + spin_lock(&msi_data->lock); > + __clear_bit(pos, msi_data->used); > + spin_unlock(&msi_data->lock); > +} > + > +static const struct irq_domain_ops ls_scfg_msi_domain_ops = { > + .alloc = ls_scfg_msi_domain_irq_alloc, > + .free = ls_scfg_msi_domain_irq_free, > +}; > + > +static void ls_scfg_msi_irq_handler(struct irq_desc *desc) > +{ > + struct ls_scfg_msi *msi_data = irq_desc_get_handler_data(desc); > + unsigned long val; > + int pos, virq; > + > + chained_irq_enter(irq_desc_get_chip(desc), desc); > + > + val = ioread32be(msi_data->regs + MSIR);
Is the device guaranteed to always be BE?
> + for_each_set_bit(pos, &val, msi_data->nr_irqs) { > + virq = irq_find_mapping(msi_data->parent, (31 - pos)); > + if (virq) > + generic_handle_irq(virq); > + } > + > + chained_irq_exit(irq_desc_get_chip(desc), desc); > +} > + > +static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data) > +{ > + /* Initialize MSI domain parent */ > + msi_data->parent = irq_domain_add_linear(NULL, > + msi_data->nr_irqs, > + &ls_scfg_msi_domain_ops, > + msi_data); > + if (!msi_data->parent) { > + dev_err(&msi_data->pdev->dev, "failed to create IRQ domain\n"); > + return -ENOMEM; > + } > + > + msi_data->msi_domain = pci_msi_create_irq_domain( > + of_node_to_fwnode(msi_data->pdev->dev.of_node), > + &ls_scfg_msi_domain_info, > + msi_data->parent); > + if (!msi_data->msi_domain) { > + dev_err(&msi_data->pdev->dev, "failed to create MSI domain\n"); > + irq_domain_remove(msi_data->parent); > + return -ENOMEM; > + } > + > + return 0; > +} > + > +static int ls_scfg_msi_probe(struct platform_device *pdev) > +{ > + struct ls_scfg_msi *msi_data; > + struct resource *res; > + int ret; > + > + msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data), GFP_KERNEL); > + if (!msi_data) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + msi_data->regs = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(msi_data->regs)) { > + dev_err(&pdev->dev, "failed to initialize 'regs'\n"); > + return PTR_ERR(msi_data->regs); > + } > + msi_data->msiir_addr = res->start; > + > + msi_data->irq = platform_get_irq(pdev, 0); > + if (msi_data->irq <= 0) { > + dev_err(&pdev->dev, "failed to get MSI irq\n"); > + return -ENODEV; > + } > + > + msi_data->pdev = pdev; > + msi_data->nr_irqs = MSI_MAX_IRQS;
So this is hardcoded, always. Why do you need a nr_irqs variable at all?
> + spin_lock_init(&msi_data->lock); > + > + ret = ls_scfg_msi_domains_init(msi_data); > + if (ret) > + return ret; > + > + irq_set_chained_handler_and_data(msi_data->irq, > + ls_scfg_msi_irq_handler, > + msi_data); > + > + platform_set_drvdata(pdev, msi_data); > + > + return 0; > +} > + > +static int ls_scfg_msi_remove(struct platform_device *pdev) > +{ > + struct ls_scfg_msi *msi_data = platform_get_drvdata(pdev); > + > + irq_set_chained_handler_and_data(msi_data->irq, NULL, NULL); > + > + irq_domain_remove(msi_data->msi_domain); > + irq_domain_remove(msi_data->parent); > + > + platform_set_drvdata(pdev, NULL); > + > + return 0; > +} > + > +static const struct of_device_id ls_scfg_msi_id[] = { > + { .compatible = "fsl,1s1021a-msi", }, > + { .compatible = "fsl,1s1043a-msi", }, > + {}, > +}; > + > +static struct platform_driver ls_scfg_msi_driver = { > + .driver = { > + .name = "ls-scfg-msi", > + .of_match_table = ls_scfg_msi_id, > + }, > + .probe = ls_scfg_msi_probe, > + .remove = ls_scfg_msi_remove, > +}; > + > +module_platform_driver(ls_scfg_msi_driver); > + > +MODULE_AUTHOR("Minghuan Lian <Minghuan.Lian@nxp.com>"); > +MODULE_DESCRIPTION("NXP Layerscape SCFG MSI controller driver"); > +MODULE_LICENSE("GPL v2"); >
Thanks,
M. -- Jazz is not dead. It just smells funny...
| |