Skip to content
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

tud_audio_write()/tud_audio_read() from ISR when using RTOS #2738

Open
1 task done
ra1nb0w opened this issue Jul 25, 2024 · 3 comments
Open
1 task done

tud_audio_write()/tud_audio_read() from ISR when using RTOS #2738

ra1nb0w opened this issue Jul 25, 2024 · 3 comments

Comments

@ra1nb0w
Copy link
Contributor

ra1nb0w commented Jul 25, 2024

Related area

tusb_fifo

Hardware specification

STM32H563

Is your feature request related to a problem?

I would like to use tud_audio_write() and tud_audio_read() from ISR when RTOS support is enabled (I am using FreeRTOS).
If you call it from ISR you end up with an issue in the mutex [1] since you need to use specific ISR aware functions.

[1]

_ff_lock(f->mutex_wr);

Describe the solution you'd like

At the moment I am using a custom ARM check, see later, but should be nice to have a generic way to do it like bool in_isr or maybe there is another way to do it correctly. My specific use case is calling it from I2S DMA callback.
Thanks

diff --git a/src/common/tusb_fifo.c b/src/common/tusb_fifo.c
index 8a0fd4417..7e4ed81db 100644
--- a/src/common/tusb_fifo.c
+++ b/src/common/tusb_fifo.c
@@ -471,7 +471,7 @@ static uint16_t _tu_fifo_write_n(tu_fifo_t* f, const void * data, uint16_t n, tu
 {
   if ( n == 0 ) return 0;
 
-  _ff_lock(f->mutex_wr);
+  if ( (SCB->ICSR & SCB_ICSR_VECTACTIVE_Msk) == 0 ) _ff_lock(f->mutex_wr);
 
   uint16_t wr_idx = f->wr_idx;
   uint16_t rd_idx = f->rd_idx;
@@ -549,14 +549,14 @@ static uint16_t _tu_fifo_write_n(tu_fifo_t* f, const void * data, uint16_t n, tu
     TU_LOG(TU_FIFO_DBG, "\tnew_wr = %u\r\n", f->wr_idx);
   }
 
-  _ff_unlock(f->mutex_wr);
+  if ( (SCB->ICSR & SCB_ICSR_VECTACTIVE_Msk) == 0 ) _ff_unlock(f->mutex_wr);
 
   return n;
 }
 
 static uint16_t _tu_fifo_read_n(tu_fifo_t* f, void * buffer, uint16_t n, tu_fifo_copy_mode_t copy_mode)
 {
-  _ff_lock(f->mutex_rd);
+  if ( (SCB->ICSR & SCB_ICSR_VECTACTIVE_Msk) == 0 ) _ff_lock(f->mutex_rd);
 
   // Peek the data
   // f->rd_idx might get modified in case of an overflow so we can not use a local variable
@@ -565,7 +565,7 @@ static uint16_t _tu_fifo_read_n(tu_fifo_t* f, void * buffer, uint16_t n, tu_fifo
   // Advance read pointer
   f->rd_idx = advance_index(f->depth, f->rd_idx, n);
 
-  _ff_unlock(f->mutex_rd);
+  if ( (SCB->ICSR & SCB_ICSR_VECTACTIVE_Msk) == 0 ) _ff_unlock(f->mutex_rd);
   return n;
 }
 

I have checked existing issues, dicussion and documentation

  • I confirm I have checked existing issues, dicussion and documentation.
@ra1nb0w
Copy link
Contributor Author

ra1nb0w commented Aug 27, 2024

Can be useful to others? How to generalize the idea such that it can be committed in master?
Thanks

@cdesjardins
Copy link
Contributor

Just my 2 cents. this is a bug. It is not ok to just ignore a mutex if you are in ISR. the correct solution is to trigger a task to do the I/O. give a semaphore from ISR, or send a message, that unblocks your task which does the I/O.

@ra1nb0w
Copy link
Contributor Author

ra1nb0w commented Sep 23, 2024

yes, it is a dirty hack and the ignored mutexes are managed by ISR priority. I used your suggestion at the beginning but I would like to copy the I2S data upon I receive the DMA ISR.
Anyway, thanks for your comment.

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

No branches or pull requests

2 participants