diff mbox

[v3,08/12] irqchip: add J-Core AIC driver

Message ID 05a5be0a4f1471272dcdd2259e4d97642ee43759.1464148904.git.dalias@libc.org (mailing list archive)
State New, archived
Headers show

Commit Message

Rich Felker May 25, 2016, 5:43 a.m. UTC
There are two versions of the J-Core interrupt controller in use, aic1
which generates interrupts with programmable priorities, but only
supports 8 irq lines and maps them to cpu traps in the range 17 to 24,
and aic2 which uses traps in the range 64-127 and supports up to 128
irqs, with priorities dependent on the interrupt number. The Linux
driver does not make use of priorities anyway.

For simplicity, there is no aic1-specific logic in the driver beyond
setting the priority register, which is necessary for interrupts to
work at all. Eventually aic1 will likely be phased out, but it's
currently in use in deployments and all released bitstream binaries.

Signed-off-by: Rich Felker <dalias@libc.org>
---

This version fixes a missing "static" reported in the previous
version of the patch. I missed Marc Zyngier's reply to the v2 when
preparing the v3 patch series, but I'm still not sure exactly what, if
any, changes should come out of that discussion, and they'll probably
mostly in the area of comments or changelog.

 drivers/irqchip/Kconfig         |  6 +++
 drivers/irqchip/Makefile        |  1 +
 drivers/irqchip/irq-jcore-aic.c | 95 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 drivers/irqchip/irq-jcore-aic.c

Comments

Rich Felker July 15, 2016, 1:27 a.m. UTC | #1
On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> There are two versions of the J-Core interrupt controller in use, aic1
> which generates interrupts with programmable priorities, but only
> supports 8 irq lines and maps them to cpu traps in the range 17 to 24,
> and aic2 which uses traps in the range 64-127 and supports up to 128
> irqs, with priorities dependent on the interrupt number. The Linux
> driver does not make use of priorities anyway.
> 
> For simplicity, there is no aic1-specific logic in the driver beyond
> setting the priority register, which is necessary for interrupts to
> work at all. Eventually aic1 will likely be phased out, but it's
> currently in use in deployments and all released bitstream binaries.

Any comments on changes I should make in resubmitting this for the 4.8
merge window? I could somewhat restrict the possible irqs, but aic1
allows the pit to be programmed to generate any trap number you want,
despite the hard-wired interrupts being limited to the range 17-24.

It might make sense to do something to allow percpu irq requests, but
I couldn't figure out how to make them work and the current timer
driver (that needs per-cpu irq delivery) just requests its irq through
normal request_irq and lets the hardware do the right thing.

One possible change would be allowing non-identity mapping between
hardware irq numbers (cpu trap numbers) and virqs, but then some
facility for setting up a mapping/dispatch function in do_IRQ is
needed.

Since the driver (and aic device) currently depends to some extent on
SH arch trap numbers and how the SH irq entry point passes them on
(identity mapping), should this driver just go in arch/sh/drivers
rather than drivers/irqchip?

Rich


