Merge pull request #12424 from OpenNuvoton/nuvoton_gpio-irq_rtc-lxt

Nuvoton: Fix GPIO IRQ and RTC

Author: 0xc0170

targets/TARGET_NUVOTON/TARGET_M2351/PeripheralPins.c

@@ -138,7 +138,7 @@ const PinMap PinMap_GPIO[] = {
     {PG_14, GPIO_G, SYS_GPG_MFPH_PG14MFP_GPIO},
     {PG_15, GPIO_G, SYS_GPG_MFPH_PG15MFP_GPIO},
 
-     // GPIO H MFP
+    // GPIO H MFP
     {PH_4, GPIO_H, SYS_GPH_MFPL_PH4MFP_GPIO},
     {PH_5, GPIO_H, SYS_GPH_MFPL_PH5MFP_GPIO},
     {PH_6, GPIO_H, SYS_GPH_MFPL_PH6MFP_GPIO},
@@ -646,4 +646,3 @@ const PinMap PinMap_CAN_RD[] = {
 
     {NC,    NC,    0}
 };
-

targets/TARGET_NUVOTON/TARGET_M2351/TARGET_M23_NS/TARGET_NU_PFM_M2351_NPSA_NS/TARGET_NU_PREBUILD_SECURE/NuMaker-mbed-TZ-secure-example.hex

@@ -52,7 +52,7 @@ __STATIC_INLINE CRPT_T *CRYPTO_MODBASE(void)
 #if defined(SCU_INIT_PNSSET1_VAL) && (SCU_INIT_PNSSET1_VAL & (1<<18))
     return CRPT_NS;
 #else
-    return CRPT;
+    return CRPT_S;
 #endif
 }
 

targets/TARGET_NUVOTON/TARGET_M2351/TARGET_M23_NS/TARGET_NU_PFM_M2351_NS/TARGET_NU_PREBUILD_SECURE/crypto_access_control.hex

@@ -182,7 +182,13 @@ void gpio_irq_set(gpio_irq_t *obj, gpio_irq_event event, uint32_t enable)
         obj->irq_types &= ~irq_type;
     }
 
-    /* Update irq types */
+    /* Update irq types:
+     *
+     * Implementations of GPIO_EnableInt(...) are inconsistent: disable or not irq type not enabled.
+     * For consistency, disable GPIO_INT_BOTH_EDGE and then enable OR'ed irq types, GPIO_INT_RISING,
+     * GPIO_INT_FALLING, or both.
+     */
+    GPIO_DisableInt(gpio_base, pin_index);
     GPIO_EnableInt(gpio_base, pin_index, obj->irq_types);
 }
 

targets/TARGET_NUVOTON/TARGET_M2351/TARGET_M23_NS/TARGET_NU_PFM_M2351_NS/TARGET_NU_PREBUILD_SECURE/spm_client.hex

@@ -29,10 +29,13 @@
 #include "tfm_ns_lock.h"
 #endif
 
-/* NOTE: BSP RTC driver judges secure/non-secure RTC by PC. This implementation cannot support non-secure RTC
- *       controlled by secure executable. A better way would be that secure/non-secure RTC base is passed
- *       to RTC API as an argument like most other APIs. With BSP RTC driver unchanged, we must enforce 
- *       secure RTC. */
+/* Secure attribution of RTC
+ *
+ * We need RTC to be secure for security concern.
+ *
+ * On M2351, configured to secure
+ * On M2351, hard-wired to secure
+ */
 #if defined(SCU_INIT_PNSSET2_VAL) && (SCU_INIT_PNSSET2_VAL & (1 << 1))
 #error("Limited by BSP/RTC, we can only support secure RTC.")
 #endif
@@ -70,7 +73,7 @@ void rtc_write(time_t t)
  *
  * NOTE: This dependents on real hardware.
  */
