* [PATCH] irqchip:gic: change access of gicc_ctrl register to read modify write. [not found] <1392761550-11662-1-git-send-email-fkan@apm.com> @ 2014-02-19 10:33 ` Marc Zyngier 0 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2014-02-19 10:33 UTC (permalink / raw) To: linux-arm-kernel Hi Feng, On 18/02/14 22:12, Feng Kan wrote: > This change is made to preserve the GIC v2 releated bits in the > GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec). > The original code only set the enable/disable group bit in this register. > This code will preserve all other bits configured by the bootload except This "all other bits" in itself is a major problem, see below. > the enable/disable bit. The main reason for this change is to allow the > bypass bits specified in the v2 spec to remain untouched by the current > GIC code. In the X-Gene platform, the bypass functionality is not used > and bypass must be disabled at all time. > > Signed-off-by: Vinayak Kale <vkale@apm.com> > Acked-by: Anup Patel <apatel@apm.com> > Signed-off-by: Feng Kan <fkan@apm.com> > --- > drivers/irqchip/irq-gic.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 341c601..9adc3e1 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic) > void __iomem *dist_base = gic_data_dist_base(gic); > void __iomem *base = gic_data_cpu_base(gic); > unsigned int cpu_mask, cpu = smp_processor_id(); > + unsigned int ctrl_mask; > int i; > > /* > @@ -449,13 +450,20 @@ 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); > + > + ctrl_mask = readl(base + GIC_CPU_CTRL); > + ctrl_mask |= 0x1; > + writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL); So what if the firmware used a different EOImode? We would end up in a situation where we don't deactivate the interrupts anymore. Not good. You should only preserve the bits that actually matter, and that won't have an impact with the way the kernel works. > } > > void gic_cpu_if_down(void) > { > + unsigned int ctrl_mask; > void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); > - writel_relaxed(0, cpu_base + GIC_CPU_CTRL); > + > + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); > + ctrl_mask &= 0xfffffffe; nit: use ctlr_mask &= ~1; in order to be consistent with the rest of the code. > + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); > } > > #ifdef CONFIG_CPU_PM > @@ -566,6 +574,7 @@ static void gic_cpu_restore(unsigned int gic_nr) > { > int i; > u32 *ptr; > + unsigned int ctrl_mask; > void __iomem *dist_base; > void __iomem *cpu_base; > > @@ -590,7 +599,10 @@ 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); > + > + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); > + ctrl_mask |= 0x1; > + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); > } > > static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) > Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] irqchip:gic: change access of gicc_ctrl register to read modify write. @ 2014-02-19 10:33 ` Marc Zyngier 0 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2014-02-19 10:33 UTC (permalink / raw) To: Feng Kan Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, tglx@linutronix.dev, patches@apm.com, Vinayak Kale Hi Feng, On 18/02/14 22:12, Feng Kan wrote: > This change is made to preserve the GIC v2 releated bits in the > GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec). > The original code only set the enable/disable group bit in this register. > This code will preserve all other bits configured by the bootload except This "all other bits" in itself is a major problem, see below. > the enable/disable bit. The main reason for this change is to allow the > bypass bits specified in the v2 spec to remain untouched by the current > GIC code. In the X-Gene platform, the bypass functionality is not used > and bypass must be disabled at all time. > > Signed-off-by: Vinayak Kale <vkale@apm.com> > Acked-by: Anup Patel <apatel@apm.com> > Signed-off-by: Feng Kan <fkan@apm.com> > --- > drivers/irqchip/irq-gic.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 341c601..9adc3e1 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic) > void __iomem *dist_base = gic_data_dist_base(gic); > void __iomem *base = gic_data_cpu_base(gic); > unsigned int cpu_mask, cpu = smp_processor_id(); > + unsigned int ctrl_mask; > int i; > > /* > @@ -449,13 +450,20 @@ 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); > + > + ctrl_mask = readl(base + GIC_CPU_CTRL); > + ctrl_mask |= 0x1; > + writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL); So what if the firmware used a different EOImode? We would end up in a situation where we don't deactivate the interrupts anymore. Not good. You should only preserve the bits that actually matter, and that won't have an impact with the way the kernel works. > } > > void gic_cpu_if_down(void) > { > + unsigned int ctrl_mask; > void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); > - writel_relaxed(0, cpu_base + GIC_CPU_CTRL); > + > + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); > + ctrl_mask &= 0xfffffffe; nit: use ctlr_mask &= ~1; in order to be consistent with the rest of the code. > + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); > } > > #ifdef CONFIG_CPU_PM > @@ -566,6 +574,7 @@ static void gic_cpu_restore(unsigned int gic_nr) > { > int i; > u32 *ptr; > + unsigned int ctrl_mask; > void __iomem *dist_base; > void __iomem *cpu_base; > > @@ -590,7 +599,10 @@ 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); > + > + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); > + ctrl_mask |= 0x1; > + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); > } > > static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) > Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip:gic: change access of gicc_ctrl register to read modify write. 2014-02-19 10:33 ` Marc Zyngier @ 2014-02-25 20:19 ` Feng Kan -1 siblings, 0 replies; 6+ messages in thread From: Feng Kan @ 2014-02-25 20:19 UTC (permalink / raw) To: linux-arm-kernel On Wed, Feb 19, 2014 at 2:33 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > Hi Feng, > > On 18/02/14 22:12, Feng Kan wrote: >> This change is made to preserve the GIC v2 releated bits in the >> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec). >> The original code only set the enable/disable group bit in this register. >> This code will preserve all other bits configured by the bootload except > > This "all other bits" in itself is a major problem, see below. > >> the enable/disable bit. The main reason for this change is to allow the >> bypass bits specified in the v2 spec to remain untouched by the current >> GIC code. In the X-Gene platform, the bypass functionality is not used >> and bypass must be disabled at all time. >> >> Signed-off-by: Vinayak Kale <vkale@apm.com> >> Acked-by: Anup Patel <apatel@apm.com> >> Signed-off-by: Feng Kan <fkan@apm.com> >> --- >> drivers/irqchip/irq-gic.c | 18 +++++++++++++++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >> index 341c601..9adc3e1 100644 >> --- a/drivers/irqchip/irq-gic.c >> +++ b/drivers/irqchip/irq-gic.c >> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic) >> void __iomem *dist_base = gic_data_dist_base(gic); >> void __iomem *base = gic_data_cpu_base(gic); >> unsigned int cpu_mask, cpu = smp_processor_id(); >> + unsigned int ctrl_mask; >> int i; >> >> /* >> @@ -449,13 +450,20 @@ 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); >> + >> + ctrl_mask = readl(base + GIC_CPU_CTRL); >> + ctrl_mask |= 0x1; >> + writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL); > > So what if the firmware used a different EOImode? We would end up in a > situation where we don't deactivate the interrupts anymore. Not good. Is there an case where the EOI mode usage would change on the fly? Bootloader's job would be to setup the bits so the kernel would work properly. Do you have a case in mind that would violate this setup? > > You should only preserve the bits that actually matter, and that won't > have an impact with the way the kernel works. > >> } >> >> void gic_cpu_if_down(void) >> { >> + unsigned int ctrl_mask; >> void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); >> - writel_relaxed(0, cpu_base + GIC_CPU_CTRL); >> + >> + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); >> + ctrl_mask &= 0xfffffffe; > > nit: use > > ctlr_mask &= ~1; > > in order to be consistent with the rest of the code. > >> + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); >> } >> >> #ifdef CONFIG_CPU_PM >> @@ -566,6 +574,7 @@ static void gic_cpu_restore(unsigned int gic_nr) >> { >> int i; >> u32 *ptr; >> + unsigned int ctrl_mask; >> void __iomem *dist_base; >> void __iomem *cpu_base; >> >> @@ -590,7 +599,10 @@ 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); >> + >> + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); >> + ctrl_mask |= 0x1; >> + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); >> } >> >> static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) >> > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] irqchip:gic: change access of gicc_ctrl register to read modify write. @ 2014-02-25 20:19 ` Feng Kan 0 siblings, 0 replies; 6+ messages in thread From: Feng Kan @ 2014-02-25 20:19 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, tglx@linutronix.dev, patches@apm.com, Vinayak Kale On Wed, Feb 19, 2014 at 2:33 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > Hi Feng, > > On 18/02/14 22:12, Feng Kan wrote: >> This change is made to preserve the GIC v2 releated bits in the >> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec). >> The original code only set the enable/disable group bit in this register. >> This code will preserve all other bits configured by the bootload except > > This "all other bits" in itself is a major problem, see below. > >> the enable/disable bit. The main reason for this change is to allow the >> bypass bits specified in the v2 spec to remain untouched by the current >> GIC code. In the X-Gene platform, the bypass functionality is not used >> and bypass must be disabled at all time. >> >> Signed-off-by: Vinayak Kale <vkale@apm.com> >> Acked-by: Anup Patel <apatel@apm.com> >> Signed-off-by: Feng Kan <fkan@apm.com> >> --- >> drivers/irqchip/irq-gic.c | 18 +++++++++++++++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >> index 341c601..9adc3e1 100644 >> --- a/drivers/irqchip/irq-gic.c >> +++ b/drivers/irqchip/irq-gic.c >> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic) >> void __iomem *dist_base = gic_data_dist_base(gic); >> void __iomem *base = gic_data_cpu_base(gic); >> unsigned int cpu_mask, cpu = smp_processor_id(); >> + unsigned int ctrl_mask; >> int i; >> >> /* >> @@ -449,13 +450,20 @@ 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); >> + >> + ctrl_mask = readl(base + GIC_CPU_CTRL); >> + ctrl_mask |= 0x1; >> + writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL); > > So what if the firmware used a different EOImode? We would end up in a > situation where we don't deactivate the interrupts anymore. Not good. Is there an case where the EOI mode usage would change on the fly? Bootloader's job would be to setup the bits so the kernel would work properly. Do you have a case in mind that would violate this setup? > > You should only preserve the bits that actually matter, and that won't > have an impact with the way the kernel works. > >> } >> >> void gic_cpu_if_down(void) >> { >> + unsigned int ctrl_mask; >> void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); >> - writel_relaxed(0, cpu_base + GIC_CPU_CTRL); >> + >> + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); >> + ctrl_mask &= 0xfffffffe; > > nit: use > > ctlr_mask &= ~1; > > in order to be consistent with the rest of the code. > >> + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); >> } >> >> #ifdef CONFIG_CPU_PM >> @@ -566,6 +574,7 @@ static void gic_cpu_restore(unsigned int gic_nr) >> { >> int i; >> u32 *ptr; >> + unsigned int ctrl_mask; >> void __iomem *dist_base; >> void __iomem *cpu_base; >> >> @@ -590,7 +599,10 @@ 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); >> + >> + ctrl_mask = readl(cpu_base + GIC_CPU_CTRL); >> + ctrl_mask |= 0x1; >> + writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL); >> } >> >> static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v) >> > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip:gic: change access of gicc_ctrl register to read modify write. 2014-02-25 20:19 ` Feng Kan @ 2014-02-26 10:05 ` Marc Zyngier -1 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2014-02-26 10:05 UTC (permalink / raw) To: linux-arm-kernel [Fixing tglx's email address so he too can enjoy the fun...] On 25/02/14 20:19, Feng Kan wrote: > On Wed, Feb 19, 2014 at 2:33 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> Hi Feng, >> >> On 18/02/14 22:12, Feng Kan wrote: >>> This change is made to preserve the GIC v2 releated bits in the >>> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec). >>> The original code only set the enable/disable group bit in this register. >>> This code will preserve all other bits configured by the bootload except >> >> This "all other bits" in itself is a major problem, see below. >> >>> the enable/disable bit. The main reason for this change is to allow the >>> bypass bits specified in the v2 spec to remain untouched by the current >>> GIC code. In the X-Gene platform, the bypass functionality is not used >>> and bypass must be disabled at all time. >>> >>> Signed-off-by: Vinayak Kale <vkale@apm.com> >>> Acked-by: Anup Patel <apatel@apm.com> >>> Signed-off-by: Feng Kan <fkan@apm.com> >>> --- >>> drivers/irqchip/irq-gic.c | 18 +++++++++++++++--- >>> 1 file changed, 15 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >>> index 341c601..9adc3e1 100644 >>> --- a/drivers/irqchip/irq-gic.c >>> +++ b/drivers/irqchip/irq-gic.c >>> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic) >>> void __iomem *dist_base = gic_data_dist_base(gic); >>> void __iomem *base = gic_data_cpu_base(gic); >>> unsigned int cpu_mask, cpu = smp_processor_id(); >>> + unsigned int ctrl_mask; >>> int i; >>> >>> /* >>> @@ -449,13 +450,20 @@ 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); >>> + >>> + ctrl_mask = readl(base + GIC_CPU_CTRL); >>> + ctrl_mask |= 0x1; >>> + writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL); >> >> So what if the firmware used a different EOImode? We would end up in a >> situation where we don't deactivate the interrupts anymore. Not good. > Is there an case where the EOI mode usage would change on the fly? > Bootloader's job would be to setup the bits so the kernel would work properly. If you trust bootloaders to get anything right, you shouldn't ever touch the Linux IRQ code. Since the ancient times, bootloaders have been known to be part of a conspiracy aiming to destroy Mankind and rule the Earth. Bootloaders are the Orcs of software. It's been scientifically proven that they are responsible for climate change. Seen the floods in England? Bootloaders. Exploding volcanoes? Bootloaders. The flat tire on your bike? Bootloaders. Enough said. > Do you have a case in mind that would violate this setup? What if your bootloader is actually a small RT kernel that uses interrupt nesting (hence EOImode==1 to be able to split deactivation from priority drop)? ECOS/RedBoot anyone? Do you think for a second that this thing will kindly reset EIOmode to zero just because it loads a Linux kernel? See the above. Remember this code is used on a lot of fairly insane platforms. You *must* be extremely conservative in any change you make. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] irqchip:gic: change access of gicc_ctrl register to read modify write. @ 2014-02-26 10:05 ` Marc Zyngier 0 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2014-02-26 10:05 UTC (permalink / raw) To: Feng Kan Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, patches@apm.com, Vinayak Kale [Fixing tglx's email address so he too can enjoy the fun...] On 25/02/14 20:19, Feng Kan wrote: > On Wed, Feb 19, 2014 at 2:33 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> Hi Feng, >> >> On 18/02/14 22:12, Feng Kan wrote: >>> This change is made to preserve the GIC v2 releated bits in the >>> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec). >>> The original code only set the enable/disable group bit in this register. >>> This code will preserve all other bits configured by the bootload except >> >> This "all other bits" in itself is a major problem, see below. >> >>> the enable/disable bit. The main reason for this change is to allow the >>> bypass bits specified in the v2 spec to remain untouched by the current >>> GIC code. In the X-Gene platform, the bypass functionality is not used >>> and bypass must be disabled at all time. >>> >>> Signed-off-by: Vinayak Kale <vkale@apm.com> >>> Acked-by: Anup Patel <apatel@apm.com> >>> Signed-off-by: Feng Kan <fkan@apm.com> >>> --- >>> drivers/irqchip/irq-gic.c | 18 +++++++++++++++--- >>> 1 file changed, 15 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >>> index 341c601..9adc3e1 100644 >>> --- a/drivers/irqchip/irq-gic.c >>> +++ b/drivers/irqchip/irq-gic.c >>> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic) >>> void __iomem *dist_base = gic_data_dist_base(gic); >>> void __iomem *base = gic_data_cpu_base(gic); >>> unsigned int cpu_mask, cpu = smp_processor_id(); >>> + unsigned int ctrl_mask; >>> int i; >>> >>> /* >>> @@ -449,13 +450,20 @@ 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); >>> + >>> + ctrl_mask = readl(base + GIC_CPU_CTRL); >>> + ctrl_mask |= 0x1; >>> + writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL); >> >> So what if the firmware used a different EOImode? We would end up in a >> situation where we don't deactivate the interrupts anymore. Not good. > Is there an case where the EOI mode usage would change on the fly? > Bootloader's job would be to setup the bits so the kernel would work properly. If you trust bootloaders to get anything right, you shouldn't ever touch the Linux IRQ code. Since the ancient times, bootloaders have been known to be part of a conspiracy aiming to destroy Mankind and rule the Earth. Bootloaders are the Orcs of software. It's been scientifically proven that they are responsible for climate change. Seen the floods in England? Bootloaders. Exploding volcanoes? Bootloaders. The flat tire on your bike? Bootloaders. Enough said. > Do you have a case in mind that would violate this setup? What if your bootloader is actually a small RT kernel that uses interrupt nesting (hence EOImode==1 to be able to split deactivation from priority drop)? ECOS/RedBoot anyone? Do you think for a second that this thing will kindly reset EIOmode to zero just because it loads a Linux kernel? See the above. Remember this code is used on a lot of fairly insane platforms. You *must* be extremely conservative in any change you make. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-26 10:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1392761550-11662-1-git-send-email-fkan@apm.com>
2014-02-19 10:33 ` [PATCH] irqchip:gic: change access of gicc_ctrl register to read modify write Marc Zyngier
2014-02-19 10:33 ` Marc Zyngier
2014-02-25 20:19 ` Feng Kan
2014-02-25 20:19 ` Feng Kan
2014-02-26 10:05 ` Marc Zyngier
2014-02-26 10:05 ` Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.