| Message ID | 1496135772-20694-4-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30/05/17 10:16, Thomas Petazzoni wrote: > This commit adds a simple driver for the Marvell GICP, a hardware unit > that converts memory writes into GIC SPI interrupts. The driver doesn't > do anything but clear all interrupts at boot time, to avoid spurious > interrupts left by the firmware. > > The GICP registers are directly written to in hardware by the ICU unit, > which is configured in a separate driver. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > drivers/irqchip/Kconfig | 3 +++ > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-mvebu-gicp.c | 53 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 57 insertions(+) > create mode 100644 drivers/irqchip/irq-mvebu-gicp.c > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index 478f8ac..e527ee5 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -268,6 +268,9 @@ config IRQ_MXS > select IRQ_DOMAIN > select STMP_DEVICE > > +config MVEBU_GICP > + bool > + > config MVEBU_ODMI > bool > select GENERIC_MSI_IRQ_DOMAIN > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index b64c59b..11eb858 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -69,6 +69,7 @@ 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_MVEBU_GICP) += irq-mvebu-gicp.o > obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o > obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o > obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o > diff --git a/drivers/irqchip/irq-mvebu-gicp.c b/drivers/irqchip/irq-mvebu-gicp.c > new file mode 100644 > index 0000000..4effed4 > --- /dev/null > +++ b/drivers/irqchip/irq-mvebu-gicp.c > @@ -0,0 +1,53 @@ > +/* > + * Copyright (C) 2017 Marvell > + * > + * Thomas Petazzoni <thomas.petazzoni@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. > + */ > + > +#include <linux/io.h> > +#include <linux/platform_device.h> > + > +#define GICP_SETSPI_NSR_OFFSET 0x0 > +#define GICP_CLRSPI_NSR_OFFSET 0x8 > + > +#define GICP_INT_COUNT 128 > + > +static int mvebu_gicp_probe(struct platform_device *pdev) > +{ > + void __iomem *regs; > + struct resource *res; > + int i; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -ENODEV; > + > + regs = ioremap(res->start, resource_size(res)); > + if (!regs) > + return -ENOMEM; > + > + for (i = 0; i < GICP_INT_COUNT; i++) > + writel(i, regs + GICP_CLRSPI_NSR_OFFSET); What does this do on an edge interrupt? I bet this doesn't have any effect, so you may want to use the irq_set_irqchip_state() API to clear a potential pending state instead (and you may want to wire it in the ICU driver itself as well). > + > + iounmap(regs); > + > + return 0; > +} > + > +static const struct of_device_id mvebu_gicp_of_match[] = { > + { .compatible = "marvell,gicp", }, > + {}, > +}; > + > +static struct platform_driver mvebu_gicp_driver = { > + .probe = mvebu_gicp_probe, > + .driver = { > + .name = "mvebu-gicp", > + .of_match_table = mvebu_gicp_of_match, > + }, > +}; > +builtin_platform_driver(mvebu_gicp_driver); > Thanks, M.
Hello, On Tue, 30 May 2017 14:55:57 +0100, Marc Zyngier wrote: > > + for (i = 0; i < GICP_INT_COUNT; i++) > > + writel(i, regs + GICP_CLRSPI_NSR_OFFSET); > > What does this do on an edge interrupt? I guess nothing. What the ICU does is: * For level interrupts: when the interrupt wire is asserted, write to SETNSR, when the interrupt wire is deasserted, write to CLRNSR * For edge interrupts: only the interrupt assertion causes a write to SETNSR. > I bet this doesn't have any effect Indeed. But do we care? Can an edge interrupt be left pending from the firmware? >, so you may want to use the irq_set_irqchip_state() API to clear a > potential pending state instead (and you may want to wire it in the > ICU driver itself as well). I'm not sure how to use this irq_set_irqchip_state() API. I guess it needs a virq that corresponds to the GIC SPI interrupt, and I'm not sure how to get that. Thomas
On 30/05/17 15:54, Thomas Petazzoni wrote: > Hello, > > On Tue, 30 May 2017 14:55:57 +0100, Marc Zyngier wrote: > >>> + for (i = 0; i < GICP_INT_COUNT; i++) >>> + writel(i, regs + GICP_CLRSPI_NSR_OFFSET); >> >> What does this do on an edge interrupt? > > I guess nothing. What the ICU does is: > > * For level interrupts: when the interrupt wire is asserted, write to > SETNSR, when the interrupt wire is deasserted, write to CLRNSR > > * For edge interrupts: only the interrupt assertion causes a write to > SETNSR. > >> I bet this doesn't have any effect > > Indeed. But do we care? Can an edge interrupt be left pending from the > firmware? I cannot see why not. It is just as likely as a level interrupt. > >> , so you may want to use the irq_set_irqchip_state() API to clear a >> potential pending state instead (and you may want to wire it in the >> ICU driver itself as well). > > I'm not sure how to use this irq_set_irqchip_state() API. I guess it > needs a virq that corresponds to the GIC SPI interrupt, and I'm not > sure how to get that. You do have the virtual interrupt when doing the allocation (it is passed as a parameter). So you could perform the mapping (call into the lower layers), and clear the pending bit using the above API. But maybe you don't have any edge interrupt on this SoC, and it doesn't matter. Thanks, M.
Hello, On Tue, 30 May 2017 16:17:41 +0100, Marc Zyngier wrote: > > Indeed. But do we care? Can an edge interrupt be left pending from the > > firmware? > > I cannot see why not. It is just as likely as a level interrupt. OK. > > I'm not sure how to use this irq_set_irqchip_state() API. I guess it > > needs a virq that corresponds to the GIC SPI interrupt, and I'm not > > sure how to get that. > > You do have the virtual interrupt when doing the allocation (it is > passed as a parameter). So you could perform the mapping (call into the > lower layers), and clear the pending bit using the above API. So in mvebu_icu_irq_domain_alloc(), if I do: irq_set_irqchip_state(virq, IRQCHIP_STATE_MASKED, true); this will go all the way to the ->irq_set_irqchip_state() in the GIC? I thought the virq we had was referring to an irq from the ICU domain, not from the GIC one. But maybe I'm still getting confused by all these irq domains. > But maybe you don't have any edge interrupt on this SoC, and it doesn't > matter. We currently don't have any in the devices we support in the SoC, but since the ICU does support edge interrupts explicitly, it's nicer if we can get this right. Plus if this actually works, we don't need the marvell,gicp "driver" anymore. Best regards, Thomas
On 30/05/17 16:25, Thomas Petazzoni wrote: > Hello, > > On Tue, 30 May 2017 16:17:41 +0100, Marc Zyngier wrote: > >>> Indeed. But do we care? Can an edge interrupt be left pending from the >>> firmware? >> >> I cannot see why not. It is just as likely as a level interrupt. > > OK. > >>> I'm not sure how to use this irq_set_irqchip_state() API. I guess it >>> needs a virq that corresponds to the GIC SPI interrupt, and I'm not >>> sure how to get that. >> >> You do have the virtual interrupt when doing the allocation (it is >> passed as a parameter). So you could perform the mapping (call into the >> lower layers), and clear the pending bit using the above API. > > So in mvebu_icu_irq_domain_alloc(), if I do: > > irq_se_irqchip_state(virq, IRQCHIP_STATE_MASKED, true); That would be irq_set_irqchip_state(virq, IRQCHIP_STATE_PENDING, false); instead. It is expected that the interrupt is already masked (otherwise, you could be in trouble). > this will go all the way to the ->irq_set_irqchip_state() in the GIC? I Yup. > thought the virq we had was referring to an irq from the ICU domain, > not from the GIC one. But maybe I'm still getting confused by all these > irq domains. I'm afraid you are... ;-) This is a hierarchical domain, so the virq is constant across the whole stack, only the irqchip-specific identifier (hwirq) changes (see how you get it from the core code, and propatage it to the parent domain). If you implement irq_set_irqchip_state at the ICU level by directly calling into the parent, you should be just fine. > >> But maybe you don't have any edge interrupt on this SoC, and it doesn't >> matter. > > We currently don't have any in the devices we support in the SoC, but > since the ICU does support edge interrupts explicitly, it's nicer if we > can get this right. Plus if this actually works, we don't need the > marvell,gicp "driver" anymore. That'd be great. Thanks, M.
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 478f8ac..e527ee5 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -268,6 +268,9 @@ config IRQ_MXS select IRQ_DOMAIN select STMP_DEVICE +config MVEBU_GICP + bool + config MVEBU_ODMI bool select GENERIC_MSI_IRQ_DOMAIN diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index b64c59b..11eb858 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -69,6 +69,7 @@ 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_MVEBU_GICP) += irq-mvebu-gicp.o obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o diff --git a/drivers/irqchip/irq-mvebu-gicp.c b/drivers/irqchip/irq-mvebu-gicp.c new file mode 100644 index 0000000..4effed4 --- /dev/null +++ b/drivers/irqchip/irq-mvebu-gicp.c @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2017 Marvell + * + * Thomas Petazzoni <thomas.petazzoni@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. + */ + +#include <linux/io.h> +#include <linux/platform_device.h> + +#define GICP_SETSPI_NSR_OFFSET 0x0 +#define GICP_CLRSPI_NSR_OFFSET 0x8 + +#define GICP_INT_COUNT 128 + +static int mvebu_gicp_probe(struct platform_device *pdev) +{ + void __iomem *regs; + struct resource *res; + int i; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; + + regs = ioremap(res->start, resource_size(res)); + if (!regs) + return -ENOMEM; + + for (i = 0; i < GICP_INT_COUNT; i++) + writel(i, regs + GICP_CLRSPI_NSR_OFFSET); + + iounmap(regs); + + return 0; +} + +static const struct of_device_id mvebu_gicp_of_match[] = { + { .compatible = "marvell,gicp", }, + {}, +}; + +static struct platform_driver mvebu_gicp_driver = { + .probe = mvebu_gicp_probe, + .driver = { + .name = "mvebu-gicp", + .of_match_table = mvebu_gicp_of_match, + }, +}; +builtin_platform_driver(mvebu_gicp_driver);
This commit adds a simple driver for the Marvell GICP, a hardware unit that converts memory writes into GIC SPI interrupts. The driver doesn't do anything but clear all interrupts at boot time, to avoid spurious interrupts left by the firmware. The GICP registers are directly written to in hardware by the ICU unit, which is configured in a separate driver. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/irqchip/Kconfig | 3 +++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-mvebu-gicp.c | 53 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 drivers/irqchip/irq-mvebu-gicp.c