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 <wei.ren@synopsys.com>

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);
 }
 
 /*

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
+ * @brief  Implement the driver control of interrupt unit
- * the *context may include IN data or/and OUT data
+ *
+ * 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);
 	}

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,12 +132,22 @@ 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)
 {
+	unsigned int key = arch_irq_lock();
+
 	z_arc_v2_aux_reg_write(_ARC_V2_IRQ_SELECT, irq);
 
 	if (secure) {
@@ -139,6 +159,8 @@ void z_arc_v2_irq_uinit_secure_set(int irq, bool secure)
 		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 <irq> trigger type.
  * Valid values for <trigger> 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 */