-#define NU_RTCCLK_PER_SEC           ((CLK->CLKSEL3 & CLK_CLKSEL3_SC0SEL_Msk) ? __LIRC : __LXT)
+#define NU_RTCCLK_PER_SEC           (__LXT)
 
 /* Strategy for implementation of RTC HAL
  *
@@ -124,7 +127,7 @@ static time_t t_write = 0;
 /* Convert date time from H/W RTC to struct TM */
 static void rtc_convert_datetime_hwrtc_to_tm(struct tm *datetime_tm, const S_RTC_TIME_DATA_T *datetime_hwrtc);
 
-static const struct nu_modinit_s rtc_modinit = {RTC_0, RTC_MODULE, 0, 0, 0, RTC_IRQn, NULL};
+static const struct nu_modinit_s rtc_modinit = {RTC_0, RTC_MODULE, CLK_CLKSEL3_RTCSEL_LXT, 0, 0, RTC_IRQn, NULL};
 
 static void rtc_init_impl(void);
 static void rtc_free_impl(void);
@@ -151,11 +154,10 @@ static void rtc_free_impl(void)
 
 static int32_t rtc_isenabled_impl(void)
 {
-    // NOTE: To access (RTC) registers, clock must be enabled first.
-    if (! (CLK->APBCLK0 & CLK_APBCLK0_RTCCKEN_Msk)) {
-        // Enable IP clock
-        CLK_EnableModuleClock_S(rtc_modinit.clkidx);
-    }
+    // To access (RTC) registers, clock must be enabled first.
+    // For TZ, with RTC being secure, we needn't call the secure gateway versions.
+    CLK_EnableModuleClock(rtc_modinit.clkidx);
+    CLK_SetModuleClock(rtc_modinit.clkidx, rtc_modinit.clksrc, rtc_modinit.clkdiv);
 
     RTC_T *rtc_base = (RTC_T *) NU_MODBASE(rtc_modinit.modname);
 

targets/TARGET_NUVOTON/TARGET_M2351/TARGET_M23_NS/TARGET_NU_PFM_M2351_NS/TARGET_NU_PREBUILD_SECURE/spm_server.hex

@@ -25,6 +25,7 @@
 #include "pinmap.h"
 #include "PeripheralPins.h"
 #include "nu_bitutil.h"
+#include "mbed_assert.h"
 
 #define NU_MAX_PIN_PER_PORT     16
 
@@ -87,6 +88,7 @@ int gpio_irq_init(gpio_irq_t *obj, PinName pin, gpio_irq_handler handler, uint32
     }
 
     obj->pin = pin;
+    obj->irq_types = 0;
     obj->irq_handler = (uint32_t) handler;
     obj->irq_id = id;
 
@@ -154,27 +156,38 @@ void gpio_irq_set(gpio_irq_t *obj, gpio_irq_event event, uint32_t enable)
     uint32_t port_index = NU_PINPORT(obj->pin);
     GPIO_T *gpio_base = NU_PORT_BASE(port_index);
 
+    /* We assume BSP has such coding so that we can easily add/remove either irq type. */
+    MBED_STATIC_ASSERT(GPIO_INT_BOTH_EDGE == (GPIO_INT_RISING | GPIO_INT_FALLING),
+        "GPIO_INT_BOTH_EDGE must be bitwise OR of GPIO_INT_RISING and GPIO_INT_FALLING");
+    uint32_t irq_type;
     switch (event) {
-    case IRQ_RISE:
-        if (enable) {
-            GPIO_EnableInt(gpio_base, pin_index, GPIO_INT_RISING);
-        } else {
-            gpio_base->INTEN &= ~(GPIO_INT_RISING << pin_index);
-        }
-        break;
+        case IRQ_RISE:
+            irq_type = GPIO_INT_RISING;
+            break;
 
-    case IRQ_FALL:
-        if (enable) {
-            GPIO_EnableInt(gpio_base, pin_index, GPIO_INT_FALLING);
-        } else {
-            gpio_base->INTEN &= ~(GPIO_INT_FALLING << pin_index);
-        }
-        break;
+        case IRQ_FALL:
+            irq_type = GPIO_INT_FALLING;
+            break;
 
-    case IRQ_NONE:
-    default:
-        break;
+        default:
+            irq_type = 0;
     }
+
+    /* We can handle invalid/null irq type. */
+    if (enable) {
+        obj->irq_types |= irq_type;
+    } else {
+        obj->irq_types &= ~irq_type;
+    }
+
+    /* Update irq types:
+     *
+     * Implementations of GPIO_EnableInt(...) are inconsistent: disable or not irq type not enabled.
+     * For consistency, disable GPIO_INT_BOTH_EDGE and then enable OR'ed irq types, GPIO_INT_RISING,
+     * GPIO_INT_FALLING, or both.
+     */
+    GPIO_DisableInt(gpio_base, pin_index);
+    GPIO_EnableInt(gpio_base, pin_index, obj->irq_types);
 }
 
 void gpio_irq_enable(gpio_irq_t *obj)

