xiaoxiang781216 commented on code in PR #6965:
URL: https://github.com/apache/incubator-nuttx/pull/6965#discussion_r985413403
##########
arch/arm/src/cxd56xx/cxd56_udmac.c:
##########
@@ -238,7 +239,7 @@ void cxd56_udmainitialize(void)
/* Initialize the channel list */
- nxsem_init(&g_dmac.exclsem, 0, 1);
+ nxmutex_init(&g_dmac.lock);
nxsem_init(&g_dmac.chansem, 0, CXD56_DMA_NCHANNELS);
Review Comment:
> Let's add `nxsem_set_protocol(&g_dmac.chansem, SEM_PRIO_NONE);`?
@anjiahao1 will create a new patch which disable priority inheritance by
default, so we can ignore the problem temporarily.
> Or waiting on DMA channel should rise priority as well?
No, the priority inheritance require sem_wait/sem_post come from the same
thread, otherwise the system will panic.
##########
arch/arm/src/efm32/efm32_dma.c:
##########
@@ -265,7 +266,7 @@ void weak_function arm_dma_initialize(void)
/* Initialize the channel list */
- nxsem_init(&g_dmac.exclsem, 0, 1);
+ nxmutex_init(&g_dmac.lock);
nxsem_init(&g_dmac.chansem, 0, EFM32_DMA_NCHANNELS);
Review Comment:
ditto
##########
arch/arm/src/cxd56xx/cxd56_udmac.c:
##########
@@ -238,7 +239,7 @@ void cxd56_udmainitialize(void)
/* Initialize the channel list */
- nxsem_init(&g_dmac.exclsem, 0, 1);
+ nxmutex_init(&g_dmac.lock);
nxsem_init(&g_dmac.chansem, 0, CXD56_DMA_NCHANNELS);
Review Comment:
> Let's add `nxsem_set_protocol(&g_dmac.chansem, SEM_PRIO_NONE);`?
@anjiahao1 will create a new patch which disable priority inheritance by
default and remove all `nxsem_set_protocol(&g_dmac.chansem, SEM_PRIO_NONE);`
after this patch get merged, so we can ignore the problem temporarily.
> Or waiting on DMA channel should rise priority as well?
No, the priority inheritance require sem_wait/sem_post come from the same
thread, otherwise the system will panic.
##########
arch/arm/src/cxd56xx/cxd56_udmac.c:
##########
@@ -238,7 +239,7 @@ void cxd56_udmainitialize(void)
/* Initialize the channel list */
- nxsem_init(&g_dmac.exclsem, 0, 1);
+ nxmutex_init(&g_dmac.lock);
nxsem_init(&g_dmac.chansem, 0, CXD56_DMA_NCHANNELS);
Review Comment:
> Let's add `nxsem_set_protocol(&g_dmac.chansem, SEM_PRIO_NONE);`?
@anjiahao1 will create a new patch which disable priority inheritance by
default and remove all `nxsem_set_protocol(&g_dmac.chansem, SEM_PRIO_NONE);`
after this patch get merged, so we can ignore the problem temporarily.
> Or waiting on DMA channel should rise priority as well?
No, the priority inheritance require sem_wait/sem_post come from the same
thread, otherwise the system will panic. This is also the key reason why is a
bad idea to enable priority inheritance by default.
##########
arch/arm/src/sama5/sam_ssc.c:
##########
@@ -931,7 +884,7 @@ static struct sam_buffer_s *ssc_buf_allocate(struct
sam_ssc_s *priv)
* have at least one free buffer container.
*/
- ret = ssc_bufsem_take(priv);
+ ret = nxsem_wait_uninterruptible(&priv->bufsem);
Review Comment:
Let's do in the next patch
##########
arch/arm/src/sama5/sam_ohci.c:
##########
@@ -4016,7 +3963,7 @@ struct usbhost_connection_s *sam_ohci_initialize(int
controller)
/* Initialize the state data structure */
nxsem_init(&g_ohci.pscsem, 0, 0);
Review Comment:
Done.
##########
arch/arm/src/sama5/sam_ehci.c:
##########
@@ -4881,7 +4828,7 @@ struct usbhost_connection_s *sam_ehci_initialize(int
controller)
/* Initialize the EHCI state data structure */
- nxsem_init(&g_ehci.exclsem, 0, 1);
+ nxmutex_init(&g_ehci.lock);
nxsem_init(&g_ehci.pscsem, 0, 0);
Review Comment:
Done.
##########
arch/arm/src/sama5/sam_dmac.c:
##########
@@ -1873,9 +1855,9 @@ void sam_dmainitialize(struct sam_dmac_s *dmac)
sam_putdmac(dmac, DMAC_EN_ENABLE, SAM_DMAC_EN_OFFSET);
- /* Initialize semaphores */
+ /* Initialize muttex & semaphores */
- nxsem_init(&dmac->chsem, 0, 1);
+ nxmutex_init(&dmac->chlock);
nxsem_init(&dmac->dsem, 0, SAM_NDMACHAN);
Review Comment:
In the next patch
##########
arch/arm/src/sam34/sam_dmac.c:
##########
@@ -1380,9 +1341,9 @@ void weak_function arm_dma_initialize(void)
putreg32(DMAC_EN_ENABLE, SAM_DMAC_EN);
- /* Initialize semaphores */
+ /* Initialize mutex & semaphores */
- nxsem_init(&g_chsem, 0, 1);
+ nxmutex_init(&g_chlock);
nxsem_init(&g_dsem, 0, CONFIG_SAM34_NLLDESC);
Review Comment:
Done.
##########
arch/arm/src/lpc54xx/lpc54_usb0_ohci.c:
##########
@@ -3884,10 +3829,10 @@ struct usbhost_connection_s
*lpc54_usbhost_initialize(int controller)
usbhost_devaddr_initialize(&priv->rhport);
- /* Initialize semaphores */
+ /* Initialize semaphores & mutex */
nxsem_init(&priv->pscsem, 0, 0);
Review Comment:
Done.
##########
arch/arm/src/sam34/sam_aes.c:
##########
@@ -169,7 +159,7 @@ static int samaes_setup_mr(uint32_t keysize, int mode, int
encrypt)
static int samaes_initialize(void)
{
- nxsem_init(&g_samaes_lock, 0, 1);
+ nxmutex_init(&g_samaes_lock);
Review Comment:
Done.
##########
arch/arm/src/sam34/sam_aes.c:
##########
@@ -59,8 +59,8 @@
* Private Data
****************************************************************************/
-static sem_t g_samaes_lock;
-static bool g_samaes_initdone = false;
+static mutex_t g_samaes_lock;
+static bool g_samaes_initdone = false;
Review Comment:
Done.
##########
arch/arm/src/rp2040/rp2040_dmac.c:
##########
@@ -160,7 +161,7 @@ void weak_function arm_dma_initialize(void)
/* Initialize the channel list */
- nxsem_init(&g_dmac.exclsem, 0, 1);
+ nxmutex_init(&g_dmac.lock);
nxsem_init(&g_dmac.chansem, 0, RP2040_DMA_NCHANNELS);
Review Comment:
Next patch
##########
arch/arm/src/lc823450/lc823450_sddrv_dep.c:
##########
@@ -427,7 +418,7 @@ SINT_T sddep_read(void *src, void *dst, UI_32 size, SINT_T
type,
}
lc823450_dmastart(_hrdma[ch], dma_callback, &_sem_rwait[ch]);
- return _sddep_semtake(&_sem_rwait[ch]);
+ return nxsem_wait_uninterruptible(&_sem_rwait[ch]);
Review Comment:
next patch
##########
arch/arm/src/lc823450/lc823450_i2s.c:
##########
@@ -686,7 +677,7 @@ static int lc823450_i2s_send(struct i2s_dev_s *dev, struct
ap_buffer_s *apb,
/* Wait for Audio Buffer */
- ret = _i2s_semtake(&_sem_buf_under);
+ ret = nxsem_wait_uninterruptible(&_sem_buf_under);
Review Comment:
next patch
##########
arch/arm/src/lc823450/lc823450_adc.c:
##########
@@ -538,12 +507,12 @@ struct adc_dev_s *lc823450_adcinitialize(void)
inst->nchannels = CONFIG_LC823450_ADC_NCHANNELS;
inst->chanlist = lc823450_chanlist;
- nxsem_init(&inst->sem_excl, 0, 1);
+ nxmutex_init(&inst->lock);
#ifndef CONFIG_ADC_POLLED
nxsem_init(&inst->sem_isr, 0, 0);
Review Comment:
next patch
##########
arch/arm/src/efm32/efm32_usbhost.c:
##########
@@ -5277,10 +5259,10 @@ static inline void efm32_sw_initialize(struct
efm32_usbhost_s *priv)
usbhost_devaddr_initialize(&priv->rhport);
- /* Initialize semaphores */
+ /* Initialize semaphores & mutex */
nxsem_init(&priv->pscsem, 0, 0);
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