> Signed-off-by: Rich Felker <dalias@libc.org>
> ---
> 
> This version fixes a missing "static" reported in the previous
> version of the patch. I missed Marc Zyngier's reply to the v2 when
> preparing the v3 patch series, but I'm still not sure exactly what, if
> any, changes should come out of that discussion, and they'll probably
> mostly in the area of comments or changelog.
> 
>  drivers/irqchip/Kconfig         |  6 +++
>  drivers/irqchip/Makefile        |  1 +
>  drivers/irqchip/irq-jcore-aic.c | 95 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+)
>  create mode 100644 drivers/irqchip/irq-jcore-aic.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 3e12479..3cb37d6 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -149,6 +149,12 @@ config PIC32_EVIC
>  	select GENERIC_IRQ_CHIP
>  	select IRQ_DOMAIN
>  
> +config JCORE_AIC
> +	bool "J-Core integrated AIC"
> +	select IRQ_DOMAIN
> +	help
> +	  Support for the J-Core integrated AIC.
> +
>  config RENESAS_INTC_IRQPIN
>  	bool
>  	select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b03cfcb..5a1f1bf 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_I8259)			+= irq-i8259.o
>  obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
>  obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
>  obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
> +obj-$(CONFIG_JCORE_AIC)			+= irq-jcore-aic.o
>  obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
>  obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
>  obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
> diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
> new file mode 100644
> index 0000000..1348cdb
> --- /dev/null
> +++ b/drivers/irqchip/irq-jcore-aic.c
> @@ -0,0 +1,95 @@
> +/*
> + * J-Core SoC AIC driver
> + *
> + * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/io.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +#define AIC1_INTPRI 8
> +
> +static struct aic_data {
> +	unsigned char __iomem *base;
> +	u32 cpu_offset;
> +	struct irq_chip chip;
> +	struct irq_domain *domain;
> +	struct notifier_block nb;
> +} aic_data;
> +
> +static int aic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
> +{
> +	struct aic_data *aic = d->host_data;
> +
> +	irq_set_chip_data(irq, aic);
> +	irq_set_chip_and_handler(irq, &aic->chip, handle_simple_irq);
> +	irq_set_probe(irq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops aic_irqdomain_ops = {
> +	.map = aic_irqdomain_map,
> +	.xlate = irq_domain_xlate_onecell,
> +};
> +
> +static void noop(struct irq_data *data)
> +{
> +}
> +
> +static void aic1_localenable(struct aic_data *aic)
> +{
> +	unsigned cpu = smp_processor_id();
> +	pr_info("Local AIC enable on cpu %u\n", cpu);
> +	writel(0xffffffff, aic->base + cpu * aic->cpu_offset + AIC1_INTPRI);
> +}
> +
> +static int aic1_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
> +{
> +	switch (action & ~CPU_TASKS_FROZEN) {
> +	case CPU_STARTING:
> +		aic1_localenable(container_of(self, struct aic_data, nb));
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}
> +
> +int __init aic_irq_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	struct aic_data *aic = &aic_data;
> +
> +	aic->base = of_iomap(node, 0);
> +	of_property_read_u32(node, "cpu-offset", &aic->cpu_offset);
> +
> +	pr_info("Initializing J-Core AIC at %p\n", aic->base);
> +
> +	if (of_device_is_compatible(node, "jcore,aic1")) {
> +		/* For aic1, need to enabled zero-priority-by-default irqs */
> +		aic->nb.notifier_call = aic1_cpu_notify;
> +		register_cpu_notifier(&aic->nb);
> +		aic1_localenable(aic);
> +	}
> +
> +	aic->chip.name = node->name;
> +	aic->chip.irq_mask = noop;
> +	aic->chip.irq_unmask = noop;
> +
> +	aic->domain = irq_domain_add_linear(node, 128, &aic_irqdomain_ops, aic);
> +	irq_create_strict_mappings(aic->domain, 16, 16, 112);
> +
> +	return 0;
> +}
> +
> +IRQCHIP_DECLARE(jcore_aic2, "jcore,aic2", aic_irq_of_init);
> +IRQCHIP_DECLARE(jcore_aic1, "jcore,aic1", aic_irq_of_init);
> -- 
> 2.8.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Gortmaker July 15, 2016, 3:35 p.m. UTC | #2
On Wed, May 25, 2016 at 1:43 AM, Rich Felker <dalias@libc.org> wrote:
> There are two versions of the J-Core interrupt controller in use, aic1
> which generates interrupts with programmable priorities, but only
> supports 8 irq lines and maps them to cpu traps in the range 17 to 24,
> and aic2 which uses traps in the range 64-127 and supports up to 128
> irqs, with priorities dependent on the interrupt number. The Linux
> driver does not make use of priorities anyway.
>
> For simplicity, there is no aic1-specific logic in the driver beyond
> setting the priority register, which is necessary for interrupts to
> work at all. Eventually aic1 will likely be phased out, but it's
> currently in use in deployments and all released bitstream binaries.
>
> Signed-off-by: Rich Felker <dalias@libc.org>
> ---
>
> This version fixes a missing "static" reported in the previous
> version of the patch. I missed Marc Zyngier's reply to the v2 when
> preparing the v3 patch series, but I'm still not sure exactly what, if
> any, changes should come out of that discussion, and they'll probably
> mostly in the area of comments or changelog.
>
>  drivers/irqchip/Kconfig         |  6 +++
>  drivers/irqchip/Makefile        |  1 +
>  drivers/irqchip/irq-jcore-aic.c | 95 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+)
>  create mode 100644 drivers/irqchip/irq-jcore-aic.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 3e12479..3cb37d6 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -149,6 +149,12 @@ config PIC32_EVIC
>         select GENERIC_IRQ_CHIP
>         select IRQ_DOMAIN
>
> +config JCORE_AIC
> +       bool "J-Core integrated AIC"
> +       select IRQ_DOMAIN
> +       help
> +         Support for the J-Core integrated AIC.
> +
>  config RENESAS_INTC_IRQPIN
>         bool
>         select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b03cfcb..5a1f1bf 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_I8259)                   += irq-i8259.o
>  obj-$(CONFIG_IMGPDC_IRQ)               += irq-imgpdc.o
>  obj-$(CONFIG_IRQ_MIPS_CPU)             += irq-mips-cpu.o
>  obj-$(CONFIG_SIRF_IRQ)                 += irq-sirfsoc.o
> +obj-$(CONFIG_JCORE_AIC)                        += irq-jcore-aic.o
>  obj-$(CONFIG_RENESAS_INTC_IRQPIN)      += irq-renesas-intc-irqpin.o
>  obj-$(CONFIG_RENESAS_IRQC)             += irq-renesas-irqc.o
>  obj-$(CONFIG_VERSATILE_FPGA_IRQ)       += irq-versatile-fpga.o
> diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
> new file mode 100644
> index 0000000..1348cdb
> --- /dev/null
> +++ b/drivers/irqchip/irq-jcore-aic.c
> @@ -0,0 +1,95 @@
> +/*
> + * J-Core SoC AIC driver
> + *
> + * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/io.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>

Please don't use module.h or MODULE_* in code that is controlled by
a bool Kconfig.

Thanks,
Paul.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rich Felker July 15, 2016, 3:41 p.m. UTC | #3
On Fri, Jul 15, 2016 at 11:35:55AM -0400, Paul Gortmaker wrote:
> > +++ b/drivers/irqchip/irq-jcore-aic.c
> > @@ -0,0 +1,95 @@
> > +/*
> > + * J-Core SoC AIC driver
> > + *
> > + * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License.  See the file "COPYING" in the main directory of this archive
> > + * for more details.
> > + */
> > +
> > +#include <linux/irq.h>
> > +#include <linux/io.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/module.h>
> 
> Please don't use module.h or MODULE_* in code that is controlled by
> a bool Kconfig.