targets/TARGET_NUVOTON/TARGET_M2351/TARGET_M23_NS/TARGET_NU_PFM_M2351_NS/TARGET_NU_PREBUILD_SECURE/spm_smoke.hex

@@ -31,6 +31,7 @@ extern "C" {
 
 struct gpio_irq_s {
     PinName     pin;
+    uint32_t    irq_types;
     uint32_t    irq_handler;
     uint32_t    irq_id;
     struct gpio_irq_s   *next;

targets/TARGET_NUVOTON/TARGET_M2351/TARGET_M23_NS/TARGET_NU_PFM_M2351_NS/TARGET_NU_PREBUILD_SECURE/tfm.hex

@@ -107,11 +107,10 @@ void rtc_free(void)
 
 int rtc_isenabled(void)
 {
-    // NOTE: To access (RTC) registers, clock must be enabled first.
-    if (! (CLK->APBCLK0 & CLK_APBCLK0_RTCCKEN_Msk)) {
-        // Enable IP clock
-        CLK_EnableModuleClock(rtc_modinit.clkidx);
-    }
+    // To access (RTC) registers, clock must be enabled first.
+    // For TZ, with RTC being secure, we needn't call the secure gateway versions.
+    CLK_EnableModuleClock(rtc_modinit.clkidx);
+    CLK_SetModuleClock(rtc_modinit.clkidx, rtc_modinit.clksrc, rtc_modinit.clkdiv);
 
     RTC_T *rtc_base = (RTC_T *) NU_MODBASE(rtc_modinit.modname);
 

targets/TARGET_NUVOTON/TARGET_M2351/crypto/crypto-misc.h

@@ -24,6 +24,7 @@
 #include "pinmap.h"
 #include "PeripheralPins.h"
 #include "nu_bitutil.h"
+#include "mbed_assert.h"
 
 #define NU_MAX_PIN_PER_PORT     16
 
@@ -88,6 +89,7 @@ int gpio_irq_init(gpio_irq_t *obj, PinName pin, gpio_irq_handler handler, uint32
     }
 
     obj->pin = pin;
+    obj->irq_types = 0;
     obj->irq_handler = (uint32_t) handler;
     obj->irq_id = id;
 
@@ -155,27 +157,38 @@ void gpio_irq_set(gpio_irq_t *obj, gpio_irq_event event, uint32_t enable)
     uint32_t port_index = NU_PINNAME_TO_PORT(obj->pin);
     GPIO_T *gpio_base = NU_PORT_BASE(port_index);
 
+    /* We assume BSP has such coding so that we can easily add/remove either irq type. */
+    MBED_STATIC_ASSERT(GPIO_INT_BOTH_EDGE == (GPIO_INT_RISING | GPIO_INT_FALLING),
+        "GPIO_INT_BOTH_EDGE must be bitwise OR of GPIO_INT_RISING and GPIO_INT_FALLING");
+    uint32_t irq_type;
     switch (event) {
-    case IRQ_RISE:
-        if (enable) {
-            GPIO_EnableInt(gpio_base, pin_index, GPIO_INT_RISING);
-        } else {
-            gpio_base->INTEN &= ~(GPIO_INT_RISING << pin_index);
-        }
-        break;
+        case IRQ_RISE:
+            irq_type = GPIO_INT_RISING;
+            break;
 
-    case IRQ_FALL:
-        if (enable) {
-            GPIO_EnableInt(gpio_base, pin_index, GPIO_INT_FALLING);
-        } else {
-            gpio_base->INTEN &= ~(GPIO_INT_FALLING << pin_index);
-        }
-        break;
+        case IRQ_FALL:
+            irq_type = GPIO_INT_FALLING;
+            break;
 
-    case IRQ_NONE:
-    default:
-        break;
+        default:
+            irq_type = 0;
     }
+
+    /* We can handle invalid/null irq type. */
+    if (enable) {
+        obj->irq_types |= irq_type;
+    } else {
+        obj->irq_types &= ~irq_type;
+    }
+
+    /* Update irq types:
+     *
+     * Implementations of GPIO_EnableInt(...) are inconsistent: disable or not irq type not enabled.
+     * For consistency, disable GPIO_INT_BOTH_EDGE and then enable OR'ed irq types, GPIO_INT_RISING,
+     * GPIO_INT_FALLING, or both.
+     */
+    GPIO_DisableInt(gpio_base, pin_index);
+    GPIO_EnableInt(gpio_base, pin_index, obj->irq_types);
 }
 
 void gpio_irq_enable(gpio_irq_t *obj)

targets/TARGET_NUVOTON/TARGET_M2351/gpio_irq_api.c

@@ -30,6 +30,7 @@ extern "C" {
 
 struct gpio_irq_s {
     PinName     pin;
+    uint32_t    irq_types;
     uint32_t    irq_handler;
     uint32_t    irq_id;
     struct gpio_irq_s   *next;

targets/TARGET_NUVOTON/TARGET_M2351/rtc_api.c

@@ -31,7 +31,7 @@
  *
  * NOTE: This dependents on real hardware.
  */
-#define NU_RTCCLK_PER_SEC           ((CLK->CLKSEL3 & CLK_CLKSEL3_SC0SEL_Msk) ? __LIRC : __LXT)
+#define NU_RTCCLK_PER_SEC           __LXT
 
 /* Strategy for implementation of RTC HAL
  *
@@ -85,7 +85,7 @@ static time_t t_write = 0;
 /* Convert date time from H/W RTC to struct TM */
 static void rtc_convert_datetime_hwrtc_to_tm(struct tm *datetime_tm, const S_RTC_TIME_DATA_T *datetime_hwrtc);
 
-static const struct nu_modinit_s rtc_modinit = {RTC_0, RTC_MODULE, 0, 0, 0, RTC_IRQn, NULL};
+static const struct nu_modinit_s rtc_modinit = {RTC_0, RTC_MODULE, CLK_CLKSEL3_RTCSEL_LXT, 0, 0, RTC_IRQn, NULL};
 
 void rtc_init(void)
 {
@@ -106,11 +106,10 @@ void rtc_free(void)
 
 int rtc_isenabled(void)
 {
-    // NOTE: To access (RTC) registers, clock must be enabled first.
-    if (! (CLK->APBCLK0 & CLK_APBCLK0_RTCCKEN_Msk)) {
-        // Enable IP clock
-        CLK_EnableModuleClock(rtc_modinit.clkidx);
-    }
+    // To access (RTC) registers, clock must be enabled first.
+    // For TZ, with RTC being secure, we needn't call the secure gateway versions.
+    CLK_EnableModuleClock(rtc_modinit.clkidx);
+    CLK_SetModuleClock(rtc_modinit.clkidx, rtc_modinit.clksrc, rtc_modinit.clkdiv);
 
     RTC_T *rtc_base = (RTC_T *) NU_MODBASE(rtc_modinit.modname);