You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2020/06/05 11:50:36 UTC

[incubator-nuttx] branch master updated: cxd56: Fix lock issue in Spresense audio driver

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new 368fbd0  cxd56: Fix lock issue in Spresense audio driver
368fbd0 is described below

commit 368fbd0dea6cddfb970faa39491b68a57f72b928
Author: Tobias Johansson <to...@sony.com>
AuthorDate: Thu Jun 4 13:37:29 2020 +0200

    cxd56: Fix lock issue in Spresense audio driver
    
    Replace semaphore with spinlock in the DMA buffer handling code
    since it is called from an interrupt.
---
 drivers/audio/cxd56.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/audio/cxd56.c b/drivers/audio/cxd56.c
index 9d5cae4..602e4ea 100644
--- a/drivers/audio/cxd56.c
+++ b/drivers/audio/cxd56.c
@@ -430,8 +430,6 @@ static void cxd56_set_mic_gains(uint8_t gain,
 static void cxd56_set_mic_out_channel(FAR struct cxd56_dev_s *dev);
 static int cxd56_set_volume(enum cxd56_vol_id_e id, int16_t vol);
 static void cxd56_swap_buffer_rl(uint32_t addr, uint16_t size);
-static int  cxd56_take_sem(sem_t *sem);
-#define cxd56_give_sem(s) (nxsem_post(s))
 static void *cxd56_workerthread(pthread_addr_t pvarg);
 
 /****************************************************************************
@@ -1097,20 +1095,6 @@ static void write_reg_addr(const cxd56_aureg_t *reg, uint32_t val)
 #define write_reg(reg, val)   (write_reg_addr(&(reg), (val)))
 #define write_reg32(reg, val) (*((volatile uint32_t *)(reg).addr) = (val))
 
-/****************************************************************************
- * Name: cxd56_take_sem
- *
- * Description:
- *  Take a semaphore count, handling the nasty EINTR return if we are
- *  interrupted by a signal.
- *
- ****************************************************************************/
-
-static int cxd56_take_sem(sem_t *sem)
-{
-  return nxsem_wait_uninterruptible(sem);
-}
-
 static void cxd56_int_clear(cxd56_dmahandle_t handle, uint8_t intbits)
 {
   if (handle == CXD56_AUDIO_DMA_MIC)
@@ -1383,13 +1367,14 @@ static void cxd56_dma_int_handler(void)
     {
       struct audio_msg_s msg;
       struct cxd56_dev_s *dev;
+      irqstate_t flags;
       int ret;
 
       dev = g_dev[hdl];
 
       /* Trigger new DMA job */
 
-      cxd56_take_sem(&dev->pendsem);
+      flags = spin_lock_irqsave();
 
       if (dq_count(&dev->runningq) > 0)
         {
@@ -1399,7 +1384,7 @@ static void cxd56_dma_int_handler(void)
           dev->dev.upper(dev->dev.priv, AUDIO_CALLBACK_DEQUEUE, apb, OK);
         }
 
-      cxd56_give_sem(&dev->pendsem);
+      spin_unlock_irqrestore(flags);
 
       if (err_code == CXD56_AUDIO_ECODE_DMA_TRANS)
         {
@@ -3067,13 +3052,14 @@ static void cxd56_swap_buffer_rl(uint32_t addr, uint16_t size)
 static int cxd56_start_dma(FAR struct cxd56_dev_s *dev)
 {
   FAR struct ap_buffer_s *apb;
+  irqstate_t flags;
   int retry;
   int timeout;
   uint32_t addr;
   uint32_t size;
   int ret = OK;
 
-  cxd56_take_sem(&dev->pendsem);
+  flags = spin_lock_irqsave();
   if (dq_count(&dev->pendingq) == 0)
     {
       /* Pending queue empty, nothing to do */
@@ -3242,7 +3228,7 @@ static int cxd56_start_dma(FAR struct cxd56_dev_s *dev)
     }
 
 exit:
-  cxd56_give_sem(&dev->pendsem);
+  spin_unlock_irqrestore(flags);
 
   return ret;
 }
@@ -3259,11 +3245,14 @@ static int cxd56_enqueuebuffer(FAR struct audio_lowerhalf_s *lower,
 {
   FAR struct cxd56_dev_s *priv = (FAR struct cxd56_dev_s *)lower;
   struct audio_msg_s msg;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave();
 
-  cxd56_take_sem(&priv->pendsem);
   apb->dq_entry.flink = NULL;
   dq_put(&priv->pendingq, &apb->dq_entry);
-  cxd56_give_sem(&priv->pendsem);
+
+  spin_unlock_irqrestore(flags);
 
   if (priv->mq != NULL)
     {