Skip to content

Nuvoton: Adhere to reworked ticker spec to release with Mbed OS 5.9 #7029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 2, 2018
Merged

Nuvoton: Adhere to reworked ticker spec to release with Mbed OS 5.9 #7029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ebd93ba
[Nuvoton] Meet new us_ticker HAL spec (Mbed OS 5.9)
ccli8 May 23, 2018
3f86142
[Nuvoton] Meet new lp_ticker HAL spec (Mbed OS 5.9)
ccli8 May 23, 2018
12792fd
[NANO130] Fix CLK_Idle incorrectly enters into deep sleep mode
ccli8 May 25, 2018
7caec46
[NANO130] Adjust static/dynamic memory allocation for IAR toolchain t…
ccli8 May 25, 2018
b18f690
Fix Greentea test code
ccli8 May 24, 2018
86e194d
[Nuvoton] Reduce blocking code in lp_ticker
ccli8 Jun 1, 2018
fe627cb
[Nuvoton] Synchronize lp_ticker code to us_ticker
ccli8 Jun 1, 2018
8e11ddf
[Nuvoton] Fix trap in lp_ticker ISR with non-blocking "clear interrup…
ccli8 Jun 11, 2018
310a1fe
[Nuvoton] Synchronize lp_ticker code to us_ticker
ccli8 Jun 11, 2018
1fa3374
[Nuvoton] Remove special handling for dummy interrupt in lp_ticker
ccli8 Jun 28, 2018
4f04ae4
[Nuvoton] Synchronize lp_ticker code to us_ticker
ccli8 Jun 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions TESTS/mbed_hal/common_tickers/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ unsigned int ticker_overflow_delta;

/* Auxiliary function to count ticker ticks elapsed during execution of N cycles of empty while loop.
* Parameter <step> is used to disable compiler optimisation. */
MBED_NOINLINE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this inserted?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind. Commit b18f690 explained it perfectly.

uint32_t count_ticks(uint32_t cycles, uint32_t step)
{
register uint32_t reg_cycles = cycles;
Expand Down
88 changes: 74 additions & 14 deletions targets/TARGET_NUVOTON/TARGET_M451/lp_ticker.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,42 @@ static const struct nu_modinit_s timer1_modinit = {TIMER_1, TMR1_MODULE, CLK_CLK

#define TIMER_MODINIT timer1_modinit

static int ticker_inited = 0;
/* Timer interrupt enable/disable
*
* Because Timer interrupt enable/disable (TIMER_EnableInt/TIMER_DisableInt) needs wait for lp_ticker,
* we call NVIC_DisableIRQ/NVIC_EnableIRQ instead.
*/

/* Track ticker status */
static volatile uint16_t ticker_inited = 0;

#define TMR_CMP_MIN 2
#define TMR_CMP_MAX 0xFFFFFFu

/* NOTE: When system clock is higher than timer clock, we need to add 3 engine clock
* (recommended by designer) delay to wait for above timer control to take effect. */
/* Synchronization issue with LXT/LIRC-clocked Timer
*
* PCLK : typical HCLK/2
* ECLK (engine clock) : LXT/LIRC for Timer used to implement lp_ticker
*
* When system clock is higher than Timer clock (LXT/LIRC), we need to add delay for ECLK
* domain to take effect:
* 1. Write : typical 1PCLK + 2ECLK
* Read-check doesn't work because it just checks PCLK domain and doesn't check into
* ECLK domain.
* 2. Clear interrupt flag : typical 2PCLK
* It is very rare that we would meet dummy interrupt and get stuck in ISR until
* 'clear interrupt flag' takes effect. The issue is ignorable because the pending
* time is very short (at most 1 dummy interrupt). We won't take special handling for it.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this comment 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c1728p9 Re-thanks for your detailed review.


void lp_ticker_init(void)
{
if (ticker_inited) {
/* By HAL spec, ticker_init allows the ticker to keep counting and disables the
* ticker interrupt. */
lp_ticker_disable_interrupt();
lp_ticker_clear_interrupt();
NVIC_ClearPendingIRQ(TIMER_MODINIT.irq_n);
return;
}
ticker_inited = 1;
Expand Down Expand Up @@ -86,7 +111,7 @@ void lp_ticker_init(void)
// Set vector
NVIC_SetVector(TIMER_MODINIT.irq_n, (uint32_t) TIMER_MODINIT.var);

NVIC_EnableIRQ(TIMER_MODINIT.irq_n);
NVIC_DisableIRQ(TIMER_MODINIT.irq_n);

TIMER_EnableInt(timer_base);
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);
Expand All @@ -101,6 +126,33 @@ void lp_ticker_init(void)
while(! (timer_base->CTL & TIMER_CTL_ACTSTS_Msk));
}

