Skip to content

added tud_task_event_ready() #414

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.

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 5 commits into from
May 20, 2020
Merged

added tud_task_event_ready() #414

merged 5 commits into from
May 20, 2020

Conversation

hathach
Copy link
Owner

@hathach hathach commented May 20, 2020

Describe the PR
added tud_task_event_ready() to check if there is pending events in the tud task without executing
it. Useful to check before entering low power mode with WFI/WFE. This helps to resovle adafruit/circuitpython#2868

This requires an additional osal_queue_empty() to be implemented.

Additional context

TockOS, like a lot of other operating systems divides interrupt processing in two halves. The top-half runs in interrupt context, which is mapped to ARMv7-M's handler mode. The bottom-half processing happens in kernel context, which is mapped to ARMv7-M's privileged thread mode.

The main USB interrupt processing happens in the bottom half, and within the bottom half we add to TinyUSB task queue. Therefore we can call wfi, only when both TinyUSB task queue is empty and no more bottom half interrupt processing is left to do.

Hence the need for tud_task_is_queue_empty() function.

Originally posted by @rajivr in #279 (comment)

hathach added 3 commits May 20, 2020 13:38
ported for osal none/freertos/mynewt
to check if there is pending events in the tud task without executing
it. Useful to check before entering low power mode with WFI/WFE
since it doesn't make any differences.
@hathach hathach requested a review from xobs May 20, 2020 07:40
@hathach hathach requested a review from tannewt May 20, 2020 07:41
@@ -203,6 +202,15 @@ static inline bool osal_queue_send(osal_queue_t const qhdl, void const * data, b
return success;
}

static inline bool osal_queue_empty(osal_queue_t qhdl)
{
_osal_q_lock(qhdl);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This approach won't work.

The user needs to call tud_task_event_ready() with interrupts already disabled in order to check whether the queue is empty. If the queue is empty, then it can issue WFI. If not, it will re-enable interrupts and not issue WFI.

I'm uncertain what happens if you issue cpsid i when interrupts are already disabled, but this function will re-enable them when it exits. If a USB event comes in between the time this function exits and when the calling function issues WFI, then that interrupt will be handled and the queue will no longer be empty, but the calling function will still issue WFI.

This function needs to be called with interrupts already disabled, and shouldn't lock/unlock the system on its own.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@xobs Yeah, you are right, I was thinking to have this function being used in normal/general mode as well. However, there is not much benefit in normal mode, also locking the queue is not crucial as well in that mode.

NVIC enable probably shouldn't have any impact on the global CPSID, but I could be wrong. I will remove the queue lock/unlock anyway since it is better to do.

@hathach hathach merged commit dc5445e into master May 20, 2020
@hathach hathach deleted the add-queue-is-empty branch May 20, 2020 08:31
@hathach
Copy link
Owner Author

hathach commented May 20, 2020

thanks @xobs for reviewing, the pr is merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants