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

Joakim Nohlgård joakim.nohlgard at eistec.se
Tue Sep 11 21:34:00 CEST 2018


Hi again,
Are you on a cortex m platform? Did you remember to add cortexm_isr_end()
at the end of the ISR function?
/Joakim

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

> Hi Joakim,
>
> I tried also exactly that, but for some reason I couldn't make it work.
> When I tried to unlock it in the ISR, the thread waiting for the mutex for
> the second time still didn't acquire it, but was blocked again in the
> scheduler.  I didn't dive deep into the scheduler to see why.  Maybe I did
> some mistake.
>
> Could you please point to some example code?
>
> --Pekka
>
> On 11.9.2018, at 20:48, Joakim Nohlgård <joakim.nohlgard at eistec.se> wrote:
>
> 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
>>
> _______________________________________________
> devel mailing list
> devel at riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel
>
>
> _______________________________________________
> 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/e111c972/attachment.html>


More information about the devel mailing list