Thanks for catching this. Fixed.

Rich
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Cooper July 15, 2016, 4:19 p.m. UTC | #4
Hi Rich,

On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> There are two versions of the J-Core interrupt controller in use, aic1
> which generates interrupts with programmable priorities, but only
> supports 8 irq lines and maps them to cpu traps in the range 17 to 24,
> and aic2 which uses traps in the range 64-127 and supports up to 128
> irqs, with priorities dependent on the interrupt number. The Linux
> driver does not make use of priorities anyway.
> 
> For simplicity, there is no aic1-specific logic in the driver beyond
> setting the priority register, which is necessary for interrupts to
> work at all. Eventually aic1 will likely be phased out, but it's
> currently in use in deployments and all released bitstream binaries.
> 
> Signed-off-by: Rich Felker <dalias@libc.org>
> ---
> 
> This version fixes a missing "static" reported in the previous
> version of the patch. I missed Marc Zyngier's reply to the v2 when
> preparing the v3 patch series, but I'm still not sure exactly what, if
> any, changes should come out of that discussion, and they'll probably
> mostly in the area of comments or changelog.
> 
>  drivers/irqchip/Kconfig         |  6 +++
>  drivers/irqchip/Makefile        |  1 +
>  drivers/irqchip/irq-jcore-aic.c | 95 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+)
>  create mode 100644 drivers/irqchip/irq-jcore-aic.c