void lp_ticker_free(void)
{
TIMER_T *timer_base = (TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname);

/* Stop counting */
TIMER_Stop(timer_base);
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);

/* Wait for timer to stop counting and unset active flag */
while((timer_base->CTL & TIMER_CTL_ACTSTS_Msk));

/* Disable wakeup */
TIMER_DisableWakeup(timer_base);
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this wait_us needed? Same question for the wait_us below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c1728p9 All these wait_us are necessary.


/* Disable interrupt */
TIMER_DisableInt(timer_base);
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);

NVIC_DisableIRQ(TIMER_MODINIT.irq_n);

/* Disable IP clock */
CLK_DisableModuleClock(TIMER_MODINIT.clkidx);

ticker_inited = 0;
}

timestamp_t lp_ticker_read()
{
if (! ticker_inited) {
Expand Down Expand Up @@ -129,27 +181,39 @@ void lp_ticker_set_interrupt(timestamp_t timestamp)
uint32_t cmp_timer = timestamp * NU_TMRCLK_PER_TICK;
cmp_timer = NU_CLAMP(cmp_timer, TMR_CMP_MIN, TMR_CMP_MAX);

/* NOTE: Rely on LPTICKER_DELAY_TICKS to be non-blocking. */
timer_base->CMP = cmp_timer;
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);

/* We can call ticker_irq_handler now. */
NVIC_EnableIRQ(TIMER_MODINIT.irq_n);
}

