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
