* [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-29 3:53 ` Rob Herring
0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2011-09-29 3:53 UTC (permalink / raw)
To: linux-arm-kernel
From: Rob Herring <rob.herring@calxeda.com>
Convert the gic interrupt controller to use irq domains in preparation
for device-tree binding and MULTI_IRQ. This allows for translation between
GIC interrupt IDs and Linux irq numbers.
The meaning of irq_offset has changed. It now is just the number of skipped
GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
for secondary GICs.
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
arch/arm/common/Kconfig | 1 +
arch/arm/common/gic.c | 80 ++++++++++++++++++++--------------
arch/arm/include/asm/hardware/gic.h | 3 +
3 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 4b71766..74df9ca 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -1,4 +1,5 @@
config ARM_GIC
+ select IRQ_DOMAIN
bool
config ARM_VIC
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 8b5be72..fa59c99 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -29,6 +29,7 @@
#include <linux/cpu_pm.h>
#include <linux/cpumask.h>
#include <linux/io.h>
+#include <linux/irqdomain.h>
#include <asm/irq.h>
#include <asm/mach/irq.h>
@@ -73,7 +74,7 @@ static inline void __iomem *gic_cpu_base(struct irq_data *d)
static inline unsigned int gic_irq(struct irq_data *d)
{
struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
- return d->irq - gic_data->irq_offset;
+ return d->hwirq + gic_data->irq_offset;
}
/*
@@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
*/
static void gic_mask_irq(struct irq_data *d)
{
- u32 mask = 1 << (d->irq % 32);
+ u32 mask = 1 << (gic_irq(d) % 32);
spin_lock(&irq_controller_lock);
writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_CLEAR + (gic_irq(d) / 32) * 4);
@@ -92,7 +93,7 @@ static void gic_mask_irq(struct irq_data *d)
static void gic_unmask_irq(struct irq_data *d)
{
- u32 mask = 1 << (d->irq % 32);
+ u32 mask = 1 << (gic_irq(d) % 32);
spin_lock(&irq_controller_lock);
if (gic_arch_extn.irq_unmask)
@@ -173,7 +174,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
bool force)
{
void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
- unsigned int shift = (d->irq % 4) * 8;
+ unsigned int shift = (gic_irq(d) % 4) * 8;
unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
u32 val, mask, bit;
@@ -224,7 +225,7 @@ static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
if (gic_irq == 1023)
goto out;
- cascade_irq = gic_irq + chip_data->irq_offset;
+ cascade_irq = irq_domain_to_irq(&chip_data->domain, gic_irq);
if (unlikely(gic_irq < 32 || gic_irq > 1020 || cascade_irq >= NR_IRQS))
do_bad_IRQ(cascade_irq, desc);
else
@@ -256,10 +257,10 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
irq_set_chained_handler(irq, gic_handle_cascade_irq);
}
-static void __init gic_dist_init(struct gic_chip_data *gic,
- unsigned int irq_start)
+static void __init gic_dist_init(struct gic_chip_data *gic)
{
- unsigned int gic_irqs, irq_limit, i;
+ unsigned int gic_irqs = gic->gic_irqs;
+ unsigned int i;
u32 cpumask;
void __iomem *base = gic->dist_base;
u32 cpu = 0;
@@ -275,17 +276,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
writel_relaxed(0, base + GIC_DIST_CTRL);
/*
- * Find out how many interrupts are supported.
- * The GIC only supports up to 1020 interrupt sources.
- */
- gic_irqs = readl_relaxed(base + GIC_DIST_CTR) & 0x1f;
- gic_irqs = (gic_irqs + 1) * 32;
- if (gic_irqs > 1020)
- gic_irqs = 1020;
-
- gic->gic_irqs = gic_irqs;
-
- /*
* Set all global interrupts to be level triggered, active low.
*/
for (i = 32; i < gic_irqs; i += 16)
@@ -311,19 +301,13 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
writel_relaxed(0xffffffff, base + GIC_DIST_ENABLE_CLEAR + i * 4 / 32);
/*
- * Limit number of interrupts registered to the platform maximum
- */
- irq_limit = gic->irq_offset + gic_irqs;
- if (WARN_ON(irq_limit > NR_IRQS))
- irq_limit = NR_IRQS;
-
- /*
* Setup the Linux IRQ subsystem.
*/
- for (i = irq_start; i < irq_limit; i++) {
- irq_set_chip_and_handler(i, &gic_chip, handle_fasteoi_irq);
- irq_set_chip_data(i, gic);
- set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
+ for (i = 0; i < gic->domain.nr_irq; i++) {
+ int irq = irq_domain_to_irq(&gic->domain, i);
+ irq_set_chip_and_handler(irq, &gic_chip, handle_fasteoi_irq);
+ irq_set_chip_data(irq, gic);
+ set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
}
writel_relaxed(1, base + GIC_DIST_CTRL);
@@ -535,23 +519,53 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
}
#endif
+const struct irq_domain_ops gic_irq_domain_ops = {
+};
+
void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
void __iomem *dist_base, void __iomem *cpu_base)
{
struct gic_chip_data *gic;
+ struct irq_domain *domain;
+ int gic_irqs;
BUG_ON(gic_nr >= MAX_GIC_NR);
gic = &gic_data[gic_nr];
+ domain = &gic->domain;
gic->dist_base = dist_base;
gic->cpu_base = cpu_base;
- gic->irq_offset = (irq_start - 1) & ~31;
- if (gic_nr == 0)
+ /*
+ * For primary GICs, skip over SGIs.
+ * For secondary GICs, skip over PPIs, too.
+ */
+ if (gic_nr == 0) {
gic_cpu_base_addr = cpu_base;
+ gic->irq_offset = 16;
+ irq_start = (irq_start & ~31) + 16;
+ } else
+ gic->irq_offset = 32;
+
+ /*
+ * Find out how many interrupts are supported.
+ * The GIC only supports up to 1020 interrupt sources.
+ */
+ gic_irqs = readl_relaxed(dist_base + GIC_DIST_CTR) & 0x1f;
+ gic_irqs = (gic_irqs + 1) * 32;
+ if (gic_irqs > 1020)
+ gic_irqs = 1020;
+ gic->gic_irqs = gic_irqs;
+
+ domain->nr_irq = gic_irqs - gic->irq_offset;
+ domain->irq_base = irq_alloc_descs(-1, irq_start, domain->nr_irq,
+ numa_node_id());
+ domain->priv = gic;
+ domain->ops = &gic_irq_domain_ops;
+ irq_domain_add(domain);
gic_chip.flags |= gic_arch_extn.flags;
- gic_dist_init(gic, irq_start);
+ gic_dist_init(gic);
gic_cpu_init(gic);
gic_pm_init(gic);
}
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index c562705..4893b10 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -33,6 +33,8 @@
#define GIC_DIST_SOFTINT 0xf00
#ifndef __ASSEMBLY__
+#include <linux/irqdomain.h>
+
extern void __iomem *gic_cpu_base_addr;
extern struct irq_chip gic_arch_extn;
@@ -53,6 +55,7 @@ struct gic_chip_data {
u32 __percpu *saved_ppi_enable;
u32 __percpu *saved_ppi_conf;
#endif
+ struct irq_domain domain;
unsigned int gic_irqs;
};
#endif
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-29 3:53 ` Rob Herring
0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2011-09-29 3:53 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Rob Herring
From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Convert the gic interrupt controller to use irq domains in preparation
for device-tree binding and MULTI_IRQ. This allows for translation between
GIC interrupt IDs and Linux irq numbers.
The meaning of irq_offset has changed. It now is just the number of skipped
GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
for secondary GICs.
Cc: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
arch/arm/common/Kconfig | 1 +
arch/arm/common/gic.c | 80 ++++++++++++++++++++--------------
arch/arm/include/asm/hardware/gic.h | 3 +
3 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 4b71766..74df9ca 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -1,4 +1,5 @@
config ARM_GIC
+ select IRQ_DOMAIN
bool
config ARM_VIC
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 8b5be72..fa59c99 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -29,6 +29,7 @@
#include <linux/cpu_pm.h>
#include <linux/cpumask.h>
#include <linux/io.h>
+#include <linux/irqdomain.h>
#include <asm/irq.h>
#include <asm/mach/irq.h>
@@ -73,7 +74,7 @@ static inline void __iomem *gic_cpu_base(struct irq_data *d)
static inline unsigned int gic_irq(struct irq_data *d)
{
struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
- return d->irq - gic_data->irq_offset;
+ return d->hwirq + gic_data->irq_offset;
}
/*
@@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
*/
static void gic_mask_irq(struct irq_data *d)
{
- u32 mask = 1 << (d->irq % 32);
+ u32 mask = 1 << (gic_irq(d) % 32);
spin_lock(&irq_controller_lock);
writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_CLEAR + (gic_irq(d) / 32) * 4);
@@ -92,7 +93,7 @@ static void gic_mask_irq(struct irq_data *d)
static void gic_unmask_irq(struct irq_data *d)
{
- u32 mask = 1 << (d->irq % 32);
+ u32 mask = 1 << (gic_irq(d) % 32);
spin_lock(&irq_controller_lock);
if (gic_arch_extn.irq_unmask)
@@ -173,7 +174,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
bool force)
{
void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
- unsigned int shift = (d->irq % 4) * 8;
+ unsigned int shift = (gic_irq(d) % 4) * 8;
unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
u32 val, mask, bit;
@@ -224,7 +225,7 @@ static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
if (gic_irq == 1023)
goto out;
- cascade_irq = gic_irq + chip_data->irq_offset;
+ cascade_irq = irq_domain_to_irq(&chip_data->domain, gic_irq);
if (unlikely(gic_irq < 32 || gic_irq > 1020 || cascade_irq >= NR_IRQS))
do_bad_IRQ(cascade_irq, desc);
else
@@ -256,10 +257,10 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
irq_set_chained_handler(irq, gic_handle_cascade_irq);
}
-static void __init gic_dist_init(struct gic_chip_data *gic,
- unsigned int irq_start)
+static void __init gic_dist_init(struct gic_chip_data *gic)
{
- unsigned int gic_irqs, irq_limit, i;
+ unsigned int gic_irqs = gic->gic_irqs;
+ unsigned int i;
u32 cpumask;
void __iomem *base = gic->dist_base;
u32 cpu = 0;
@@ -275,17 +276,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
writel_relaxed(0, base + GIC_DIST_CTRL);
/*
- * Find out how many interrupts are supported.
- * The GIC only supports up to 1020 interrupt sources.
- */
- gic_irqs = readl_relaxed(base + GIC_DIST_CTR) & 0x1f;
- gic_irqs = (gic_irqs + 1) * 32;
- if (gic_irqs > 1020)
- gic_irqs = 1020;
-
- gic->gic_irqs = gic_irqs;
-
- /*
* Set all global interrupts to be level triggered, active low.
*/
for (i = 32; i < gic_irqs; i += 16)
@@ -311,19 +301,13 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
writel_relaxed(0xffffffff, base + GIC_DIST_ENABLE_CLEAR + i * 4 / 32);
/*
- * Limit number of interrupts registered to the platform maximum
- */
- irq_limit = gic->irq_offset + gic_irqs;
- if (WARN_ON(irq_limit > NR_IRQS))
- irq_limit = NR_IRQS;
-
- /*
* Setup the Linux IRQ subsystem.
*/
- for (i = irq_start; i < irq_limit; i++) {
- irq_set_chip_and_handler(i, &gic_chip, handle_fasteoi_irq);
- irq_set_chip_data(i, gic);
- set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
+ for (i = 0; i < gic->domain.nr_irq; i++) {
+ int irq = irq_domain_to_irq(&gic->domain, i);
+ irq_set_chip_and_handler(irq, &gic_chip, handle_fasteoi_irq);
+ irq_set_chip_data(irq, gic);
+ set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
}
writel_relaxed(1, base + GIC_DIST_CTRL);
@@ -535,23 +519,53 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
}
#endif
+const struct irq_domain_ops gic_irq_domain_ops = {
+};
+
void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
void __iomem *dist_base, void __iomem *cpu_base)
{
struct gic_chip_data *gic;
+ struct irq_domain *domain;
+ int gic_irqs;
BUG_ON(gic_nr >= MAX_GIC_NR);
gic = &gic_data[gic_nr];
+ domain = &gic->domain;
gic->dist_base = dist_base;
gic->cpu_base = cpu_base;
- gic->irq_offset = (irq_start - 1) & ~31;
- if (gic_nr == 0)
+ /*
+ * For primary GICs, skip over SGIs.
+ * For secondary GICs, skip over PPIs, too.
+ */
+ if (gic_nr == 0) {
gic_cpu_base_addr = cpu_base;
+ gic->irq_offset = 16;
+ irq_start = (irq_start & ~31) + 16;
+ } else
+ gic->irq_offset = 32;
+
+ /*
+ * Find out how many interrupts are supported.
+ * The GIC only supports up to 1020 interrupt sources.
+ */
+ gic_irqs = readl_relaxed(dist_base + GIC_DIST_CTR) & 0x1f;
+ gic_irqs = (gic_irqs + 1) * 32;
+ if (gic_irqs > 1020)
+ gic_irqs = 1020;
+ gic->gic_irqs = gic_irqs;
+
+ domain->nr_irq = gic_irqs - gic->irq_offset;
+ domain->irq_base = irq_alloc_descs(-1, irq_start, domain->nr_irq,
+ numa_node_id());
+ domain->priv = gic;
+ domain->ops = &gic_irq_domain_ops;
+ irq_domain_add(domain);
gic_chip.flags |= gic_arch_extn.flags;
- gic_dist_init(gic, irq_start);
+ gic_dist_init(gic);
gic_cpu_init(gic);
gic_pm_init(gic);
}
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index c562705..4893b10 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -33,6 +33,8 @@
#define GIC_DIST_SOFTINT 0xf00
#ifndef __ASSEMBLY__
+#include <linux/irqdomain.h>
+
extern void __iomem *gic_cpu_base_addr;
extern struct irq_chip gic_arch_extn;
@@ -53,6 +55,7 @@ struct gic_chip_data {
u32 __percpu *saved_ppi_enable;
u32 __percpu *saved_ppi_conf;
#endif
+ struct irq_domain domain;
unsigned int gic_irqs;
};
#endif
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-29 17:15 ` Grant Likely
0 siblings, 0 replies; 20+ messages in thread
From: Grant Likely @ 2011-09-29 17:15 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, devicetree-discuss, linux-kernel, marc.zyngier,
thomas.abraham, jamie, b-cousson, shawn.guo, Rob Herring
On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Convert the gic interrupt controller to use irq domains in preparation
> for device-tree binding and MULTI_IRQ. This allows for translation between
> GIC interrupt IDs and Linux irq numbers.
>
> The meaning of irq_offset has changed. It now is just the number of skipped
> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> for secondary GICs.
[...]
> /*
> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
> */
> static void gic_mask_irq(struct irq_data *d)
> {
> - u32 mask = 1 << (d->irq % 32);
> + u32 mask = 1 << (gic_irq(d) % 32);
This can probably change simply to d->hwirq if irq_offset is
eliminated as I describe below.
> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
> void __iomem *dist_base, void __iomem *cpu_base)
> {
> struct gic_chip_data *gic;
> + struct irq_domain *domain;
> + int gic_irqs;
>
> BUG_ON(gic_nr >= MAX_GIC_NR);
>
> gic = &gic_data[gic_nr];
> + domain = &gic->domain;
> gic->dist_base = dist_base;
> gic->cpu_base = cpu_base;
> - gic->irq_offset = (irq_start - 1) & ~31;
>
> - if (gic_nr == 0)
> + /*
> + * For primary GICs, skip over SGIs.
> + * For secondary GICs, skip over PPIs, too.
> + */
> + if (gic_nr == 0) {
> gic_cpu_base_addr = cpu_base;
> + gic->irq_offset = 16;
> + irq_start = (irq_start & ~31) + 16;
With the switch to irq_domain, there should no longer be any need for
a ~31 mask on the irq_start number. Yes, you'll want to make sure
that it doesn't allocate below irq 16, but the driver should
completely use the irq_domain to manage the mapping from linux-irq
number to hwirq number. The ~31 mask appears to have been an
optimization to quickly calculate hwirq number from the linux one, but
that value is now found in irq_data->hwirq.
irq_offset could almost be dropped entirely outside of probe because
it no longer matters. Instead, populate the to_irq() hook so that the
hwirq value gets set correctly from the outset.
It would be better in general to have a consistent view of what the
hwirq number means regardless of if it is a root GIC.
g.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-29 17:15 ` Grant Likely
0 siblings, 0 replies; 20+ messages in thread
From: Grant Likely @ 2011-09-29 17:15 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Convert the gic interrupt controller to use irq domains in preparation
> for device-tree binding and MULTI_IRQ. This allows for translation between
> GIC interrupt IDs and Linux irq numbers.
>
> The meaning of irq_offset has changed. It now is just the number of skipped
> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> for secondary GICs.
[...]
> /*
> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
> */
> static void gic_mask_irq(struct irq_data *d)
> {
> - u32 mask = 1 << (d->irq % 32);
> + u32 mask = 1 << (gic_irq(d) % 32);
This can probably change simply to d->hwirq if irq_offset is
eliminated as I describe below.
> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
> void __iomem *dist_base, void __iomem *cpu_base)
> {
> struct gic_chip_data *gic;
> + struct irq_domain *domain;
> + int gic_irqs;
>
> BUG_ON(gic_nr >= MAX_GIC_NR);
>
> gic = &gic_data[gic_nr];
> + domain = &gic->domain;
> gic->dist_base = dist_base;
> gic->cpu_base = cpu_base;
> - gic->irq_offset = (irq_start - 1) & ~31;
>
> - if (gic_nr == 0)
> + /*
> + * For primary GICs, skip over SGIs.
> + * For secondary GICs, skip over PPIs, too.
> + */
> + if (gic_nr == 0) {
> gic_cpu_base_addr = cpu_base;
> + gic->irq_offset = 16;
> + irq_start = (irq_start & ~31) + 16;
With the switch to irq_domain, there should no longer be any need for
a ~31 mask on the irq_start number. Yes, you'll want to make sure
that it doesn't allocate below irq 16, but the driver should
completely use the irq_domain to manage the mapping from linux-irq
number to hwirq number. The ~31 mask appears to have been an
optimization to quickly calculate hwirq number from the linux one, but
that value is now found in irq_data->hwirq.
irq_offset could almost be dropped entirely outside of probe because
it no longer matters. Instead, populate the to_irq() hook so that the
hwirq value gets set correctly from the outset.
It would be better in general to have a consistent view of what the
hwirq number means regardless of if it is a root GIC.
g.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-29 17:15 ` Grant Likely
0 siblings, 0 replies; 20+ messages in thread
From: Grant Likely @ 2011-09-29 17:15 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>
> Convert the gic interrupt controller to use irq domains in preparation
> for device-tree binding and MULTI_IRQ. This allows for translation between
> GIC interrupt IDs and Linux irq numbers.
>
> The meaning of irq_offset has changed. It now is just the number of skipped
> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> for secondary GICs.
[...]
> /*
> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
> */
> static void gic_mask_irq(struct irq_data *d)
> {
> - u32 mask = 1 << (d->irq % 32);
> + u32 mask = 1 << (gic_irq(d) % 32);
This can probably change simply to d->hwirq if irq_offset is
eliminated as I describe below.
> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
> void __iomem *dist_base, void __iomem *cpu_base)
> {
> struct gic_chip_data *gic;
> + struct irq_domain *domain;
> + int gic_irqs;
>
> BUG_ON(gic_nr >= MAX_GIC_NR);
>
> gic = &gic_data[gic_nr];
> + domain = &gic->domain;
> gic->dist_base = dist_base;
> gic->cpu_base = cpu_base;
> - gic->irq_offset = (irq_start - 1) & ~31;
>
> - if (gic_nr == 0)
> + /*
> + * For primary GICs, skip over SGIs.
> + * For secondary GICs, skip over PPIs, too.
> + */
> + if (gic_nr == 0) {
> gic_cpu_base_addr = cpu_base;
> + gic->irq_offset = 16;
> + irq_start = (irq_start & ~31) + 16;
With the switch to irq_domain, there should no longer be any need for
a ~31 mask on the irq_start number. Yes, you'll want to make sure
that it doesn't allocate below irq 16, but the driver should
completely use the irq_domain to manage the mapping from linux-irq
number to hwirq number. The ~31 mask appears to have been an
optimization to quickly calculate hwirq number from the linux one, but
that value is now found in irq_data->hwirq.
irq_offset could almost be dropped entirely outside of probe because
it no longer matters. Instead, populate the to_irq() hook so that the
hwirq value gets set correctly from the outset.
It would be better in general to have a consistent view of what the
hwirq number means regardless of if it is a root GIC.
g.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] ARM: gic: add irq_domain support
2011-09-29 17:15 ` Grant Likely
(?)
@ 2011-09-29 21:02 ` Rob Herring
-1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2011-09-29 21:02 UTC (permalink / raw)
To: Grant Likely
Cc: linux-arm-kernel, devicetree-discuss, linux-kernel, marc.zyngier,
thomas.abraham, jamie, b-cousson, shawn.guo
On 09/29/2011 12:15 PM, Grant Likely wrote:
> On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Convert the gic interrupt controller to use irq domains in preparation
>> for device-tree binding and MULTI_IRQ. This allows for translation between
>> GIC interrupt IDs and Linux irq numbers.
>>
>> The meaning of irq_offset has changed. It now is just the number of skipped
>> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
>> for secondary GICs.
>
> [...]
>
>> /*
>> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
>> */
>> static void gic_mask_irq(struct irq_data *d)
>> {
>> - u32 mask = 1 << (d->irq % 32);
>> + u32 mask = 1 << (gic_irq(d) % 32);
>
> This can probably change simply to d->hwirq if irq_offset is
> eliminated as I describe below.
>
>> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>> void __iomem *dist_base, void __iomem *cpu_base)
>> {
>> struct gic_chip_data *gic;
>> + struct irq_domain *domain;
>> + int gic_irqs;
>>
>> BUG_ON(gic_nr >= MAX_GIC_NR);
>>
>> gic = &gic_data[gic_nr];
>> + domain = &gic->domain;
>> gic->dist_base = dist_base;
>> gic->cpu_base = cpu_base;
>> - gic->irq_offset = (irq_start - 1) & ~31;
>>
>> - if (gic_nr == 0)
>> + /*
>> + * For primary GICs, skip over SGIs.
>> + * For secondary GICs, skip over PPIs, too.
>> + */
>> + if (gic_nr == 0) {
>> gic_cpu_base_addr = cpu_base;
>> + gic->irq_offset = 16;
>> + irq_start = (irq_start & ~31) + 16;
>
> With the switch to irq_domain, there should no longer be any need for
> a ~31 mask on the irq_start number. Yes, you'll want to make sure
> that it doesn't allocate below irq 16, but the driver should
> completely use the irq_domain to manage the mapping from linux-irq
> number to hwirq number. The ~31 mask appears to have been an
> optimization to quickly calculate hwirq number from the linux one, but
> that value is now found in irq_data->hwirq.
I started out exactly this way removing irq_offset. The problem is the
core irq domain code assumes that hwirq starts at 0, but for the gic it
starts at 16 or 32.
It could be fixed in the domain core, but that would effectively mean
moving irq_offset into the core.
The masking here is mainly to not break exynos. Everyone else passes in
a value of 16-31 which I could just hardcode to 16. So when exynos gets
all the interrupts converted over to domains, this can be removed. I
looked at it briefly and it doesn't look trivial.
>
> irq_offset could almost be dropped entirely outside of probe because
> it no longer matters. Instead, populate the to_irq() hook so that the
> hwirq value gets set correctly from the outset.
>
> It would be better in general to have a consistent view of what the
> hwirq number means regardless of if it is a root GIC.
Agreed.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-29 21:02 ` Rob Herring
0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2011-09-29 21:02 UTC (permalink / raw)
To: linux-arm-kernel
On 09/29/2011 12:15 PM, Grant Likely wrote:
> On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Convert the gic interrupt controller to use irq domains in preparation
>> for device-tree binding and MULTI_IRQ. This allows for translation between
>> GIC interrupt IDs and Linux irq numbers.
>>
>> The meaning of irq_offset has changed. It now is just the number of skipped
>> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
>> for secondary GICs.
>
> [...]
>
>> /*
>> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
>> */
>> static void gic_mask_irq(struct irq_data *d)
>> {
>> - u32 mask = 1 << (d->irq % 32);
>> + u32 mask = 1 << (gic_irq(d) % 32);
>
> This can probably change simply to d->hwirq if irq_offset is
> eliminated as I describe below.
>
>> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>> void __iomem *dist_base, void __iomem *cpu_base)
>> {
>> struct gic_chip_data *gic;
>> + struct irq_domain *domain;
>> + int gic_irqs;
>>
>> BUG_ON(gic_nr >= MAX_GIC_NR);
>>
>> gic = &gic_data[gic_nr];
>> + domain = &gic->domain;
>> gic->dist_base = dist_base;
>> gic->cpu_base = cpu_base;
>> - gic->irq_offset = (irq_start - 1) & ~31;
>>
>> - if (gic_nr == 0)
>> + /*
>> + * For primary GICs, skip over SGIs.
>> + * For secondary GICs, skip over PPIs, too.
>> + */
>> + if (gic_nr == 0) {
>> gic_cpu_base_addr = cpu_base;
>> + gic->irq_offset = 16;
>> + irq_start = (irq_start & ~31) + 16;
>
> With the switch to irq_domain, there should no longer be any need for
> a ~31 mask on the irq_start number. Yes, you'll want to make sure
> that it doesn't allocate below irq 16, but the driver should
> completely use the irq_domain to manage the mapping from linux-irq
> number to hwirq number. The ~31 mask appears to have been an
> optimization to quickly calculate hwirq number from the linux one, but
> that value is now found in irq_data->hwirq.
I started out exactly this way removing irq_offset. The problem is the
core irq domain code assumes that hwirq starts at 0, but for the gic it
starts at 16 or 32.
It could be fixed in the domain core, but that would effectively mean
moving irq_offset into the core.
The masking here is mainly to not break exynos. Everyone else passes in
a value of 16-31 which I could just hardcode to 16. So when exynos gets
all the interrupts converted over to domains, this can be removed. I
looked at it briefly and it doesn't look trivial.
>
> irq_offset could almost be dropped entirely outside of probe because
> it no longer matters. Instead, populate the to_irq() hook so that the
> hwirq value gets set correctly from the outset.
>
> It would be better in general to have a consistent view of what the
> hwirq number means regardless of if it is a root GIC.
Agreed.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-29 21:02 ` Rob Herring
0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2011-09-29 21:02 UTC (permalink / raw)
To: Grant Likely
Cc: b-cousson, marc.zyngier, devicetree-discuss, linux-kernel,
thomas.abraham, jamie, shawn.guo, linux-arm-kernel
On 09/29/2011 12:15 PM, Grant Likely wrote:
> On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Convert the gic interrupt controller to use irq domains in preparation
>> for device-tree binding and MULTI_IRQ. This allows for translation between
>> GIC interrupt IDs and Linux irq numbers.
>>
>> The meaning of irq_offset has changed. It now is just the number of skipped
>> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
>> for secondary GICs.
>
> [...]
>
>> /*
>> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
>> */
>> static void gic_mask_irq(struct irq_data *d)
>> {
>> - u32 mask = 1 << (d->irq % 32);
>> + u32 mask = 1 << (gic_irq(d) % 32);
>
> This can probably change simply to d->hwirq if irq_offset is
> eliminated as I describe below.
>
>> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>> void __iomem *dist_base, void __iomem *cpu_base)
>> {
>> struct gic_chip_data *gic;
>> + struct irq_domain *domain;
>> + int gic_irqs;
>>
>> BUG_ON(gic_nr >= MAX_GIC_NR);
>>
>> gic = &gic_data[gic_nr];
>> + domain = &gic->domain;
>> gic->dist_base = dist_base;
>> gic->cpu_base = cpu_base;
>> - gic->irq_offset = (irq_start - 1) & ~31;
>>
>> - if (gic_nr == 0)
>> + /*
>> + * For primary GICs, skip over SGIs.
>> + * For secondary GICs, skip over PPIs, too.
>> + */
>> + if (gic_nr == 0) {
>> gic_cpu_base_addr = cpu_base;
>> + gic->irq_offset = 16;
>> + irq_start = (irq_start & ~31) + 16;
>
> With the switch to irq_domain, there should no longer be any need for
> a ~31 mask on the irq_start number. Yes, you'll want to make sure
> that it doesn't allocate below irq 16, but the driver should
> completely use the irq_domain to manage the mapping from linux-irq
> number to hwirq number. The ~31 mask appears to have been an
> optimization to quickly calculate hwirq number from the linux one, but
> that value is now found in irq_data->hwirq.
I started out exactly this way removing irq_offset. The problem is the
core irq domain code assumes that hwirq starts at 0, but for the gic it
starts at 16 or 32.
It could be fixed in the domain core, but that would effectively mean
moving irq_offset into the core.
The masking here is mainly to not break exynos. Everyone else passes in
a value of 16-31 which I could just hardcode to 16. So when exynos gets
all the interrupts converted over to domains, this can be removed. I
looked at it briefly and it doesn't look trivial.
>
> irq_offset could almost be dropped entirely outside of probe because
> it no longer matters. Instead, populate the to_irq() hook so that the
> hwirq value gets set correctly from the outset.
>
> It would be better in general to have a consistent view of what the
> hwirq number means regardless of if it is a root GIC.
Agreed.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] ARM: gic: add irq_domain support
2011-09-29 21:02 ` Rob Herring
@ 2011-09-30 0:27 ` Grant Likely
-1 siblings, 0 replies; 20+ messages in thread
From: Grant Likely @ 2011-09-30 0:27 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, devicetree-discuss, linux-kernel, marc.zyngier,
thomas.abraham, jamie, b-cousson, shawn.guo
On Thu, Sep 29, 2011 at 04:02:44PM -0500, Rob Herring wrote:
> On 09/29/2011 12:15 PM, Grant Likely wrote:
> > On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> >> From: Rob Herring <rob.herring@calxeda.com>
> >>
> >> Convert the gic interrupt controller to use irq domains in preparation
> >> for device-tree binding and MULTI_IRQ. This allows for translation between
> >> GIC interrupt IDs and Linux irq numbers.
> >>
> >> The meaning of irq_offset has changed. It now is just the number of skipped
> >> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> >> for secondary GICs.
> >
> > [...]
> >
> >> /*
> >> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
> >> */
> >> static void gic_mask_irq(struct irq_data *d)
> >> {
> >> - u32 mask = 1 << (d->irq % 32);
> >> + u32 mask = 1 << (gic_irq(d) % 32);
> >
> > This can probably change simply to d->hwirq if irq_offset is
> > eliminated as I describe below.
> >
> >> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
> >> void __iomem *dist_base, void __iomem *cpu_base)
> >> {
> >> struct gic_chip_data *gic;
> >> + struct irq_domain *domain;
> >> + int gic_irqs;
> >>
> >> BUG_ON(gic_nr >= MAX_GIC_NR);
> >>
> >> gic = &gic_data[gic_nr];
> >> + domain = &gic->domain;
> >> gic->dist_base = dist_base;
> >> gic->cpu_base = cpu_base;
> >> - gic->irq_offset = (irq_start - 1) & ~31;
> >>
> >> - if (gic_nr == 0)
> >> + /*
> >> + * For primary GICs, skip over SGIs.
> >> + * For secondary GICs, skip over PPIs, too.
> >> + */
> >> + if (gic_nr == 0) {
> >> gic_cpu_base_addr = cpu_base;
> >> + gic->irq_offset = 16;
> >> + irq_start = (irq_start & ~31) + 16;
> >
> > With the switch to irq_domain, there should no longer be any need for
> > a ~31 mask on the irq_start number. Yes, you'll want to make sure
> > that it doesn't allocate below irq 16, but the driver should
> > completely use the irq_domain to manage the mapping from linux-irq
> > number to hwirq number. The ~31 mask appears to have been an
> > optimization to quickly calculate hwirq number from the linux one, but
> > that value is now found in irq_data->hwirq.
>
> I started out exactly this way removing irq_offset. The problem is the
> core irq domain code assumes that hwirq starts at 0, but for the gic it
> starts at 16 or 32.
>
> It could be fixed in the domain core, but that would effectively mean
> moving irq_offset into the core.
Fix irq_domain. If it cannot handle this trivial case, then it won't
be able to handle the complex powerpc use cases.
g.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: gic: add irq_domain support
@ 2011-09-30 0:27 ` Grant Likely
0 siblings, 0 replies; 20+ messages in thread
From: Grant Likely @ 2011-09-30 0:27 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 29, 2011 at 04:02:44PM -0500, Rob Herring wrote:
> On 09/29/2011 12:15 PM, Grant Likely wrote:
> > On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
> >> From: Rob Herring <rob.herring@calxeda.com>
> >>
> >> Convert the gic interrupt controller to use irq domains in preparation
> >> for device-tree binding and MULTI_IRQ. This allows for translation between
> >> GIC interrupt IDs and Linux irq numbers.
> >>
> >> The meaning of irq_offset has changed. It now is just the number of skipped
> >> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
> >> for secondary GICs.
> >
> > [...]
> >
> >> /*
> >> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
> >> */
> >> static void gic_mask_irq(struct irq_data *d)
> >> {
> >> - u32 mask = 1 << (d->irq % 32);
> >> + u32 mask = 1 << (gic_irq(d) % 32);
> >
> > This can probably change simply to d->hwirq if irq_offset is
> > eliminated as I describe below.
> >
> >> void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
> >> void __iomem *dist_base, void __iomem *cpu_base)
> >> {
> >> struct gic_chip_data *gic;
> >> + struct irq_domain *domain;
> >> + int gic_irqs;
> >>
> >> BUG_ON(gic_nr >= MAX_GIC_NR);
> >>
> >> gic = &gic_data[gic_nr];
> >> + domain = &gic->domain;
> >> gic->dist_base = dist_base;
> >> gic->cpu_base = cpu_base;
> >> - gic->irq_offset = (irq_start - 1) & ~31;
> >>
> >> - if (gic_nr == 0)
> >> + /*
> >> + * For primary GICs, skip over SGIs.
> >> + * For secondary GICs, skip over PPIs, too.
> >> + */
> >> + if (gic_nr == 0) {
> >> gic_cpu_base_addr = cpu_base;
> >> + gic->irq_offset = 16;
> >> + irq_start = (irq_start & ~31) + 16;
> >
> > With the switch to irq_domain, there should no longer be any need for
> > a ~31 mask on the irq_start number. Yes, you'll want to make sure
> > that it doesn't allocate below irq 16, but the driver should
> > completely use the irq_domain to manage the mapping from linux-irq
> > number to hwirq number. The ~31 mask appears to have been an
> > optimization to quickly calculate hwirq number from the linux one, but
> > that value is now found in irq_data->hwirq.
>
> I started out exactly this way removing irq_offset. The problem is the
> core irq domain code assumes that hwirq starts at 0, but for the gic it
> starts at 16 or 32.
>
> It could be fixed in the domain core, but that would effectively mean
> moving irq_offset into the core.
Fix irq_domain. If it cannot handle this trivial case, then it won't
be able to handle the complex powerpc use cases.
g.
^ permalink raw reply [flat|nested] 20+ messages in thread