Fix all Cortex-M3/4 implementations of up_disable_irq(). They were doing nothing. Thanks to Manuel Stühn for the tip.

This commit is contained in:
Gregory Nutt 2014-01-15 09:56:30 -06:00
parent 836057e340
commit be474523c4
7 changed files with 226 additions and 62 deletions

View File

@ -6447,9 +6447,21 @@
done for all FAT types. From Tridge via Lorenz Meier
(2014-1-14).
* arch/arm/src/armv6-m/up_doirq.c and armv7-m/up_doirq.c and all
implementations of up_mask_acq() for all Cortex-M architectures: Do
implementations of up_maskack_irq() for all Cortex-M architectures: Do
not disable and enable the IRQ on each interrupt. Because (1)
interrupts are already disabled on interrupt entry, (2) this
interferes with controlling the IRQ interrrupt setting from interrupt
handlers, and (3) up_disable_irq() does not work anyway so that this
has never done anything (2014-1-15).
* All implementations of up_disable_irq() for all Cortex-M3 and M4
architectures: To enable an interrupt on the Cortex-M3/4 CPU, you
need to set a bit in the ISER register. To disable the interrupt, you
need to set a bit in the ICER register. Existing logic was trying to
disable interrupts by clearing the bit in the ISER register. That will
not work; writing a '0' to the ISER register has no effect. That
means that up_disable_irq() was doing nothing! It turns out that that
is not really important because up_disable_irq() is not really used
for that purpose. But some spurions STM32 ADC interrupts have been
reported to me and this turned out to be the cause in that case. My
concern not that up_disable_irq() works is that there may now be
unmasked bugs that leave devices in the disabled state? (2014-1-15).

View File

@ -1,7 +1,7 @@
/****************************************************************************
* arch/arm/src/lpc17/kinetis_irq.c
*
* Copyright (C) 2011, 2013 Gregory Nutt. All rights reserved.
* Copyright (C) 2011, 2013-14 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gnutt@nuttx.org>
*
* Redistribution and use in source and binary forms, with or without
@ -65,6 +65,13 @@
NVIC_SYSH_PRIORITY_DEFAULT << 8 |\
NVIC_SYSH_PRIORITY_DEFAULT)
/* Given the address of a NVIC ENABLE register, this is the offset to
* the corresponding CLEAR ENABLE register.
*/
#define NVIC_ENA_OFFSET (0)
#define NVIC_CLRENA_OFFSET (NVIC_IRQ0_31_CLEAR - NVIC_IRQ0_31_ENABLE)
/****************************************************************************
* Public Data
****************************************************************************/
@ -232,7 +239,8 @@ static inline void kinetis_prioritize_syscall(int priority)
*
****************************************************************************/
static int kinetis_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
static int kinetis_irqinfo(int irq, uintptr_t *regaddr, uint32_t *bit,
uintptr_t offset)
{
DEBUGASSERT(irq >= KINETIS_IRQ_NMI && irq < NR_IRQS);
@ -242,22 +250,22 @@ static int kinetis_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
{
if (irq < (KINETIS_IRQ_EXTINT+32))
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << (irq - KINETIS_IRQ_EXTINT);
}
else if (irq < (KINETIS_IRQ_EXTINT+64))
{
*regaddr = NVIC_IRQ32_63_ENABLE;
*regaddr = (NVIC_IRQ32_63_ENABLE + offset);
*bit = 1 << (irq - KINETIS_IRQ_EXTINT - 32);
}
else if (irq < (KINETIS_IRQ_EXTINT+96))
{
*regaddr = NVIC_IRQ64_95_ENABLE;
*regaddr = (NVIC_IRQ64_95_ENABLE + offset);
*bit = 1 << (irq - KINETIS_IRQ_EXTINT - 64);
}
else if (irq < NR_IRQS)
{
*regaddr = NVIC_IRQ96_127_ENABLE;
*regaddr = (NVIC_IRQ96_127_ENABLE + offset);
*bit = 1 << (irq - KINETIS_IRQ_EXTINT - 96);
}
else
@ -436,18 +444,36 @@ void up_irqinitialize(void)
void up_disable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (kinetis_irqinfo(irq, &regaddr, &bit) == 0)
if (kinetis_irqinfo(irq, &regaddr, &bit, NVIC_CLRENA_OFFSET) == 0)
{
/* Clear the appropriate bit in the register to enable the interrupt */
/* Modify the appropriate bit in the register to disable the interrupt */
regval = getreg32(regaddr);
regval &= ~bit;
/* This is awkward... For normal interrupts, we need to set the bit
* in the associated Interrupt Clear Enable register. For other
* exceptions, we need to clear the bit in the System Handler Control
* and State Register.
*/
if (irq >= KINETIS_IRQ_EXTINT)
{
regval |= bit;
}
else
{
regval &= ~bit;
}
/* Save the appropriately modified register */
putreg32(regval, regaddr);
}
kinetis_dumpnvic("disable", irq);
}
@ -461,11 +487,11 @@ void up_disable_irq(int irq)
void up_enable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (kinetis_irqinfo(irq, &regaddr, &bit) == 0)
if (kinetis_irqinfo(irq, &regaddr, &bit, NVIC_ENA_OFFSET) == 0)
{
/* Set the appropriate bit in the register to enable the interrupt */
@ -473,6 +499,7 @@ void up_enable_irq(int irq)
regval |= bit;
putreg32(regval, regaddr);
}
kinetis_dumpnvic("enable", irq);
}

