Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Tue, Nov 26, 2013 at 9:05 PM, Rob Herring wrote:
> On Mon, Nov 25, 2013 at 10:00 AM, Anup Patel wrote:
>> On Mon, Nov 25, 2013 at 9:13 PM, Rob Herring wrote:
>>> On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
> [dropping from the CC list, as someone seems to have
> tripped on the config file, and I'm tired of getting bounces]
>
> Feng,
>
> On 19/11/13 21:42, Feng Kan wrote:
>> The GIC-400 implementation allows for FIQ and IRQ bypass. In the
>> X-Gene implementation, the FIQ bypass must be enabled at all time.
>> Otherwise, some PPI will appear as FIQ and cause kernel problem.
>
> How comes? Are only PPIs affected? When you say "some PPIs", can you be
> more specific?
>
>> Signed-off-by: Feng Kan
>> ---
>> drivers/irqchip/irq-gic.c | 15 +++
>> include/linux/irqchip/arm-gic.h |4 ++--
>> 2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index d0e9480..aa7342e 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -65,6 +65,7 @@ struct gic_chip_data {
>> #endif
>> struct irq_domain *domain;
>> unsigned int gic_irqs;
>> + unsigned int bypass_flag;
>> #ifdef CONFIG_GIC_NON_BANKED
>> void __iomem *(*get_base)(union gic_base *);
>> #endif
>> @@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i *
>> 4 / 4);
>>
>> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>> - writel_relaxed(1, base + GIC_CPU_CTRL);
>> + writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
>> }
>>
>> void gic_cpu_if_down(void)
>> @@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i *
>> 4);
>>
>> writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>> - writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>> + writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
>> GIC_CPU_CTRL);
>> }
>>
>> static int gic_notifier(struct notifier_block *self, unsigned long cmd,
>> void *v)
>> @@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
>>
>> void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>> void __iomem *dist_base, void __iomem *cpu_base,
>> -u32 percpu_offset, struct device_node *node)
>> +u32 percpu_offset, u32 bypass_val,
>> +struct device_node *node)
>> {
>> irq_hw_number_t hwirq_base;
>> struct gic_chip_data *gic;
>> @@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
>> irq_start,
>>
>> set_handle_irq(gic_handle_irq);
>>
>> + gic->bypass_flag = (bypass_val & 0xf) << 4;
>
> Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed
> with.
>
>> gic_chip.flags |= gic_arch_extn.flags;
>> gic_dist_init(gic);
>> gic_cpu_init(gic);
>> @@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
>> struct device_node *parent)
>> void __iomem *cpu_base;
>> void __iomem *dist_base;
>> u32 percpu_offset;
>> + u32 bypass_val;
>> int irq;
>>
>> if (WARN_ON(!node))
>> @@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
>> struct device_node *parent)
>> if (of_property_read_u32(node, "cpu-offset", _offset))
>> percpu_offset = 0;
>>
>> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
>> node);
>> + if (of_property_read_u32(node, "bypass-flags", _val))
>> + bypass_val = 0;
>
> [adding Mark on Cc, so he can comment on the DT parts]
>
> Where's the DT documentation update? Also, as this is an
> implementation-specific quirk, you should consider using a separate
> compatible string and move the handling of this issue into some X-Gene
> specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to "arm,cortex-a15-gic". We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in
X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0,
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Mon, Nov 25, 2013 at 10:00 AM, Anup Patel wrote:
> On Mon, Nov 25, 2013 at 9:13 PM, Rob Herring wrote:
>> On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel wrote:
>>> On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
[dropping from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
> The GIC-400 implementation allows for FIQ and IRQ bypass. In the
> X-Gene implementation, the FIQ bypass must be enabled at all time.
> Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say "some PPIs", can you be
more specific?
> Signed-off-by: Feng Kan
> ---
> drivers/irqchip/irq-gic.c | 15 +++
> include/linux/irqchip/arm-gic.h |4 ++--
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index d0e9480..aa7342e 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -65,6 +65,7 @@ struct gic_chip_data {
> #endif
> struct irq_domain *domain;
> unsigned int gic_irqs;
> + unsigned int bypass_flag;
> #ifdef CONFIG_GIC_NON_BANKED
> void __iomem *(*get_base)(union gic_base *);
> #endif
> @@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4
> / 4);
>
> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
> - writel_relaxed(1, base + GIC_CPU_CTRL);
> + writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
> }
>
> void gic_cpu_if_down(void)
> @@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i *
> 4);
>
> writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
> - writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
> + writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
> GIC_CPU_CTRL);
> }
>
> static int gic_notifier(struct notifier_block *self, unsigned long cmd,
> void *v)
> @@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
>
> void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> void __iomem *dist_base, void __iomem *cpu_base,
> -u32 percpu_offset, struct device_node *node)
> +u32 percpu_offset, u32 bypass_val,
> +struct device_node *node)
> {
> irq_hw_number_t hwirq_base;
> struct gic_chip_data *gic;
> @@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
> irq_start,
>
> set_handle_irq(gic_handle_irq);
>
> + gic->bypass_flag = (bypass_val & 0xf) << 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
> gic_chip.flags |= gic_arch_extn.flags;
> gic_dist_init(gic);
> gic_cpu_init(gic);
> @@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
> struct device_node *parent)
> void __iomem *cpu_base;
> void __iomem *dist_base;
> u32 percpu_offset;
> + u32 bypass_val;
> int irq;
>
> if (WARN_ON(!node))
> @@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
> struct device_node *parent)
> if (of_property_read_u32(node, "cpu-offset", _offset))
> percpu_offset = 0;
>
> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
> node);
> + if (of_property_read_u32(node, "bypass-flags", _val))
> + bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
>>>
>>> Adding separate compatible string for X-Gene specific GIC will break
>>> VGIC code for X-Gene because VGIC code looks for DT node compatible
>>> to "arm,cortex-a15-gic". We don't want to break currently working VGIC
>>> code for X-Gene.
>>>
>>> The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
>>> feature of GIC-400 and its not X-Gene specific. The only difference in
>>> X-Gene
>>> is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
>>> event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
>>> and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
>>> these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
>>> PPI28 (Legacy-FIQ).
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Mon, Nov 25, 2013 at 10:00 AM, Anup Patel a...@brainfault.org wrote:
On Mon, Nov 25, 2013 at 9:13 PM, Rob Herring robherri...@gmail.com wrote:
On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel a...@brainfault.org wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4
/ 4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i *
4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in
X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
IIRC, all these bits are secure mode only (or only a subset are
banked). Whether an SoC supports secure mode or not, the current
policy for the kernel is to do any secure mode setup in firmware or
bootloader.
Group0 are secure interrupts and
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Tue, Nov 26, 2013 at 9:05 PM, Rob Herring robherri...@gmail.com wrote:
On Mon, Nov 25, 2013 at 10:00 AM, Anup Patel a...@brainfault.org wrote:
On Mon, Nov 25, 2013 at 9:13 PM, Rob Herring robherri...@gmail.com wrote:
On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel a...@brainfault.org wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i *
4 / 4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i *
4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed
with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in
X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
IIRC, all these bits are secure mode only (or only a subset are
banked). Whether an SoC supports secure mode or not, the current
policy for the kernel is to do any
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
> >> The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a >> feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene >> is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf > > Amazing. Someone managed to push the weird-o-meter one level higher. > Were you *that* short on PPIs that you had to use these two? We don't support bypass enable at all, nothing to do with the PPI. It must be set to bypass disable always for us. Those two are the unfortunate victims. > >> event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1 >> and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set >> these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and >> PPI28 (Legacy-FIQ). >> >> We should have more cleaner and optional device tree binding for GIC >> which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1 >> and FIQBypDisGrp1 bits for X-Gene. > > Well, here's an alternative approach for you. As you said, this is in no > way X-Gene specific, so maybe we should address it directly in the GIC > code. Just hook something in the irq_mask/irq_unmask methods, so we can > detect the use of these two PPIs, and toggle the bypass flags > accordingly bits accordingly. Yes, I hope Catalin could comment on this one. I don't think he would like this as well since it is our platform specific. > > M. > -- > Jazz is not dead. It just smells funny... > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Mon, Nov 25, 2013 at 9:13 PM, Rob Herring wrote:
> On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel wrote:
>> On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
>>> [dropping from the CC list, as someone seems to have
>>> tripped on the config file, and I'm tired of getting bounces]
>>>
>>> Feng,
>>>
>>> On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
>>>
>>> How comes? Are only PPIs affected? When you say "some PPIs", can you be
>>> more specific?
>>>
Signed-off-by: Feng Kan
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4
/ 4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic->bypass_flag = (bypass_val & 0xf) << 4;
>>>
>>> Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
>>>
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, "cpu-offset", _offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
node);
+ if (of_property_read_u32(node, "bypass-flags", _val))
+ bypass_val = 0;
>>>
>>> [adding Mark on Cc, so he can comment on the DT parts]
>>>
>>> Where's the DT documentation update? Also, as this is an
>>> implementation-specific quirk, you should consider using a separate
>>> compatible string and move the handling of this issue into some X-Gene
>>> specific code.
>>
>> Adding separate compatible string for X-Gene specific GIC will break
>> VGIC code for X-Gene because VGIC code looks for DT node compatible
>> to "arm,cortex-a15-gic". We don't want to break currently working VGIC
>> code for X-Gene.
>>
>> The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
>> feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
>> is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
>> event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
>> and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
>> these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
>> PPI28 (Legacy-FIQ).
>>
>> We should have more cleaner and optional device tree binding for GIC
>> which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
>> and FIQBypDisGrp1 bits for X-Gene.
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel wrote:
> On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
>> [dropping from the CC list, as someone seems to have
>> tripped on the config file, and I'm tired of getting bounces]
>>
>> Feng,
>>
>> On 19/11/13 21:42, Feng Kan wrote:
>>> The GIC-400 implementation allows for FIQ and IRQ bypass. In the
>>> X-Gene implementation, the FIQ bypass must be enabled at all time.
>>> Otherwise, some PPI will appear as FIQ and cause kernel problem.
>>
>> How comes? Are only PPIs affected? When you say "some PPIs", can you be
>> more specific?
>>
>>> Signed-off-by: Feng Kan
>>> ---
>>> drivers/irqchip/irq-gic.c | 15 +++
>>> include/linux/irqchip/arm-gic.h |4 ++--
>>> 2 files changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index d0e9480..aa7342e 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -65,6 +65,7 @@ struct gic_chip_data {
>>> #endif
>>> struct irq_domain *domain;
>>> unsigned int gic_irqs;
>>> + unsigned int bypass_flag;
>>> #ifdef CONFIG_GIC_NON_BANKED
>>> void __iomem *(*get_base)(union gic_base *);
>>> #endif
>>> @@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
>>> 4);
>>>
>>> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>>> - writel_relaxed(1, base + GIC_CPU_CTRL);
>>> + writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
>>> }
>>>
>>> void gic_cpu_if_down(void)
>>> @@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>>
>>> writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>>> - writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>>> + writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
>>> GIC_CPU_CTRL);
>>> }
>>>
>>> static int gic_notifier(struct notifier_block *self, unsigned long cmd,
>>> void *v)
>>> @@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
>>>
>>> void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>> void __iomem *dist_base, void __iomem *cpu_base,
>>> -u32 percpu_offset, struct device_node *node)
>>> +u32 percpu_offset, u32 bypass_val,
>>> +struct device_node *node)
>>> {
>>> irq_hw_number_t hwirq_base;
>>> struct gic_chip_data *gic;
>>> @@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
>>> irq_start,
>>>
>>> set_handle_irq(gic_handle_irq);
>>>
>>> + gic->bypass_flag = (bypass_val & 0xf) << 4;
>>
>> Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
>>
>>> gic_chip.flags |= gic_arch_extn.flags;
>>> gic_dist_init(gic);
>>> gic_cpu_init(gic);
>>> @@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
>>> device_node *parent)
>>> void __iomem *cpu_base;
>>> void __iomem *dist_base;
>>> u32 percpu_offset;
>>> + u32 bypass_val;
>>> int irq;
>>>
>>> if (WARN_ON(!node))
>>> @@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
>>> struct device_node *parent)
>>> if (of_property_read_u32(node, "cpu-offset", _offset))
>>> percpu_offset = 0;
>>>
>>> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
>>> + if (of_property_read_u32(node, "bypass-flags", _val))
>>> + bypass_val = 0;
>>
>> [adding Mark on Cc, so he can comment on the DT parts]
>>
>> Where's the DT documentation update? Also, as this is an
>> implementation-specific quirk, you should consider using a separate
>> compatible string and move the handling of this issue into some X-Gene
>> specific code.
>
> Adding separate compatible string for X-Gene specific GIC will break
> VGIC code for X-Gene because VGIC code looks for DT node compatible
> to "arm,cortex-a15-gic". We don't want to break currently working VGIC
> code for X-Gene.
>
> The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
> feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
> is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
> event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
> and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
> these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
> PPI28 (Legacy-FIQ).
>
> We should have more cleaner and optional device tree binding for GIC
> which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
> and FIQBypDisGrp1 bits for X-Gene.
IIRC, all these bits are secure mode only (or only a subset are
banked). Whether an SoC supports secure mode or not, the current
policy for the kernel is to do any secure mode
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
(Adding correct "Kumar Sankaran" to CC. My mistake.)
(Adding back patc...@apm.com. It is fixed now).
On Mon, Nov 25, 2013 at 2:52 PM, Marc Zyngier wrote:
> On 23/11/13 08:41, Anup Patel wrote:
>> On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
>>> [dropping from the CC list, as someone seems to have
>>> tripped on the config file, and I'm tired of getting bounces]
>>>
>>> Feng,
>>>
>>> On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
>>>
>>> How comes? Are only PPIs affected? When you say "some PPIs", can you be
>>> more specific?
>>>
Signed-off-by: Feng Kan
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4
/ 4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic->bypass_flag = (bypass_val & 0xf) << 4;
>>>
>>> Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
>>>
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, "cpu-offset", _offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
node);
+ if (of_property_read_u32(node, "bypass-flags", _val))
+ bypass_val = 0;
>>>
>>> [adding Mark on Cc, so he can comment on the DT parts]
>>>
>>> Where's the DT documentation update? Also, as this is an
>>> implementation-specific quirk, you should consider using a separate
>>> compatible string and move the handling of this issue into some X-Gene
>>> specific code.
>>
>> Adding separate compatible string for X-Gene specific GIC will break
>> VGIC code for X-Gene because VGIC code looks for DT node compatible
>> to "arm,cortex-a15-gic". We don't want to break currently working VGIC
>> code for X-Gene.
>
> I'm sure you could manage adding another compat string to the vgic code...
>
>> The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
>> feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
>> is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
>
> Amazing. Someone managed to push the weird-o-meter one level higher.
> Were you *that* short on PPIs that you had to use these two?
>
>> event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
>> and FIQBypDisGrp1 bits are 0 by default and
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On 23/11/13 08:41, Anup Patel wrote:
> On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
>> [dropping from the CC list, as someone seems to have
>> tripped on the config file, and I'm tired of getting bounces]
>>
>> Feng,
>>
>> On 19/11/13 21:42, Feng Kan wrote:
>>> The GIC-400 implementation allows for FIQ and IRQ bypass. In the
>>> X-Gene implementation, the FIQ bypass must be enabled at all time.
>>> Otherwise, some PPI will appear as FIQ and cause kernel problem.
>>
>> How comes? Are only PPIs affected? When you say "some PPIs", can you be
>> more specific?
>>
>>> Signed-off-by: Feng Kan
>>> ---
>>> drivers/irqchip/irq-gic.c | 15 +++
>>> include/linux/irqchip/arm-gic.h |4 ++--
>>> 2 files changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index d0e9480..aa7342e 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -65,6 +65,7 @@ struct gic_chip_data {
>>> #endif
>>> struct irq_domain *domain;
>>> unsigned int gic_irqs;
>>> + unsigned int bypass_flag;
>>> #ifdef CONFIG_GIC_NON_BANKED
>>> void __iomem *(*get_base)(union gic_base *);
>>> #endif
>>> @@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
>>> 4);
>>>
>>> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>>> - writel_relaxed(1, base + GIC_CPU_CTRL);
>>> + writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
>>> }
>>>
>>> void gic_cpu_if_down(void)
>>> @@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>>
>>> writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>>> - writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>>> + writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
>>> GIC_CPU_CTRL);
>>> }
>>>
>>> static int gic_notifier(struct notifier_block *self, unsigned long cmd,
>>> void *v)
>>> @@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
>>>
>>> void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>> void __iomem *dist_base, void __iomem *cpu_base,
>>> -u32 percpu_offset, struct device_node *node)
>>> +u32 percpu_offset, u32 bypass_val,
>>> +struct device_node *node)
>>> {
>>> irq_hw_number_t hwirq_base;
>>> struct gic_chip_data *gic;
>>> @@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
>>> irq_start,
>>>
>>> set_handle_irq(gic_handle_irq);
>>>
>>> + gic->bypass_flag = (bypass_val & 0xf) << 4;
>>
>> Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
>>
>>> gic_chip.flags |= gic_arch_extn.flags;
>>> gic_dist_init(gic);
>>> gic_cpu_init(gic);
>>> @@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
>>> device_node *parent)
>>> void __iomem *cpu_base;
>>> void __iomem *dist_base;
>>> u32 percpu_offset;
>>> + u32 bypass_val;
>>> int irq;
>>>
>>> if (WARN_ON(!node))
>>> @@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
>>> struct device_node *parent)
>>> if (of_property_read_u32(node, "cpu-offset", _offset))
>>> percpu_offset = 0;
>>>
>>> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
>>> + if (of_property_read_u32(node, "bypass-flags", _val))
>>> + bypass_val = 0;
>>
>> [adding Mark on Cc, so he can comment on the DT parts]
>>
>> Where's the DT documentation update? Also, as this is an
>> implementation-specific quirk, you should consider using a separate
>> compatible string and move the handling of this issue into some X-Gene
>> specific code.
>
> Adding separate compatible string for X-Gene specific GIC will break
> VGIC code for X-Gene because VGIC code looks for DT node compatible
> to "arm,cortex-a15-gic". We don't want to break currently working VGIC
> code for X-Gene.
I'm sure you could manage adding another compat string to the vgic code...
> The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
> feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
> is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
Amazing. Someone managed to push the weird-o-meter one level higher.
Were you *that* short on PPIs that you had to use these two?
> event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
> and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
> these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
> PPI28 (Legacy-FIQ).
>
> We should have more cleaner and optional device tree binding for GIC
> which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
> and FIQBypDisGrp1
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On 23/11/13 08:41, Anup Patel wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
I'm sure you could manage adding another compat string to the vgic code...
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
Amazing. Someone managed to push the weird-o-meter one level higher.
Were you *that* short on PPIs that you had to use these two?
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
Well, here's an alternative approach for you. As you said, this is in no
way X-Gene specific, so maybe we should address it directly in the GIC
code. Just hook something in the irq_mask/irq_unmask methods, so we can
detect the
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
(Adding correct Kumar Sankaran to CC. My mistake.)
(Adding back patc...@apm.com. It is fixed now).
On Mon, Nov 25, 2013 at 2:52 PM, Marc Zyngier marc.zyng...@arm.com wrote:
On 23/11/13 08:41, Anup Patel wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4
/ 4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
I'm sure you could manage adding another compat string to the vgic code...
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
Amazing. Someone managed to push the weird-o-meter one level higher.
Were you *that* short on PPIs that you had to use these two?
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
Well, here's an alternative approach for you.
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel a...@brainfault.org wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
IIRC, all these bits are secure mode only (or only a subset are
banked). Whether an SoC supports secure mode or not, the current
policy for the kernel is to do any secure mode setup in firmware or
bootloader.
Rob
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Mon, Nov 25, 2013 at 9:13 PM, Rob Herring robherri...@gmail.com wrote:
On Sat, Nov 23, 2013 at 2:41 AM, Anup Patel a...@brainfault.org wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4
/ 4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
IIRC, all these bits are secure mode only (or only a subset are
banked). Whether an SoC supports secure mode or not, the current
policy for the kernel is to do any secure mode setup in firmware or
bootloader.
Group0 are secure interrupts and Group1 are non-secure interrupts.
Does this mean we should only touch
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf Amazing. Someone managed to push the weird-o-meter one level higher. Were you *that* short on PPIs that you had to use these two? We don't support bypass enable at all, nothing to do with the PPI. It must be set to bypass disable always for us. Those two are the unfortunate victims. event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1 and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and PPI28 (Legacy-FIQ). We should have more cleaner and optional device tree binding for GIC which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1 and FIQBypDisGrp1 bits for X-Gene. Well, here's an alternative approach for you. As you said, this is in no way X-Gene specific, so maybe we should address it directly in the GIC code. Just hook something in the irq_mask/irq_unmask methods, so we can detect the use of these two PPIs, and toggle the bypass flags accordingly bits accordingly. Yes, I hope Catalin could comment on this one. I don't think he would like this as well since it is our platform specific. M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Sat, Nov 23, 2013 at 2:11 PM, Anup Patel wrote:
> On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
>> [dropping from the CC list, as someone seems to have
>> tripped on the config file, and I'm tired of getting bounces]
>>
>> Feng,
>>
>> On 19/11/13 21:42, Feng Kan wrote:
>>> The GIC-400 implementation allows for FIQ and IRQ bypass. In the
>>> X-Gene implementation, the FIQ bypass must be enabled at all time.
>>> Otherwise, some PPI will appear as FIQ and cause kernel problem.
>>
>> How comes? Are only PPIs affected? When you say "some PPIs", can you be
>> more specific?
>>
>>> Signed-off-by: Feng Kan
>>> ---
>>> drivers/irqchip/irq-gic.c | 15 +++
>>> include/linux/irqchip/arm-gic.h |4 ++--
>>> 2 files changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index d0e9480..aa7342e 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -65,6 +65,7 @@ struct gic_chip_data {
>>> #endif
>>> struct irq_domain *domain;
>>> unsigned int gic_irqs;
>>> + unsigned int bypass_flag;
>>> #ifdef CONFIG_GIC_NON_BANKED
>>> void __iomem *(*get_base)(union gic_base *);
>>> #endif
>>> @@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
>>> 4);
>>>
>>> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>>> - writel_relaxed(1, base + GIC_CPU_CTRL);
>>> + writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
>>> }
>>>
>>> void gic_cpu_if_down(void)
>>> @@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>>
>>> writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>>> - writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>>> + writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
>>> GIC_CPU_CTRL);
>>> }
>>>
>>> static int gic_notifier(struct notifier_block *self, unsigned long cmd,
>>> void *v)
>>> @@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
>>>
>>> void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>> void __iomem *dist_base, void __iomem *cpu_base,
>>> -u32 percpu_offset, struct device_node *node)
>>> +u32 percpu_offset, u32 bypass_val,
>>> +struct device_node *node)
>>> {
>>> irq_hw_number_t hwirq_base;
>>> struct gic_chip_data *gic;
>>> @@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
>>> irq_start,
>>>
>>> set_handle_irq(gic_handle_irq);
>>>
>>> + gic->bypass_flag = (bypass_val & 0xf) << 4;
>>
>> Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
>>
>>> gic_chip.flags |= gic_arch_extn.flags;
>>> gic_dist_init(gic);
>>> gic_cpu_init(gic);
>>> @@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
>>> device_node *parent)
>>> void __iomem *cpu_base;
>>> void __iomem *dist_base;
>>> u32 percpu_offset;
>>> + u32 bypass_val;
>>> int irq;
>>>
>>> if (WARN_ON(!node))
>>> @@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
>>> struct device_node *parent)
>>> if (of_property_read_u32(node, "cpu-offset", _offset))
>>> percpu_offset = 0;
>>>
>>> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
>>> + if (of_property_read_u32(node, "bypass-flags", _val))
>>> + bypass_val = 0;
>>
>> [adding Mark on Cc, so he can comment on the DT parts]
>>
>> Where's the DT documentation update? Also, as this is an
>> implementation-specific quirk, you should consider using a separate
>> compatible string and move the handling of this issue into some X-Gene
>> specific code.
>
> Adding separate compatible string for X-Gene specific GIC will break
> VGIC code for X-Gene because VGIC code looks for DT node compatible
> to "arm,cortex-a15-gic". We don't want to break currently working VGIC
> code for X-Gene.
>
> The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
> feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
> is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
> event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
> and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
> these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
> PPI28 (Legacy-FIQ).
>
> We should have more cleaner and optional device tree binding for GIC
> which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
> and FIQBypDisGrp1 bits for X-Gene.
>
> Regards,
> Anup
Adding Kumar Sankaran.
>
>>
>>> + gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
>>> bypass_val, node);
>>>
>>> if (parent) {
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier wrote:
> [dropping from the CC list, as someone seems to have
> tripped on the config file, and I'm tired of getting bounces]
>
> Feng,
>
> On 19/11/13 21:42, Feng Kan wrote:
>> The GIC-400 implementation allows for FIQ and IRQ bypass. In the
>> X-Gene implementation, the FIQ bypass must be enabled at all time.
>> Otherwise, some PPI will appear as FIQ and cause kernel problem.
>
> How comes? Are only PPIs affected? When you say "some PPIs", can you be
> more specific?
>
>> Signed-off-by: Feng Kan
>> ---
>> drivers/irqchip/irq-gic.c | 15 +++
>> include/linux/irqchip/arm-gic.h |4 ++--
>> 2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index d0e9480..aa7342e 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -65,6 +65,7 @@ struct gic_chip_data {
>> #endif
>> struct irq_domain *domain;
>> unsigned int gic_irqs;
>> + unsigned int bypass_flag;
>> #ifdef CONFIG_GIC_NON_BANKED
>> void __iomem *(*get_base)(union gic_base *);
>> #endif
>> @@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
>> 4);
>>
>> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>> - writel_relaxed(1, base + GIC_CPU_CTRL);
>> + writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
>> }
>>
>> void gic_cpu_if_down(void)
>> @@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>
>> writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>> - writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>> + writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
>> GIC_CPU_CTRL);
>> }
>>
>> static int gic_notifier(struct notifier_block *self, unsigned long cmd,
>> void *v)
>> @@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
>>
>> void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>> void __iomem *dist_base, void __iomem *cpu_base,
>> -u32 percpu_offset, struct device_node *node)
>> +u32 percpu_offset, u32 bypass_val,
>> +struct device_node *node)
>> {
>> irq_hw_number_t hwirq_base;
>> struct gic_chip_data *gic;
>> @@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
>> irq_start,
>>
>> set_handle_irq(gic_handle_irq);
>>
>> + gic->bypass_flag = (bypass_val & 0xf) << 4;
>
> Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
>
>> gic_chip.flags |= gic_arch_extn.flags;
>> gic_dist_init(gic);
>> gic_cpu_init(gic);
>> @@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
>> device_node *parent)
>> void __iomem *cpu_base;
>> void __iomem *dist_base;
>> u32 percpu_offset;
>> + u32 bypass_val;
>> int irq;
>>
>> if (WARN_ON(!node))
>> @@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node, struct
>> device_node *parent)
>> if (of_property_read_u32(node, "cpu-offset", _offset))
>> percpu_offset = 0;
>>
>> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
>> + if (of_property_read_u32(node, "bypass-flags", _val))
>> + bypass_val = 0;
>
> [adding Mark on Cc, so he can comment on the DT parts]
>
> Where's the DT documentation update? Also, as this is an
> implementation-specific quirk, you should consider using a separate
> compatible string and move the handling of this issue into some X-Gene
> specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to "arm,cortex-a15-gic". We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
Regards,
Anup
>
>> + gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
>> bypass_val, node);
>>
>> if (parent) {
>> irq = irq_of_parse_and_map(node, 0);
>> diff --git a/include/linux/irqchip/arm-gic.h
>> b/include/linux/irqchip/arm-gic.h
>> index 0e5d9ec..999515b 100644
>> --- a/include/linux/irqchip/arm-gic.h
>>
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
Regards,
Anup
+ gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
bypass_val, node);
if (parent) {
irq = irq_of_parse_and_map(node, 0);
diff --git a/include/linux/irqchip/arm-gic.h
b/include/linux/irqchip/arm-gic.h
index 0e5d9ec..999515b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -64,14 +64,14 @@ struct device_node;
extern struct irq_chip gic_arch_extn;
void gic_init_bases(unsigned
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
On Sat, Nov 23, 2013 at 2:11 PM, Anup Patel a...@brainfault.org wrote:
On Wed, Nov 20, 2013 at 4:28 PM, Marc Zyngier marc.zyng...@arm.com wrote:
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node,
struct device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Adding separate compatible string for X-Gene specific GIC will break
VGIC code for X-Gene because VGIC code looks for DT node compatible
to arm,cortex-a15-gic. We don't want to break currently working VGIC
code for X-Gene.
The Legacy-IRQ bypass disable and Legacy-FIQ bypass disable is a
feature of GIC-400 and its not X-Gene specific. The only difference in X-Gene
is that we use PPI31 (Legacy-IRQ) for timer and PPI28 (Legacy-FIQ) for perf
event. The issue is that IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits are 0 by default and for X-Gene we need to set
these bits to 1 so that GIC-400 does not bypass PPI31 (Legacy-IRQ) and
PPI28 (Legacy-FIQ).
We should have more cleaner and optional device tree binding for GIC
which can help us set IRQBypDisGrp0, FIQBypDisGrp0, IRQBypDisGrp1
and FIQBypDisGrp1 bits for X-Gene.
Regards,
Anup
Adding Kumar Sankaran.
+ gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
bypass_val, node);
if (parent) {
irq = irq_of_parse_and_map(node, 0);
diff --git a/include/linux/irqchip/arm-gic.h
b/include/linux/irqchip/arm-gic.h
index 0e5d9ec..999515b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
>> set_handle_irq(gic_handle_irq);
>>
>> + gic->bypass_flag = (bypass_val & 0xf) << 4;
>Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
The only time those bits are touched are when the dts is modified with the
bypass flag. Otherwise those bits remain untouched as before.
>> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
>> + if (of_property_read_u32(node, "bypass-flags", _val))
>> + bypass_val = 0;
>
> [adding Mark on Cc, so he can comment on the DT parts]
>
> Where's the DT documentation update? Also, as this is an
> implementation-specific quirk, you should consider using a separate
> compatible string and move the handling of this issue into some X-Gene
> specific code.
Yes, the only way to do that nicely would be to add another irq-xgene file.
Since I see little of gic specific implementation in the irq-gic file. To add
another irq-xgene file for one change seems excessive at the time. Given
that in v2 these bypass bits are added, is it better to check for v2 dts
property before modify the CPU_CTRL register?
>
>> + gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
>> bypass_val, node);
>>
>> if (parent) {
>> irq = irq_of_parse_and_map(node, 0);
>> diff --git a/include/linux/irqchip/arm-gic.h
>> b/include/linux/irqchip/arm-gic.h
>> index 0e5d9ec..999515b 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -64,14 +64,14 @@ struct device_node;
>> extern struct irq_chip gic_arch_extn;
>>
>> void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
>> - u32 offset, struct device_node *);
>> + u32 offset, u32 bypass_val, struct device_node *);
>> void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>> void gic_cpu_if_down(void);
>>
>> static inline void gic_init(unsigned int nr, int start,
>> void __iomem *dist , void __iomem *cpu)
>> {
>> - gic_init_bases(nr, start, dist, cpu, 0, NULL);
>> + gic_init_bases(nr, start, dist, cpu, 0, 0, NULL);
>> }
>>
>> #endif /* __ASSEMBLY */
>>
>
> Cheers,
>
> M.
> --
> Jazz is not dead. It just smells funny...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
The only time those bits are touched are when the dts is modified with the
bypass flag. Otherwise those bits remain untouched as before.
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
Yes, the only way to do that nicely would be to add another irq-xgene file.
Since I see little of gic specific implementation in the irq-gic file. To add
another irq-xgene file for one change seems excessive at the time. Given
that in v2 these bypass bits are added, is it better to check for v2 dts
property before modify the CPU_CTRL register?
+ gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
bypass_val, node);
if (parent) {
irq = irq_of_parse_and_map(node, 0);
diff --git a/include/linux/irqchip/arm-gic.h
b/include/linux/irqchip/arm-gic.h
index 0e5d9ec..999515b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -64,14 +64,14 @@ struct device_node;
extern struct irq_chip gic_arch_extn;
void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
- u32 offset, struct device_node *);
+ u32 offset, u32 bypass_val, struct device_node *);
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
void gic_cpu_if_down(void);
static inline void gic_init(unsigned int nr, int start,
void __iomem *dist , void __iomem *cpu)
{
- gic_init_bases(nr, start, dist, cpu, 0, NULL);
+ gic_init_bases(nr, start, dist, cpu, 0, 0, NULL);
}
#endif /* __ASSEMBLY */
Cheers,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
[dropping from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
> The GIC-400 implementation allows for FIQ and IRQ bypass. In the
> X-Gene implementation, the FIQ bypass must be enabled at all time.
> Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say "some PPIs", can you be
more specific?
> Signed-off-by: Feng Kan
> ---
> drivers/irqchip/irq-gic.c | 15 +++
> include/linux/irqchip/arm-gic.h |4 ++--
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index d0e9480..aa7342e 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -65,6 +65,7 @@ struct gic_chip_data {
> #endif
> struct irq_domain *domain;
> unsigned int gic_irqs;
> + unsigned int bypass_flag;
> #ifdef CONFIG_GIC_NON_BANKED
> void __iomem *(*get_base)(union gic_base *);
> #endif
> @@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
> 4);
>
> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
> - writel_relaxed(1, base + GIC_CPU_CTRL);
> + writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
> }
>
> void gic_cpu_if_down(void)
> @@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
> writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>
> writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
> - writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
> + writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
> GIC_CPU_CTRL);
> }
>
> static int gic_notifier(struct notifier_block *self, unsigned long cmd,
> void *v)
> @@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
>
> void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> void __iomem *dist_base, void __iomem *cpu_base,
> -u32 percpu_offset, struct device_node *node)
> +u32 percpu_offset, u32 bypass_val,
> +struct device_node *node)
> {
> irq_hw_number_t hwirq_base;
> struct gic_chip_data *gic;
> @@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
> irq_start,
>
> set_handle_irq(gic_handle_irq);
>
> + gic->bypass_flag = (bypass_val & 0xf) << 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
> gic_chip.flags |= gic_arch_extn.flags;
> gic_dist_init(gic);
> gic_cpu_init(gic);
> @@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
> device_node *parent)
> void __iomem *cpu_base;
> void __iomem *dist_base;
> u32 percpu_offset;
> + u32 bypass_val;
> int irq;
>
> if (WARN_ON(!node))
> @@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node, struct
> device_node *parent)
> if (of_property_read_u32(node, "cpu-offset", _offset))
> percpu_offset = 0;
>
> - gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
> + if (of_property_read_u32(node, "bypass-flags", _val))
> + bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
> + gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
> bypass_val, node);
>
> if (parent) {
> irq = irq_of_parse_and_map(node, 0);
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 0e5d9ec..999515b 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -64,14 +64,14 @@ struct device_node;
> extern struct irq_chip gic_arch_extn;
>
> void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
> - u32 offset, struct device_node *);
> + u32 offset, u32 bypass_val, struct device_node *);
> void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
> void gic_cpu_if_down(void);
>
> static inline void gic_init(unsigned int nr, int start,
> void __iomem *dist , void __iomem *cpu)
> {
> - gic_init_bases(nr, start, dist, cpu, 0, NULL);
> + gic_init_bases(nr, start, dist, cpu, 0, 0, NULL);
> }
>
> #endif /* __ASSEMBLY */
>
Cheers,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
[dropping patc...@apm.com from the CC list, as someone seems to have
tripped on the config file, and I'm tired of getting bounces]
Feng,
On 19/11/13 21:42, Feng Kan wrote:
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
How comes? Are only PPIs affected? When you say some PPIs, can you be
more specific?
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
-u32 percpu_offset, struct device_node *node)
+u32 percpu_offset, u32 bypass_val,
+struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
Beware, the top 2 bits are reserved on GICv1, and shouldn't be messed with.
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
[adding Mark on Cc, so he can comment on the DT parts]
Where's the DT documentation update? Also, as this is an
implementation-specific quirk, you should consider using a separate
compatible string and move the handling of this issue into some X-Gene
specific code.
+ gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
bypass_val, node);
if (parent) {
irq = irq_of_parse_and_map(node, 0);
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 0e5d9ec..999515b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -64,14 +64,14 @@ struct device_node;
extern struct irq_chip gic_arch_extn;
void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
- u32 offset, struct device_node *);
+ u32 offset, u32 bypass_val, struct device_node *);
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
void gic_cpu_if_down(void);
static inline void gic_init(unsigned int nr, int start,
void __iomem *dist , void __iomem *cpu)
{
- gic_init_bases(nr, start, dist, cpu, 0, NULL);
+ gic_init_bases(nr, start, dist, cpu, 0, 0, NULL);
}
#endif /* __ASSEMBLY */
Cheers,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
Signed-off-by: Feng Kan
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic->bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
- u32 percpu_offset, struct device_node *node)
+ u32 percpu_offset, u32 bypass_val,
+ struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic->bypass_flag = (bypass_val & 0xf) << 4;
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
if (of_property_read_u32(node, "cpu-offset", _offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, "bypass-flags", _val))
+ bypass_val = 0;
+
+ gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
bypass_val, node);
if (parent) {
irq = irq_of_parse_and_map(node, 0);
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 0e5d9ec..999515b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -64,14 +64,14 @@ struct device_node;
extern struct irq_chip gic_arch_extn;
void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
- u32 offset, struct device_node *);
+ u32 offset, u32 bypass_val, struct device_node *);
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
void gic_cpu_if_down(void);
static inline void gic_init(unsigned int nr, int start,
void __iomem *dist , void __iomem *cpu)
{
- gic_init_bases(nr, start, dist, cpu, 0, NULL);
+ gic_init_bases(nr, start, dist, cpu, 0, 0, NULL);
}
#endif /* __ASSEMBLY */
--
1.7.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] irq-gic: add capability to set bypass flag in GIC
The GIC-400 implementation allows for FIQ and IRQ bypass. In the
X-Gene implementation, the FIQ bypass must be enabled at all time.
Otherwise, some PPI will appear as FIQ and cause kernel problem.
Signed-off-by: Feng Kan f...@apm.com
---
drivers/irqchip/irq-gic.c | 15 +++
include/linux/irqchip/arm-gic.h |4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d0e9480..aa7342e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -65,6 +65,7 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
+ unsigned int bypass_flag;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
@@ -450,7 +451,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 /
4);
writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(gic-bypass_flag | 1, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -591,7 +592,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(gic_data[gic_nr].bypass_flag | 1, cpu_base +
GIC_CPU_CTRL);
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd,
void *v)
@@ -733,7 +734,8 @@ const struct irq_domain_ops gic_irq_domain_ops = {
void __init gic_init_bases(unsigned int gic_nr, int irq_start,
void __iomem *dist_base, void __iomem *cpu_base,
- u32 percpu_offset, struct device_node *node)
+ u32 percpu_offset, u32 bypass_val,
+ struct device_node *node)
{
irq_hw_number_t hwirq_base;
struct gic_chip_data *gic;
@@ -821,6 +823,7 @@ void __init gic_init_bases(unsigned int gic_nr, int
irq_start,
set_handle_irq(gic_handle_irq);
+ gic-bypass_flag = (bypass_val 0xf) 4;
gic_chip.flags |= gic_arch_extn.flags;
gic_dist_init(gic);
gic_cpu_init(gic);
@@ -835,6 +838,7 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
void __iomem *cpu_base;
void __iomem *dist_base;
u32 percpu_offset;
+ u32 bypass_val;
int irq;
if (WARN_ON(!node))
@@ -849,7 +853,10 @@ int __init gic_of_init(struct device_node *node, struct
device_node *parent)
if (of_property_read_u32(node, cpu-offset, percpu_offset))
percpu_offset = 0;
- gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+ if (of_property_read_u32(node, bypass-flags, bypass_val))
+ bypass_val = 0;
+
+ gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset,
bypass_val, node);
if (parent) {
irq = irq_of_parse_and_map(node, 0);
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 0e5d9ec..999515b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -64,14 +64,14 @@ struct device_node;
extern struct irq_chip gic_arch_extn;
void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
- u32 offset, struct device_node *);
+ u32 offset, u32 bypass_val, struct device_node *);
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
void gic_cpu_if_down(void);
static inline void gic_init(unsigned int nr, int start,
void __iomem *dist , void __iomem *cpu)
{
- gic_init_bases(nr, start, dist, cpu, 0, NULL);
+ gic_init_bases(nr, start, dist, cpu, 0, 0, NULL);
}
#endif /* __ASSEMBLY */
--
1.7.6.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
