Thanks a lot! On Sat, Feb 27, 2016 at 8:28 PM, Ben Gras <b...@shrike-systems.com> wrote: > All, > > I tested this on current HEAD and verified it fixes a case I'd ran > into trouble with before, namely gpio-triggered irqs. Great to see. I > pushed your change with a commit message based on your original email, > Martin. I hope you like it. > > Thank you! > > > On Sat, Feb 27, 2016 at 10:16 PM, Ben Gras <b...@shrike-systems.com> wrote: >> I'm doing a rebase & build right now, thanks for the reminder. >> >> On Fri, Feb 26, 2016 at 10:32 PM, Martin Galvan >> <martin.gal...@tallertechnologies.com> wrote: >>> Hi Ben! Sorry to bother, were you able to test my changes to the BBB >>> interrupt handler? >>> >>> On Fri, Feb 12, 2016 at 11:09 AM, Martin Galvan >>> <martin.gal...@tallertechnologies.com> wrote: >>>> Thanks Ben! Indeed, any further testing is more than welcome :) >>>> >>>> On Thu, Feb 11, 2016 at 7:55 PM, Ben Gras <b...@shrike-systems.com> wrote: >>>>> This looks like great work. Please let me test it (I'll try the GPIO >>>>> interrupt trigger) & I'll merge it as soon as I have time. >>>>> >>>>> Thank you! >>>>> >>>>> Cheers, >>>>> Ben >>>>> >>>>> >>>>> On Thu, Feb 11, 2016 at 3:27 PM, Martin Galvan >>>>> <martin.gal...@tallertechnologies.com> wrote: >>>>>> This patch makes the following changes to the Beaglebone IRQ handling >>>>>> code: >>>>>> >>>>>> - Disable support for nested interrupts. >>>>>> - Detect spurious IRQs using the SPURIOUSIRQ field of the INTC_SIR_IRQ >>>>>> register. >>>>>> - Acknowledge spurious IRQs by setting the NewIRQAgr bit of the >>>>>> INTC_CONTROL >>>>>> register. This cleans the SPURIOUSIRQ field and allows new interrupts >>>>>> to be generated. >>>>>> - Improve the get_mir_reg function a bit. >>>>>> >>>>>> The Beaglebone bsp_interrupt_dispach function has been troublesome for a >>>>>> while now. >>>>>> We've seen it break the GPIO API >>>>>> (https://lists.rtems.org/pipermail/devel/2015-November/012995.html), >>>>>> the RTEMS interrupt server >>>>>> (https://lists.rtems.org/pipermail/devel/2015-July/011865.html), >>>>>> and now we've been getting spurious interrupts when trying to use the >>>>>> I2C module. >>>>>> >>>>>> We've done a lot of testing and concluded that the cause of most of >>>>>> these issues >>>>>> is the way nested interrupts are being handled. The AM335X manual states >>>>>> that >>>>>> the interrupt handling sequence should be as follows: >>>>>> >>>>>> 1. Identify the IRQ source by reading the ActiveIRQ field of the >>>>>> INTC_SIR_IRQ >>>>>> register. >>>>>> 2. Jump to the corresponding IRQ handler, which should serve the IRQ and >>>>>> deassert the interrupt condition at the peripheral side. >>>>>> 3. Enable the processing of new IRQs at the Interrupt Controller side by >>>>>> setting >>>>>> the NewIRQAgr bit of the INTC_CONTROL register. >>>>>> 4. Finally, enable IRQs at the CPU side. This is done later in >>>>>> _ARMV4_Exception_interrupt. >>>>>> >>>>>> Right now the Beaglebone bsp_interrupt_dispach enables IRQs at the INTC >>>>>> and CPU >>>>>> before jumping to the interrupt handler to allow for nested IRQs. >>>>>> Before doing so, it calls bsp_interrupt_disable to mask the IRQ source >>>>>> and avoid >>>>>> having it constantly fire new IRQs. After it's done it re-enables the IRQ >>>>>> by calling bsp_interrupt_enable. These calls break the GPIO API and the >>>>>> RTEMS interrupt server machinery, and we suspect it's also causing the >>>>>> spurious >>>>>> interrupts we saw with the I2C module. >>>>>> >>>>>> The correct way to enable interrupt nesting according to both the manual >>>>>> and >>>>>> the AM335X StarterWare code is to allow only interrupts of a higher >>>>>> priority >>>>>> to preempt the current one. This can be achieved by setting the >>>>>> INTC_THRESHOLD >>>>>> register to the priority of the current IRQ. However, in our case this >>>>>> isn't >>>>>> necessary since all the interrupt priorities are set to 0 (the highest >>>>>> possible) >>>>>> in bsp_interrupt_facility_initialize. We may implement this in a future >>>>>> patch, >>>>>> if required. >>>>>> >>>>>> We've tested this quite extensively on a number of different >>>>>> applications, and >>>>>> it's working fine. >>>>>> >>>>>> Closes #2580. >>>>>> --- >>>>>> c/src/lib/libbsp/arm/beagle/irq.c | 82 >>>>>> +++++++++++++++-------------- >>>>>> c/src/lib/libcpu/arm/shared/include/omap3.h | 3 +- >>>>>> 2 files changed, 44 insertions(+), 41 deletions(-) >>>>>> >>>>>> diff --git a/c/src/lib/libbsp/arm/beagle/irq.c >>>>>> b/c/src/lib/libbsp/arm/beagle/irq.c >>>>>> index c6485cd..d080a5e 100644 >>>>>> --- a/c/src/lib/libbsp/arm/beagle/irq.c >>>>>> +++ b/c/src/lib/libbsp/arm/beagle/irq.c >>>>>> @@ -18,6 +18,7 @@ >>>>>> #include <bsp.h> >>>>>> #include <bsp/irq-generic.h> >>>>>> #include <bsp/linker-symbols.h> >>>>>> +#include <bsp/fatal.h> >>>>>> >>>>>> #include <rtems/score/armv4.h> >>>>>> >>>>>> @@ -43,77 +44,78 @@ static struct omap_intr omap_intr = { >>>>>> }; >>>>>> #endif >>>>>> >>>>>> -static int irqs_enabled[BSP_INTERRUPT_VECTOR_MAX+1]; >>>>>> +/* Enables interrupts at the Interrupt Controller side. */ >>>>>> +static inline void omap_irq_ack(void) >>>>>> +{ >>>>>> + mmio_write(omap_intr.base + OMAP3_INTCPS_CONTROL, >>>>>> OMAP3_INTR_NEWIRQAGR); >>>>>> >>>>>> -volatile static int level = 0; >>>>>> + /* Flush data cache to make sure all the previous writes are done >>>>>> + before re-enabling interrupts. */ >>>>>> + flush_data_cache(); >>>>>> +} >>>>>> >>>>>> void bsp_interrupt_dispatch(void) >>>>>> { >>>>>> - /* get irq */ >>>>>> - uint32_t reg = mmio_read(omap_intr.base + OMAP3_INTCPS_SIR_IRQ); >>>>>> - int irq; >>>>>> - irq = reg & OMAP3_INTR_ACTIVEIRQ_MASK; >>>>>> + const uint32_t reg = mmio_read(omap_intr.base + OMAP3_INTCPS_SIR_IRQ); >>>>>> >>>>>> - if(!irqs_enabled[irq]) { >>>>>> - /* Ignore spurious interrupt */ >>>>>> - } else { >>>>>> - bsp_interrupt_vector_disable(irq); >>>>>> - >>>>>> - /* enable new interrupts, and flush data cache to make sure >>>>>> - * it hits the intc >>>>>> - */ >>>>>> - mmio_write(omap_intr.base + OMAP3_INTCPS_CONTROL, >>>>>> OMAP3_INTR_NEWIRQAGR); >>>>>> - flush_data_cache(); >>>>>> - mmio_read(omap_intr.base + OMAP3_INTCPS_SIR_IRQ); >>>>>> - flush_data_cache(); >>>>>> - >>>>>> - /* keep current irq masked but enable unmasked ones */ >>>>>> - uint32_t psr = _ARMV4_Status_irq_enable(); >>>>>> - bsp_interrupt_handler_dispatch(irq); >>>>>> - >>>>>> - _ARMV4_Status_restore(psr); >>>>>> + if ((reg & OMAP3_INTR_SPURIOUSIRQ_MASK) != >>>>>> OMAP3_INTR_SPURIOUSIRQ_MASK) { >>>>>> + const rtems_vector_number irq = reg & OMAP3_INTR_ACTIVEIRQ_MASK; >>>>>> >>>>>> - bsp_interrupt_vector_enable(irq); >>>>>> + bsp_interrupt_handler_dispatch(irq); >>>>>> + } else { >>>>>> + /* Ignore spurious interrupts. We'll still ACK it so new interrupts >>>>>> + can be generated. */ >>>>>> } >>>>>> + >>>>>> + omap_irq_ack(); >>>>>> } >>>>>> >>>>>> -static uint32_t get_mir_reg(int vector, uint32_t *mask) >>>>>> +/* There are 4 32-bit interrupt mask registers for a total of 128 >>>>>> interrupts. >>>>>> + The IRQ number tells us which register to use. */ >>>>>> +static uint32_t omap_get_mir_reg(rtems_vector_number vector, uint32_t >>>>>> *const mask) >>>>>> { >>>>>> - *mask = 1UL << (vector % 32); >>>>>> - >>>>>> - if(vector < 0) while(1) ; >>>>>> - if(vector < 32) return OMAP3_INTCPS_MIR0; >>>>>> - if(vector < 64) return OMAP3_INTCPS_MIR1; >>>>>> - if(vector < 96) return OMAP3_INTCPS_MIR2; >>>>>> - if(vector < 128) return OMAP3_INTCPS_MIR3; >>>>>> - while(1) ; >>>>>> + uint32_t mir_reg; >>>>>> + >>>>>> + /* Select which bit to set/clear in the MIR register. */ >>>>>> + *mask = 1ul << (vector % 32u); >>>>>> + >>>>>> + if (vector < 32u) { >>>>>> + mir_reg = OMAP3_INTCPS_MIR0; >>>>>> + } else if (vector < 64u) { >>>>>> + mir_reg = OMAP3_INTCPS_MIR1; >>>>>> + } else if (vector < 96u) { >>>>>> + mir_reg = OMAP3_INTCPS_MIR2; >>>>>> + } else if (vector < 128u) { >>>>>> + mir_reg = OMAP3_INTCPS_MIR3; >>>>>> + } else { >>>>>> + /* Invalid IRQ number. This should never happen. */ >>>>>> + bsp_fatal(0); >>>>>> + } >>>>>> + >>>>>> + return mir_reg; >>>>>> } >>>>>> >>>>>> rtems_status_code bsp_interrupt_vector_enable(rtems_vector_number >>>>>> vector) >>>>>> { >>>>>> uint32_t mask, cur; >>>>>> - uint32_t mir_reg = get_mir_reg(vector, &mask); >>>>>> + uint32_t mir_reg = omap_get_mir_reg(vector, &mask); >>>>>> >>>>>> cur = mmio_read(omap_intr.base + mir_reg); >>>>>> mmio_write(omap_intr.base + mir_reg, cur & ~mask); >>>>>> flush_data_cache(); >>>>>> >>>>>> - irqs_enabled[vector] = 1; >>>>>> - >>>>>> return RTEMS_SUCCESSFUL; >>>>>> } >>>>>> >>>>>> rtems_status_code bsp_interrupt_vector_disable(rtems_vector_number >>>>>> vector) >>>>>> { >>>>>> uint32_t mask, cur; >>>>>> - uint32_t mir_reg = get_mir_reg(vector, &mask); >>>>>> + uint32_t mir_reg = omap_get_mir_reg(vector, &mask); >>>>>> >>>>>> cur = mmio_read(omap_intr.base + mir_reg); >>>>>> mmio_write(omap_intr.base + mir_reg, cur | mask); >>>>>> flush_data_cache(); >>>>>> >>>>>> - irqs_enabled[vector] = 0; >>>>>> - >>>>>> return RTEMS_SUCCESSFUL; >>>>>> } >>>>>> >>>>>> diff --git a/c/src/lib/libcpu/arm/shared/include/omap3.h >>>>>> b/c/src/lib/libcpu/arm/shared/include/omap3.h >>>>>> index f28e5e5..0cc43d6 100644 >>>>>> --- a/c/src/lib/libcpu/arm/shared/include/omap3.h >>>>>> +++ b/c/src/lib/libcpu/arm/shared/include/omap3.h >>>>>> @@ -72,7 +72,8 @@ >>>>>> #define OMAP3_INTR_ILR(base,m) \ >>>>>> (base + OMAP3_INTCPS_ILR0 + 0x4 * (m)) >>>>>> >>>>>> -#define OMAP3_INTR_ACTIVEIRQ_MASK 0x7f /* Active IRQ mask for SIR_IRQ */ >>>>>> +#define OMAP3_INTR_SPURIOUSIRQ_MASK (0x1FFFFFF << 7) /* Spurious IRQ >>>>>> mask for SIR_IRQ */ >>>>>> +#define OMAP3_INTR_ACTIVEIRQ_MASK 0x7F /* Active IRQ mask for SIR_IRQ */ >>>>>> #define OMAP3_INTR_NEWIRQAGR 0x1 /* New IRQ Generation */ >>>>>> >>>>>> #define OMAP3_DM337X_NR_IRQ_VECTORS 96 >>>>>> -- >>>>>> 2.7.1 >>>>>> >>>> >>>> >>>> >>>> -- >>>> >>>> >>>> Martin Galvan >>>> >>>> Software Engineer >>>> >>>> Taller Technologies Argentina >>>> >>>> >>>> San Lorenzo 47, 3rd Floor, Office 5 >>>> >>>> Córdoba, Argentina >>>> >>>> Phone: 54 351 4217888 / +54 351 4218211 >>> >>> >>> >>> -- >>> >>> >>> Martin Galvan >>> >>> Software Engineer >>> >>> Taller Technologies Argentina >>> >>> >>> San Lorenzo 47, 3rd Floor, Office 5 >>> >>> Córdoba, Argentina >>> >>> Phone: 54 351 4217888 / +54 351 4218211
-- Martin Galvan Software Engineer Taller Technologies Argentina San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: 54 351 4217888 / +54 351 4218211 _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
