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.nikan...@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.nohlg...@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.nikan...@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@riot-os.org >> https://lists.riot-os.org/mailman/listinfo/devel >> > _______________________________________________ > devel mailing list > devel@riot-os.org > https://lists.riot-os.org/mailman/listinfo/devel > > > _______________________________________________ > devel mailing list > devel@riot-os.org > https://lists.riot-os.org/mailman/listinfo/devel >
_______________________________________________ devel mailing list devel@riot-os.org https://lists.riot-os.org/mailman/listinfo/devel