void lp_ticker_disable_interrupt(void)
{
TIMER_DisableInt((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);
/* We cannot call ticker_irq_handler now. */
NVIC_DisableIRQ(TIMER_MODINIT.irq_n);
}

void lp_ticker_clear_interrupt(void)
{
TIMER_ClearIntFlag((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);
/* To avoid sync issue, we clear TIF/TWKF simultaneously rather than call separate
* driver API:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is on the PCLK domain does writing these separately cause issues? If not you could revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c1728p9 TIMER_ClearIntFlag/TIMER_ClearWakeupFlag are so close. It is safer to clear these interrupt flags altogether.

*
* TIMER_ClearIntFlag((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
* TIMER_ClearWakeupFlag((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
*/
TIMER_T *timer_base = (TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname);
timer_base->INTSTS = TIMER_INTSTS_TIF_Msk | TIMER_INTSTS_TWKF_Msk;
}

void lp_ticker_fire_interrupt(void)
{
// NOTE: This event was in the past. Set the interrupt as pending, but don't process it here.
// This prevents a recursive loop under heavy load which can lead to a stack overflow.
NVIC_SetPendingIRQ(TIMER_MODINIT.irq_n);

/* We can call ticker_irq_handler now. */
NVIC_EnableIRQ(TIMER_MODINIT.irq_n);
}

const ticker_info_t* lp_ticker_get_info()
Expand All @@ -163,11 +227,7 @@ const ticker_info_t* lp_ticker_get_info()

static void tmr1_vec(void)
{
TIMER_ClearIntFlag((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);

TIMER_ClearWakeupFlag((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
wait_us((NU_US_PER_SEC / NU_TMRCLK_PER_SEC) * 3);
lp_ticker_clear_interrupt();

// NOTE: lp_ticker_set_interrupt() may get called in lp_ticker_irq_handler();
lp_ticker_irq_handler();
Expand Down
48 changes: 43 additions & 5 deletions targets/TARGET_NUVOTON/TARGET_M451/us_ticker.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/

#include "us_ticker_api.h"

#if DEVICE_USTICKER

#include "sleep_api.h"
#include "mbed_assert.h"
#include "nu_modutil.h"
Expand All @@ -37,14 +40,20 @@ static const struct nu_modinit_s timer0_modinit = {TIMER_0, TMR0_MODULE, CLK_CLK

#define TIMER_MODINIT timer0_modinit

static int ticker_inited = 0;
/* Track ticker status */
static volatile uint16_t ticker_inited = 0;

#define TMR_CMP_MIN 2
#define TMR_CMP_MAX 0xFFFFFFu

void us_ticker_init(void)
{
if (ticker_inited) {
/* By HAL spec, ticker_init allows the ticker to keep counting and disables the
* ticker interrupt. */
us_ticker_disable_interrupt();
us_ticker_clear_interrupt();
NVIC_ClearPendingIRQ(TIMER_MODINIT.irq_n);
return;
}
ticker_inited = 1;
Expand Down Expand Up @@ -73,7 +82,7 @@ void us_ticker_init(void)

NVIC_SetVector(TIMER_MODINIT.irq_n, (uint32_t) TIMER_MODINIT.var);

NVIC_EnableIRQ(TIMER_MODINIT.irq_n);
NVIC_DisableIRQ(TIMER_MODINIT.irq_n);

TIMER_EnableInt(timer_base);

Expand All @@ -82,6 +91,26 @@ void us_ticker_init(void)
while(! (timer_base->CTL & TIMER_CTL_ACTSTS_Msk));
}

void us_ticker_free(void)
{
TIMER_T *timer_base = (TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname);

/* Stop counting */
TIMER_Stop(timer_base);

/* Wait for timer to stop counting and unset active flag */
while((timer_base->CTL & TIMER_CTL_ACTSTS_Msk));

/* Disable interrupt */
TIMER_DisableInt(timer_base);
NVIC_DisableIRQ(TIMER_MODINIT.irq_n);

/* Disable IP clock */
CLK_DisableModuleClock(TIMER_MODINIT.clkidx);

ticker_inited = 0;
}

uint32_t us_ticker_read()
{
if (! ticker_inited) {
Expand Down Expand Up @@ -110,11 +139,15 @@ void us_ticker_set_interrupt(timestamp_t timestamp)
uint32_t cmp_timer = timestamp * NU_TMRCLK_PER_TICK;
cmp_timer = NU_CLAMP(cmp_timer, TMR_CMP_MIN, TMR_CMP_MAX);
timer_base->CMP = cmp_timer;

/* We can call ticker_irq_handler now. */
NVIC_EnableIRQ(TIMER_MODINIT.irq_n);
}

void us_ticker_disable_interrupt(void)
{
TIMER_DisableInt((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
/* We cannot call ticker_irq_handler now. */
NVIC_DisableIRQ(TIMER_MODINIT.irq_n);
}

void us_ticker_clear_interrupt(void)
Expand All @@ -127,6 +160,9 @@ void us_ticker_fire_interrupt(void)
// NOTE: This event was in the past. Set the interrupt as pending, but don't process it here.
// This prevents a recursive loop under heavy load which can lead to a stack overflow.
NVIC_SetPendingIRQ(TIMER_MODINIT.irq_n);

/* We can call ticker_irq_handler now. */
NVIC_EnableIRQ(TIMER_MODINIT.irq_n);
}

const ticker_info_t* us_ticker_get_info()
Expand All @@ -140,8 +176,10 @@ const ticker_info_t* us_ticker_get_info()

static void tmr0_vec(void)
{
TIMER_ClearIntFlag((TIMER_T *) NU_MODBASE(TIMER_MODINIT.modname));
us_ticker_clear_interrupt();

// NOTE: us_ticker_set_interrupt() may get called in us_ticker_irq_handler();
us_ticker_irq_handler();
}

#endif
Loading