Thanks for the ping, this slipped off my plate while getting back up to
speed... :-/

> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 3e12479..3cb37d6 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -149,6 +149,12 @@ config PIC32_EVIC
>  	select GENERIC_IRQ_CHIP
>  	select IRQ_DOMAIN
>  
> +config JCORE_AIC
> +	bool "J-Core integrated AIC"
> +	select IRQ_DOMAIN
> +	help
> +	  Support for the J-Core integrated AIC.
> +
>  config RENESAS_INTC_IRQPIN
>  	bool
>  	select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b03cfcb..5a1f1bf 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_I8259)			+= irq-i8259.o
>  obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
>  obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
>  obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
> +obj-$(CONFIG_JCORE_AIC)			+= irq-jcore-aic.o
>  obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
>  obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
>  obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
> diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
> new file mode 100644
> index 0000000..1348cdb
> --- /dev/null
> +++ b/drivers/irqchip/irq-jcore-aic.c
> @@ -0,0 +1,95 @@
> +/*
> + * J-Core SoC AIC driver
> + *
> + * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/io.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +#define AIC1_INTPRI 8
> +
> +static struct aic_data {
> +	unsigned char __iomem *base;

void __iomem *base;

> +	u32 cpu_offset;
> +	struct irq_chip chip;
> +	struct irq_domain *domain;
> +	struct notifier_block nb;
> +} aic_data;
> +
> +static int aic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
> +{
> +	struct aic_data *aic = d->host_data;
> +
> +	irq_set_chip_data(irq, aic);
> +	irq_set_chip_and_handler(irq, &aic->chip, handle_simple_irq);
> +	irq_set_probe(irq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops aic_irqdomain_ops = {
> +	.map = aic_irqdomain_map,
> +	.xlate = irq_domain_xlate_onecell,
> +};
> +
> +static void noop(struct irq_data *data)
> +{
> +}
> +
> +static void aic1_localenable(struct aic_data *aic)
> +{
> +	unsigned cpu = smp_processor_id();
> +	pr_info("Local AIC enable on cpu %u\n", cpu);
> +	writel(0xffffffff, aic->base + cpu * aic->cpu_offset + AIC1_INTPRI);
> +}
> +
> +static int aic1_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
> +{
> +	switch (action & ~CPU_TASKS_FROZEN) {
> +	case CPU_STARTING:
> +		aic1_localenable(container_of(self, struct aic_data, nb));
> +		break;
> +	}
> +	return NOTIFY_OK;
> +}

Please take a look at the series posted by Anna-Maria Gleixner for
recent changes to the cpu hp state machine.

  https://lkml.kernel.org/r/20160713153333.416260485@linutronix.de


> +
> +int __init aic_irq_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	struct aic_data *aic = &aic_data;
> +
> +	aic->base = of_iomap(node, 0);
> +	of_property_read_u32(node, "cpu-offset", &aic->cpu_offset);
> +
> +	pr_info("Initializing J-Core AIC at %p\n", aic->base);
> +
> +	if (of_device_is_compatible(node, "jcore,aic1")) {
> +		/* For aic1, need to enabled zero-priority-by-default irqs */
> +		aic->nb.notifier_call = aic1_cpu_notify;
> +		register_cpu_notifier(&aic->nb);
> +		aic1_localenable(aic);
> +	}
> +
> +	aic->chip.name = node->name;
> +	aic->chip.irq_mask = noop;
> +	aic->chip.irq_unmask = noop;
> +
> +	aic->domain = irq_domain_add_linear(node, 128, &aic_irqdomain_ops, aic);
> +	irq_create_strict_mappings(aic->domain, 16, 16, 112);
> +
> +	return 0;
> +}
> +
> +IRQCHIP_DECLARE(jcore_aic2, "jcore,aic2", aic_irq_of_init);
> +IRQCHIP_DECLARE(jcore_aic1, "jcore,aic1", aic_irq_of_init);

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rich Felker July 15, 2016, 5:02 p.m. UTC | #5
On Fri, Jul 15, 2016 at 04:19:17PM +0000, Jason Cooper wrote:
> > +	u32 cpu_offset;
> > +	struct irq_chip chip;
> > +	struct irq_domain *domain;
> > +	struct notifier_block nb;
> > +} aic_data;
> > +
> > +static int aic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
> > +{
> > +	struct aic_data *aic = d->host_data;
> > +
> > +	irq_set_chip_data(irq, aic);
> > +	irq_set_chip_and_handler(irq, &aic->chip, handle_simple_irq);
> > +	irq_set_probe(irq);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct irq_domain_ops aic_irqdomain_ops = {
> > +	.map = aic_irqdomain_map,
> > +	.xlate = irq_domain_xlate_onecell,
> > +};
> > +
> > +static void noop(struct irq_data *data)
> > +{
> > +}
> > +
> > +static void aic1_localenable(struct aic_data *aic)
> > +{
> > +	unsigned cpu = smp_processor_id();
> > +	pr_info("Local AIC enable on cpu %u\n", cpu);
> > +	writel(0xffffffff, aic->base + cpu * aic->cpu_offset + AIC1_INTPRI);
> > +}
> > +
> > +static int aic1_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
> > +{
> > +	switch (action & ~CPU_TASKS_FROZEN) {
> > +	case CPU_STARTING:
> > +		aic1_localenable(container_of(self, struct aic_data, nb));
> > +		break;
> > +	}
> > +	return NOTIFY_OK;
> > +}
> 
> Please take a look at the series posted by Anna-Maria Gleixner for
> recent changes to the cpu hp state machine.
> 
>   https://lkml.kernel.org/r/20160713153333.416260485@linutronix.de

I saw this but was utterly confused about what's going on. Is the
general notifier system that doesn't need to be aware of specific
devices actually being replaced by a huge enum table of all possible
devices that need special treatment as cpu starting/dying?? Or do I
misunderstand what's going on?

Rich
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rich Felker July 15, 2016, 6:19 p.m. UTC | #6
On Thu, Jul 14, 2016 at 09:27:50PM -0400, Rich Felker wrote:
> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
> > There are two versions of the J-Core interrupt controller in use, aic1
> > which generates interrupts with programmable priorities, but only
> > supports 8 irq lines and maps them to cpu traps in the range 17 to 24,
> > and aic2 which uses traps in the range 64-127 and supports up to 128
> > irqs, with priorities dependent on the interrupt number. The Linux
> > driver does not make use of priorities anyway.
> > 
> > For simplicity, there is no aic1-specific logic in the driver beyond
> > setting the priority register, which is necessary for interrupts to
> > work at all. Eventually aic1 will likely be phased out, but it's
> > currently in use in deployments and all released bitstream binaries.
> 
> Any comments on changes I should make in resubmitting this for the 4.8
> merge window? I could somewhat restrict the possible irqs, but aic1
> allows the pit to be programmed to generate any trap number you want,
> despite the hard-wired interrupts being limited to the range 17-24.
> 
> It might make sense to do something to allow percpu irq requests, but
> I couldn't figure out how to make them work and the current timer
> driver (that needs per-cpu irq delivery) just requests its irq through
> normal request_irq and lets the hardware do the right thing.

I've looked into this some more, and the request_percpu_irq system
looks misdesigned and unusable. It requires the IRQ controller driver
to know in advance, before it knows what devices/drivers are connected
to it, whether each irq will be used with a __percpu dev_id, and this
is not a property of the irq controller hardware or the connected
peripheral device hardware, but rather of the _driver_ for the
connected hardware. This is because the irq controller driver must, at
irqdomain mapping time, decide whether to register the handler as
handle_percpu_devid_irq (which interprets dev_id as a __percpu pointer
and remaps it for the local cpu before invoking the driver's handler)
or one of the other handlers that does not perform any percpu
remapping.

The right way for this to work would be for handle_irq_event_percpu to
be responsible for the remapping, but do it conditionally on whether
the irq was requested via request_irq or request_percpu_irq.

In the mean time until this is overhauled correctly, I believe drivers
(in my case, the J-Core timer driver and IPI "driver") should just
call request_irq with the IRQF_PERCPU flag (to ensure the handler runs
on the cpu the irq was received for) and should do their own
this_cpu_ptr(dev_id) as needed.

Rich
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 3e12479..3cb37d6 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -149,6 +149,12 @@  config PIC32_EVIC
 	select GENERIC_IRQ_CHIP
 	select IRQ_DOMAIN
 
+config JCORE_AIC
+	bool "J-Core integrated AIC"
+	select IRQ_DOMAIN
+	help
+	  Support for the J-Core integrated AIC.
+
 config RENESAS_INTC_IRQPIN
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b03cfcb..5a1f1bf 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -37,6 +37,7 @@  obj-$(CONFIG_I8259)			+= irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
 obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
+obj-$(CONFIG_JCORE_AIC)			+= irq-jcore-aic.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
new file mode 100644
index 0000000..1348cdb
--- /dev/null
+++ b/drivers/irqchip/irq-jcore-aic.c
@@ -0,0 +1,95 @@ 
+/*
+ * J-Core SoC AIC driver
+ *
+ * Copyright (C) 2015-2016 Smart Energy Instruments, Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/cpu.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define AIC1_INTPRI 8
+
+static struct aic_data {
+	unsigned char __iomem *base;
+	u32 cpu_offset;
+	struct irq_chip chip;
+	struct irq_domain *domain;
+	struct notifier_block nb;
+} aic_data;
+
+static int aic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
+{
+	struct aic_data *aic = d->host_data;
+
+	irq_set_chip_data(irq, aic);
+	irq_set_chip_and_handler(irq, &aic->chip, handle_simple_irq);
+	irq_set_probe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aic_irqdomain_ops = {
+	.map = aic_irqdomain_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static void noop(struct irq_data *data)
+{
+}
+
+static void aic1_localenable(struct aic_data *aic)
+{
+	unsigned cpu = smp_processor_id();
+	pr_info("Local AIC enable on cpu %u\n", cpu);
+	writel(0xffffffff, aic->base + cpu * aic->cpu_offset + AIC1_INTPRI);
+}
+
+static int aic1_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
+{
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		aic1_localenable(container_of(self, struct aic_data, nb));
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+int __init aic_irq_of_init(struct device_node *node, struct device_node *parent)
+{
+	struct aic_data *aic = &aic_data;
+
+	aic->base = of_iomap(node, 0);
+	of_property_read_u32(node, "cpu-offset", &aic->cpu_offset);
+
+	pr_info("Initializing J-Core AIC at %p\n", aic->base);
+
+	if (of_device_is_compatible(node, "jcore,aic1")) {
+		/* For aic1, need to enabled zero-priority-by-default irqs */
+		aic->nb.notifier_call = aic1_cpu_notify;
+		register_cpu_notifier(&aic->nb);
+		aic1_localenable(aic);
+	}
+
+	aic->chip.name = node->name;
+	aic->chip.irq_mask = noop;
+	aic->chip.irq_unmask = noop;
+
+	aic->domain = irq_domain_add_linear(node, 128, &aic_irqdomain_ops, aic);
+	irq_create_strict_mappings(aic->domain, 16, 16, 112);
+
+	return 0;
+}
+
+IRQCHIP_DECLARE(jcore_aic2, "jcore,aic2", aic_irq_of_init);
+IRQCHIP_DECLARE(jcore_aic1, "jcore,aic1", aic_irq_of_init);