From 8f76233029a549f3c1b660a9975fd524b53a5763 Mon Sep 17 00:00:00 2001 From: Wayne Ren Date: Mon, 6 Apr 2020 11:56:08 +0800 Subject: [PATCH] arch: arc: optimize the arc v2 interrupt unit driver * add interrupt lock in low level API to gurantee the correctness of operations. * make some functions as in-line functions * clean up and optimize the code comments Signed-off-by: Wayne Ren --- arch/arc/core/irq_manage.c | 13 +-- .../intc_arcv2_irq_unit.c | 60 +++++----- include/arch/arc/v2/arcv2_irq_unit.h | 106 +++++++++++++----- 3 files changed, 112 insertions(+), 67 deletions(-) diff --git a/arch/arc/core/irq_manage.c b/arch/arc/core/irq_manage.c index f69ce80bfc2..c1db189b825 100644 --- a/arch/arc/core/irq_manage.c +++ b/arch/arc/core/irq_manage.c @@ -57,7 +57,7 @@ void z_arc_firq_stack_set(void) /* the z_arc_firq_stack_set must be called when irq diasbled, as * it can be called not only in the init phase but also other places */ - unsigned int key = irq_lock(); + unsigned int key = arch_irq_lock(); __asm__ volatile ( /* only ilink will not be banked, so use ilink as channel @@ -79,7 +79,7 @@ void z_arc_firq_stack_set(void) "i"(_ARC_V2_STATUS32_RB(1)), "i"(~_ARC_V2_STATUS32_RB(7)) ); - irq_unlock(key); + arch_irq_unlock(key); } #endif @@ -95,10 +95,7 @@ void z_arc_firq_stack_set(void) void arch_irq_enable(unsigned int irq) { - unsigned int key = irq_lock(); - z_arc_v2_irq_unit_int_enable(irq); - irq_unlock(key); } /* @@ -112,10 +109,7 @@ void arch_irq_enable(unsigned int irq) void arch_irq_disable(unsigned int irq) { - unsigned int key = irq_lock(); - z_arc_v2_irq_unit_int_disable(irq); - irq_unlock(key); } /** @@ -147,8 +141,6 @@ void z_irq_priority_set(unsigned int irq, unsigned int prio, u32_t flags) { ARG_UNUSED(flags); - unsigned int key = irq_lock(); - __ASSERT(prio < CONFIG_NUM_IRQ_PRIO_LEVELS, "invalid priority %d for irq %d", prio, irq); /* 0 -> CONFIG_NUM_IRQ_PRIO_LEVELS allocted to secure world @@ -162,7 +154,6 @@ void z_irq_priority_set(unsigned int irq, unsigned int prio, u32_t flags) ARC_N_IRQ_START_LEVEL : prio; #endif z_arc_v2_irq_unit_prio_set(irq, prio); - irq_unlock(key); } /* diff --git a/drivers/interrupt_controller/intc_arcv2_irq_unit.c b/drivers/interrupt_controller/intc_arcv2_irq_unit.c index 30189f6ecfd..1086c4dda76 100644 --- a/drivers/interrupt_controller/intc_arcv2_irq_unit.c +++ b/drivers/interrupt_controller/intc_arcv2_irq_unit.c @@ -1,5 +1,6 @@ /* * Copyright (c) 2014 Wind River Systems, Inc. + * Copyright (c) 2020 Synopsys. * * SPDX-License-Identifier: Apache-2.0 */ @@ -58,9 +59,8 @@ static struct arc_v2_irq_unit_ctx ctx; * the window between a write to IRQ_SELECT and subsequent writes to the * selected IRQ's registers. * - * @return N/A + * @return 0 for success */ - static int arc_v2_irq_unit_init(struct device *unused) { ARG_UNUSED(unused); @@ -88,26 +88,16 @@ static int arc_v2_irq_unit_init(struct device *unused) return 0; } -void z_arc_v2_irq_unit_int_eoi(int irq) -{ - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PULSE_CANCEL, 1); -} - -void z_arc_v2_irq_unit_trigger_set(int irq, unsigned int trigger) -{ - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_TRIGGER, trigger); -} - -unsigned int z_arc_v2_irq_unit_trigger_get(int irq) -{ - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); - return z_arc_v2_aux_reg_read(_ARC_V2_IRQ_TRIGGER); -} - #ifdef CONFIG_DEVICE_POWER_MANAGEMENT +/* + * @brief Suspend the interrupt unit device driver + * + * Suspends the interrupt unit device driver and the device + * itself. + * + * @return 0 for success + */ static int arc_v2_irq_unit_suspend(struct device *dev) { u8_t irq; @@ -136,10 +126,17 @@ static int arc_v2_irq_unit_suspend(struct device *dev) return 0; } +/* + * @brief Resume the interrupt unit device driver + * + * Resume the interrupt unit device driver and the device + * itself. + * + * @return 0 for success + */ static int arc_v2_irq_unit_resume(struct device *dev) { u8_t irq; - u32_t status32; ARG_UNUSED(dev); @@ -170,16 +167,16 @@ static int arc_v2_irq_unit_resume(struct device *dev) #endif z_arc_v2_aux_reg_write(_ARC_V2_IRQ_VECT_BASE, ctx.irq_vect_base); - status32 = z_arc_v2_aux_reg_read(_ARC_V2_STATUS32); - status32 |= Z_ARC_V2_STATUS32_E(_ARC_V2_DEF_IRQ_LEVEL); - - __builtin_arc_kflag(status32); - _arc_v2_irq_unit_device_power_state = DEVICE_PM_ACTIVE_STATE; return 0; } +/* + * @brief Get the power state of interrupt unit + * + * @return the power state of interrupt unit + */ static int arc_v2_irq_unit_get_state(struct device *dev) { ARG_UNUSED(dev); @@ -188,13 +185,18 @@ static int arc_v2_irq_unit_get_state(struct device *dev) } /* - * Implements the driver control management functionality - * the *context may include IN data or/and OUT data + * @brief Implement the driver control of interrupt unit + * + * The operation on interrupt unit requires interrupt lock. + * The *context may include IN data or/and OUT data + * + * @return operation result */ static int arc_v2_irq_unit_device_ctrl(struct device *device, u32_t ctrl_command, void *context, device_pm_cb cb, void *arg) { int ret = 0; + unsigned int key = arch_irq_lock(); if (ctrl_command == DEVICE_PM_SET_POWER_STATE) { if (*((u32_t *)context) == DEVICE_PM_SUSPEND_STATE) { @@ -206,6 +208,8 @@ static int arc_v2_irq_unit_device_ctrl(struct device *device, *((u32_t *)context) = arc_v2_irq_unit_get_state(device); } + arch_irq_unlock(key); + if (cb) { cb(device, ret, context, arg); } diff --git a/include/arch/arc/v2/arcv2_irq_unit.h b/include/arch/arc/v2/arcv2_irq_unit.h index 893436791f8..2fa57af608c 100644 --- a/include/arch/arc/v2/arcv2_irq_unit.h +++ b/include/arch/arc/v2/arcv2_irq_unit.h @@ -2,6 +2,7 @@ /* * Copyright (c) 2014 Wind River Systems, Inc. + * Copyright (c) 2020 Synopsys. * * SPDX-License-Identifier: Apache-2.0 */ @@ -24,20 +25,16 @@ extern "C" { #ifndef _ASMLANGUAGE /* - * WARNING: + * NOTE: * - * All APIs provided by this file must be invoked with INTERRUPTS LOCKED. The + * All APIs provided by this file are protected with INTERRUPTS LOCKED. The * APIs themselves are writing the IRQ_SELECT, selecting which IRQ's registers * it wants to write to, then write to them: THIS IS NOT AN ATOMIC OPERATION. * - * Not locking the interrupts inside of the APIs allows a caller to: + * Locking the interrupts inside of the APIs are some kind of self-protection + * to guarantee the correctness of operation if the callers don't lock + * the interrupt. * - * - lock interrupts - * - call many of these APIs - * - unlock interrupts - * - * thus being more efficient then if the APIs themselves would lock - * interrupts. */ /* @@ -54,8 +51,12 @@ void z_arc_v2_irq_unit_irq_enable_set( unsigned char enable ) { + unsigned int key = arch_irq_lock(); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); z_arc_v2_aux_reg_write(_ARC_V2_IRQ_ENABLE, enable); + + arch_irq_unlock(key); } /* @@ -97,8 +98,15 @@ void z_arc_v2_irq_unit_int_disable(int irq) static ALWAYS_INLINE bool z_arc_v2_irq_unit_int_enabled(int irq) { + bool ret; + unsigned int key = arch_irq_lock(); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); - return z_arc_v2_aux_reg_read(_ARC_V2_IRQ_ENABLE) & 0x1; + ret = z_arc_v2_aux_reg_read(_ARC_V2_IRQ_ENABLE) & 0x1; + + arch_irq_unlock(key); + + return ret; } @@ -114,6 +122,8 @@ static ALWAYS_INLINE void z_arc_v2_irq_unit_prio_set(int irq, unsigned char prio) { + unsigned int key = arch_irq_lock(); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); #if defined(CONFIG_ARC_SECURE_FIRMWARE) z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PRIORITY, @@ -122,23 +132,35 @@ void z_arc_v2_irq_unit_prio_set(int irq, unsigned char prio) #else z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PRIORITY, prio); #endif + arch_irq_unlock(key); } #if defined(CONFIG_ARC_SECURE_FIRMWARE) +/* + * @brief Configure the secure state of interrupt + * + * Configure the secure state of the specified interrupt + * + * @return N/A + */ static ALWAYS_INLINE void z_arc_v2_irq_uinit_secure_set(int irq, bool secure) { - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); + unsigned int key = arch_irq_lock(); - if (secure) { - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PRIORITY, - z_arc_v2_aux_reg_read(_ARC_V2_IRQ_PRIORITY) | - _ARC_V2_IRQ_PRIORITY_SECURE); - } else { - z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PRIORITY, - z_arc_v2_aux_reg_read(_ARC_V2_IRQ_PRIORITY) & - _ARC_V2_INT_PRIO_MASK); - } + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); + + if (secure) { + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PRIORITY, + z_arc_v2_aux_reg_read(_ARC_V2_IRQ_PRIORITY) | + _ARC_V2_IRQ_PRIORITY_SECURE); + } else { + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PRIORITY, + z_arc_v2_aux_reg_read(_ARC_V2_IRQ_PRIORITY) & + _ARC_V2_INT_PRIO_MASK); + } + + arch_irq_unlock(key); } #endif @@ -156,8 +178,12 @@ void z_arc_v2_irq_uinit_secure_set(int irq, bool secure) static ALWAYS_INLINE void z_arc_v2_irq_unit_sensitivity_set(int irq, int s) { + unsigned int key = arch_irq_lock(); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); z_arc_v2_aux_reg_write(_ARC_V2_IRQ_TRIGGER, s); + + arch_irq_unlock(key); } /* @@ -165,7 +191,7 @@ void z_arc_v2_irq_unit_sensitivity_set(int irq, int s) * * Check whether processor in interrupt/exception state * - * @return N/A + * @return 1 in interrupt/exception state; 0 not in */ static ALWAYS_INLINE bool z_arc_v2_irq_unit_is_in_isr(void) @@ -189,8 +215,16 @@ bool z_arc_v2_irq_unit_is_in_isr(void) * * @return N/A */ +static ALWAYS_INLINE +void z_arc_v2_irq_unit_trigger_set(int irq, unsigned int trigger) +{ + unsigned int key = arch_irq_lock(); -void z_arc_v2_irq_unit_trigger_set(int irq, unsigned int trigger); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_TRIGGER, trigger); + + arch_irq_unlock(key); +} /* * @brief Returns an IRQ line trigger type @@ -198,10 +232,21 @@ void z_arc_v2_irq_unit_trigger_set(int irq, unsigned int trigger); * Gets the IRQ line trigger type. * Valid values for are _ARC_V2_INT_LEVEL and _ARC_V2_INT_PULSE. * - * @return N/A + * @return trigger state */ +static ALWAYS_INLINE +unsigned int z_arc_v2_irq_unit_trigger_get(int irq) +{ + unsigned int ret; + unsigned int key = arch_irq_lock(); -unsigned int z_arc_v2_irq_unit_trigger_get(int irq); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); + ret = z_arc_v2_aux_reg_read(_ARC_V2_IRQ_TRIGGER); + + arch_irq_unlock(key); + + return ret; +} /* * @brief Send EOI signal to interrupt unit @@ -209,13 +254,18 @@ unsigned int z_arc_v2_irq_unit_trigger_get(int irq); * This routine sends an EOI (End Of Interrupt) signal to the interrupt unit * to clear a pulse-triggered interrupt. * - * Interrupts must be locked or the ISR operating at P0 when invoking this - * function. - * * @return N/A */ +static ALWAYS_INLINE +void z_arc_v2_irq_unit_int_eoi(int irq) +{ + unsigned int key = arch_irq_lock(); -void z_arc_v2_irq_unit_int_eoi(int irq); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq); + z_arc_v2_aux_reg_write(_ARC_V2_IRQ_PULSE_CANCEL, 1); + + arch_irq_unlock(key); +} #endif /* _ASMLANGUAGE */