[riot-devel] Current best practice for sleeping in a device driver?

Joakim NohlgÄrd joakim.nohlgard at eistec.se
Tue Sep 11 19:48:57 CEST 2018


Hi Pekka,
What we have done in several other places is to use a mutex. The device
driver blocking code will attempt to lock the mutex twice, and the ISR will
unlock it to let the thread continue.
Does that work in your use case?

Best regards, Joakim

Den tis 11 sep. 2018 16:40Nikander Pekka <pekka.nikander at aalto.fi> skrev:

> Hi,
>
> Sorry for my ignorance, but what is the current best practice to sleep in
> a blocking device driver, waiting for an interrupt to take place?  I
> couldn't find anything reasonable.
>
> In many other context I've seen people to use `sleep` and `wakeup`, and
> even went that far that I tried to introduce a variant of `thread_sleep`
> that can be entered with interrupts disabled (see below), but couldn't get
> it working.
>
> Here is an excerpt of my current code:
> ```
> void isr_saadc(void) {
>     NRF_SAADC->EVENTS_END = 0;
>     NRF_SAADC->INTENCLR = SAADC_INTEN_DONE_Msk;
>     thread_wakeup(sleeper);
> }
>
> int adc_sample(adc_t line, adc_res_t res)
> {
>     ...
>     /* enable EVENTS_END interrupt */
>     const uint32_t irqs = irq_disable();
>     sleeper = thread_getpid();
>     NRF_SAADC->INTENSET = SAADC_INTEN_END_Msk; /* Disabled at ISR handler
> */
>
>     ...
>
>     /* Wait for the interrupt */
>     thread_sleep_with_interrupts_disabled(irqs);
> ```
>
> This doesn't work.  The thread doesn't wake up, even though the interrupt
> does take place and `thread_wakeup` states that the thread _is_ sleeping
> when called:
> ```
> Created thread idle. PID: 1. Priority: 15.
> Created thread main. PID: 2. Priority: 7.
> main(): This is RIOT! (Version:
> 2018.10-devel-585-gd2de4-dhcp-85-86.eduroam.aalto.fi-feature-refactor-ble-core)
> Entering main function.
> Entering main loop.
> thread_wakeup: Trying to wakeup PID 2...
> thread_wakeup: Thread is sleeping.
> thread_wakeup: Trying to wakeup PID 2...
> thread_wakeup: Thread is sleeping.
> ```
>
> And then nothing happens.
>
> Here is the tentative patch to introduce
> `thread_sleep_with_interrupts_disabled`:
> ```
> diff --git a/core/include/thread.h b/core/include/thread.h
> index 64a828ab3..4abdb3d25 100644
> --- a/core/include/thread.h
> +++ b/core/include/thread.h
> @@ -399,6 +399,20 @@ int thread_getstatus(kernel_pid_t pid);
>   */
>  void thread_sleep(void);
>
> +/**
> + * @brief Puts the current thread into sleep mode with the interrupts
> already disabled.
> + *
> + * @param[in] interrupt_state  Saved interrupt state from irq_disable().
> + *
> + *          Places the calling thread to sleep and re-enables interrupts
> to
> + *          the `interrupt_state` before calling thread_yield_higher().
> + *          Meant to be called from drivers that want to be waken up
> + *          from an interrupt routine.
> + *          The calling thread to be woken up externally.
> + *
> + */
> +void thread_sleep_with_interrupts_disabled(int interrupt_state);
> +
>  /**
>   * @brief   Lets current thread yield.
>   *
> diff --git a/core/thread.c b/core/thread.c
> index 6f7a71362..05400a0af 100644
> --- a/core/thread.c
> +++ b/core/thread.c
> @@ -62,9 +62,12 @@ void thread_sleep(void)
>      }
>
>      unsigned state = irq_disable();
> +    thread_sleep_with_interrupts_disabled(state);
> +}
> +
> +void thread_sleep_with_interrupts_disabled(int state) {
>      sched_set_status((thread_t *)sched_active_thread, STATUS_SLEEPING);
>      irq_restore(state);
>      thread_yield_higher();
>  }
>
>  int thread_wakeup(kernel_pid_t pid)
>
> ```
>
> --Pekka Nikander
>
> _______________________________________________
> devel mailing list
> devel at riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/devel/attachments/20180911/a6ce8093/attachment.html>


More information about the devel mailing list