View File

@ -68,6 +68,13 @@
NVIC_SYSH_PRIORITY_DEFAULT << 8 |\
NVIC_SYSH_PRIORITY_DEFAULT)
/* Given the address of a NVIC ENABLE register, this is the offset to
* the corresponding CLEAR ENABLE register.
*/
#define NVIC_ENA_OFFSET (0)
#define NVIC_CLRENA_OFFSET (NVIC_IRQ0_31_CLEAR - NVIC_IRQ0_31_ENABLE)
/****************************************************************************
* Public Data
****************************************************************************/
@ -217,7 +224,8 @@ static inline void lm_prioritize_syscall(int priority)
*
****************************************************************************/
static int lm_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
static int lm_irqinfo(int irq, uintptr_t *regaddr, uint32_t *bit,
uintptr_t offset)
{
DEBUGASSERT(irq >= LM_IRQ_NMI && irq < NR_IRQS);
@ -227,12 +235,12 @@ static int lm_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
{
if (irq < LM_IRQ_INTERRUPTS + 32)
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << (irq - LM_IRQ_INTERRUPTS);
}
else if (irq < NR_IRQS)
{
*regaddr = NVIC_IRQ32_63_ENABLE;
*regaddr = (NVIC_IRQ32_63_ENABLE + offset);
*bit = 1 << (irq - LM_IRQ_INTERRUPTS - 32);
}
else
@ -390,16 +398,33 @@ void up_irqinitialize(void)
void up_disable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (lm_irqinfo(irq, &regaddr, &bit) == 0)
if (lm_irqinfo(irq, &regaddr, &bit, NVIC_CLRENA_OFFSET) == 0)
{
/* Clear the appropriate bit in the register to enable the interrupt */
/* Modify the appropriate bit in the register to disable the interrupt */
regval = getreg32(regaddr);
regval &= ~bit;
/* This is awkward... For normal interrupts, we need to set the bit
* in the associated Interrupt Clear Enable register. For other
* exceptions, we need to clear the bit in the System Handler Control
* and State Register.
*/
if (irq >= LM_IRQ_INTERRUPTS)
{
regval |= bit;
}
else
{
regval &= ~bit;
}
/* Save the appropriately modified register */
putreg32(regval, regaddr);
}
@ -416,11 +441,11 @@ void up_disable_irq(int irq)
void up_enable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (lm_irqinfo(irq, &regaddr, &bit) == 0)
if (lm_irqinfo(irq, &regaddr, &bit, NVIC_ENA_OFFSET) == 0)
{
/* Set the appropriate bit in the register to enable the interrupt */

View File

@ -66,6 +66,13 @@
NVIC_SYSH_PRIORITY_DEFAULT << 8 |\
NVIC_SYSH_PRIORITY_DEFAULT)
/* Given the address of a NVIC ENABLE register, this is the offset to
* the corresponding CLEAR ENABLE register.
*/
#define NVIC_ENA_OFFSET (0)
#define NVIC_CLRENA_OFFSET (NVIC_IRQ0_31_CLEAR - NVIC_IRQ0_31_ENABLE)
/****************************************************************************
* Public Data
****************************************************************************/
@ -214,7 +221,8 @@ static inline void lpc17_prioritize_syscall(int priority)
*
****************************************************************************/
static int lpc17_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
static int lpc17_irqinfo(int irq, uintptr_t *regaddr, uint32_t *bit,
uintptr_t offset)
{
DEBUGASSERT(irq >= LPC17_IRQ_NMI && irq < NR_IRQS);
@ -224,12 +232,12 @@ static int lpc17_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
{
if (irq < (LPC17_IRQ_EXTINT+32))
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << (irq - LPC17_IRQ_EXTINT);
}
else if (irq < LPC17_IRQ_NIRQS)
{
*regaddr = NVIC_IRQ32_63_ENABLE;
*regaddr = (NVIC_IRQ32_63_ENABLE + offset);
*bit = 1 << (irq - LPC17_IRQ_EXTINT - 32);
}
else
@ -383,16 +391,33 @@ void up_irqinitialize(void)
void up_disable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (lpc17_irqinfo(irq, &regaddr, &bit) == 0)
{
/* Clear the appropriate bit in the register to enable the interrupt */
/* Modify the appropriate bit in the register to disable the interrupt */
regval = getreg32(regaddr);
regval &= ~bit;
/* This is awkward... For normal interrupts, we need to set the bit
* in the associated Interrupt Clear Enable register. For other
* exceptions, we need to clear the bit in the System Handler Control
* and State Register.
*/
if (irq >= LPC17_IRQ_EXTINT)
{
regval |= bit;
}
else
{
regval &= ~bit;
}
/* Save the appropriately modified register */
putreg32(regval, regaddr);
}
#ifdef CONFIG_GPIO_IRQ
@ -417,11 +442,11 @@ void up_disable_irq(int irq)
void up_enable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (lpc17_irqinfo(irq, &regaddr, &bit) == 0)
if (lpc17_irqinfo(irq, &regaddr, &bit, NVIC_ENA_OFFSET) == 0)
{
/* Set the appropriate bit in the register to enable the interrupt */

View File

@ -67,6 +67,13 @@
LPC43M4_SYSH_PRIORITY_DEFAULT << 8 |\
LPC43M4_SYSH_PRIORITY_DEFAULT)
/* Given the address of a NVIC ENABLE register, this is the offset to
* the corresponding CLEAR ENABLE register.
*/
#define NVIC_ENA_OFFSET (0)
#define NVIC_CLRENA_OFFSET (NVIC_IRQ0_31_CLEAR - NVIC_IRQ0_31_ENABLE)
/****************************************************************************
* Public Data
****************************************************************************/
@ -223,7 +230,8 @@ static inline void lpc43_prioritize_syscall(int priority)
*
****************************************************************************/
static int lpc43_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
static int lpc43_irqinfo(int irq, uintptr_t *regaddr, uint32_t *bit,
uintptr_t offset)
{
DEBUGASSERT(irq >= LPC43_IRQ_NMI && irq < NR_IRQS);
@ -233,12 +241,12 @@ static int lpc43_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
{
if (irq < (LPC43_IRQ_EXTINT + 32))
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << (irq - LPC43_IRQ_EXTINT);
}
else if (irq < LPC43M4_IRQ_NIRQS)
{
*regaddr = NVIC_IRQ32_63_ENABLE;
*regaddr = (NVIC_IRQ32_63_ENABLE + offset);
*bit = 1 << (irq - LPC43_IRQ_EXTINT - 32);
}
else
@ -421,16 +429,33 @@ void up_irqinitialize(void)
void up_disable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (lpc43_irqinfo(irq, &regaddr, &bit) == 0)
if (lpc43_irqinfo(irq, &regaddr, &bit, NVIC_CLRENA_OFFSET) == 0)
{
/* Clear the appropriate bit in the register to enable the interrupt */
/* Modify the appropriate bit in the register to disable the interrupt */
regval = getreg32(regaddr);
regval &= ~bit;
/* This is awkward... For normal interrupts, we need to set the bit
* in the associated Interrupt Clear Enable register. For other
* exceptions, we need to clear the bit in the System Handler Control
* and State Register.
*/
if (irq >= LPC43_IRQ_EXTINT)
{
regval |= bit;
}
else
{
regval &= ~bit;
}
/* Save the appropriately modified register */
putreg32(regval, regaddr);
}
#ifdef CONFIG_GPIO_IRQ
@ -455,11 +480,11 @@ void up_disable_irq(int irq)
void up_enable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (lpc43_irqinfo(irq, &regaddr, &bit) == 0)
if (lpc43_irqinfo(irq, &regaddr, &bit, NVIC_ENA_OFFSET) == 0)
{
/* Set the appropriate bit in the register to enable the interrupt */

View File

@ -68,6 +68,13 @@
NVIC_SYSH_PRIORITY_DEFAULT << 8 |\
NVIC_SYSH_PRIORITY_DEFAULT)
/* Given the address of a NVIC ENABLE register, this is the offset to
* the corresponding CLEAR ENABLE register.
*/
#define NVIC_ENA_OFFSET (0)
#define NVIC_CLRENA_OFFSET (NVIC_IRQ0_31_CLEAR - NVIC_IRQ0_31_ENABLE)
/****************************************************************************
* Public Data
****************************************************************************/
@ -215,7 +222,8 @@ static inline void sam_prioritize_syscall(int priority)
*
****************************************************************************/
static int sam_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
static int sam_irqinfo(int irq, uintptr_t *regaddr, uint32_t *bit,
uintptr_t offset)
{
unsigned int extint = irq - SAM_IRQ_EXTINT;
@ -228,36 +236,36 @@ static int sam_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
#if SAM_IRQ_NEXTINT <= 32
if (extint < SAM_IRQ_NEXTINT)
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << extint;
}
else
#elif SAM_IRQ_NEXTINT <= 64
if (extint < 32)
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << extint;
}
else if (extint < SAM_IRQ_NEXTINT)
{
*regaddr = NVIC_IRQ32_63_ENABLE;
*regaddr = (NVIC_IRQ32_63_ENABLE + offset);
*bit = 1 << (extint - 32);
}
else
#elif SAM_IRQ_NEXTINT <= 96
if (extint < 32)
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << extint;
}
else if (extint < 64)
{
*regaddr = NVIC_IRQ32_63_ENABLE;
*regaddr = (NVIC_IRQ32_63_ENABLE + offset);
*bit = 1 << (extint - 32);
}
else if (extint < SAM_IRQ_NEXTINT)
{
*regaddr = NVIC_IRQ64_95_ENABLE;
*regaddr = (NVIC_IRQ64_95_ENABLE + offset);
*bit = 1 << (extint - 64);
}
else
@ -450,16 +458,33 @@ void up_irqinitialize(void)
void up_disable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (sam_irqinfo(irq, &regaddr, &bit) == 0)
if (sam_irqinfo(irq, &regaddr, &bit, NVIC_CLRENA_OFFSET) == 0)
{
/* Clear the appropriate bit in the register to enable the interrupt */
/* Modify the appropriate bit in the register to disable the interrupt */
regval = getreg32(regaddr);
regval &= ~bit;
/* This is awkward... For normal interrupts, we need to set the bit
* in the associated Interrupt Clear Enable register. For other
* exceptions, we need to clear the bit in the System Handler Control
* and State Register.
*/
if (irq >= SAM_IRQ_EXTINT)
{
regval |= bit;
}
else
{
regval &= ~bit;
}
/* Save the appropriately modified register */
putreg32(regval, regaddr);
}
#ifdef CONFIG_GPIO_IRQ
@ -483,11 +508,11 @@ void up_disable_irq(int irq)
void up_enable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (sam_irqinfo(irq, &regaddr, &bit) == 0)
if (sam_irqinfo(irq, &regaddr, &bit, NVIC_ENA_OFFSET) == 0)
{
/* Set the appropriate bit in the register to enable the interrupt */

View File

@ -66,6 +66,13 @@
NVIC_SYSH_PRIORITY_DEFAULT << 8 |\
NVIC_SYSH_PRIORITY_DEFAULT)
/* Given the address of a NVIC ENABLE register, this is the offset to
* the corresponding CLEAR ENABLE register.
*/
#define NVIC_ENA_OFFSET (0)
#define NVIC_CLRENA_OFFSET (NVIC_IRQ0_31_CLEAR - NVIC_IRQ0_31_ENABLE)
/****************************************************************************
* Public Data
****************************************************************************/
@ -221,7 +228,8 @@ static inline void stm32_prioritize_syscall(int priority)
*
****************************************************************************/
static int stm32_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
static int stm32_irqinfo(int irq, uintptr_t *regaddr, uint32_t *bit,
uintptr_t offset)
{
DEBUGASSERT(irq >= STM32_IRQ_NMI && irq < NR_IRQS);
@ -231,17 +239,17 @@ static int stm32_irqinfo(int irq, uint32_t *regaddr, uint32_t *bit)
{
if (irq < STM32_IRQ_INTERRUPTS + 32)
{
*regaddr = NVIC_IRQ0_31_ENABLE;
*regaddr = (NVIC_IRQ0_31_ENABLE + offset);
*bit = 1 << (irq - STM32_IRQ_INTERRUPTS);
}
else if (irq < STM32_IRQ_INTERRUPTS + 64)
{
*regaddr = NVIC_IRQ32_63_ENABLE;
*regaddr = (NVIC_IRQ32_63_ENABLE + offset);
*bit = 1 << (irq - STM32_IRQ_INTERRUPTS - 32);
}
else if (irq < NR_IRQS)
{
*regaddr = NVIC_IRQ64_95_ENABLE;
*regaddr = (NVIC_IRQ64_95_ENABLE + offset);
*bit = 1 << (irq - STM32_IRQ_INTERRUPTS - 64);
}
else
@ -415,16 +423,33 @@ void up_irqinitialize(void)
void up_disable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (stm32_irqinfo(irq, &regaddr, &bit) == 0)
if (stm32_irqinfo(irq, &regaddr, &bit, NVIC_CLRENA_OFFSET) == 0)
{
/* Clear the appropriate bit in the register to enable the interrupt */
/* Modify the appropriate bit in the register to disable the interrupt */
regval = getreg32(regaddr);
regval &= ~bit;
/* This is awkward... For normal interrupts, we need to set the bit
* in the associated Interrupt Clear Enable register. For other
* exceptions, we need to clear the bit in the System Handler Control
* and State Register.
*/
if (irq >= STM32_IRQ_INTERRUPTS)
{
regval |= bit;
}
else
{
regval &= ~bit;
}
/* Save the appropriately modified register */
putreg32(regval, regaddr);
}
@ -441,11 +466,11 @@ void up_disable_irq(int irq)
void up_enable_irq(int irq)
{
uint32_t regaddr;
uintptr_t regaddr;
uint32_t regval;
uint32_t bit;
if (stm32_irqinfo(irq, &regaddr, &bit) == 0)
if (stm32_irqinfo(irq, &regaddr, &bit, NVIC_ENA_OFFSET) == 0)
{
/* Set the appropriate bit in the register to enable the interrupt */