pkarashchenko commented on code in PR #6965:
URL: https://github.com/apache/incubator-nuttx/pull/6965#discussion_r994989775
##########
arch/arm/src/lpc54xx/lpc54_dma.c:
##########
@@ -437,8 +438,8 @@ int lpc54_dma_setup(int ch, uint32_t cfg, uint32_t xfrcfg,
uint8_t trigsrc,
putreg32(xfrcfg, base + LPC54_DMA_XFERCFG_OFFSET);
ret = OK;
-errout_with_exclsem:
- nxsem_post(&g_dma.exclsem);
+errout_with_excllock:
Review Comment:
```suggestion
errout_with_lock:
```
##########
arch/arm/src/cxd56xx/cxd56_udmac.c:
##########
@@ -67,8 +68,8 @@ struct dma_channel_s
struct dma_controller_s
{
- sem_t exclsem; /* Protects channel table */
- sem_t chansem; /* Count of free channels */
+ mutex_t lock; /* Protects channel table */
+ sem_t chansem; /* Count of free channels */
Review Comment:
```suggestion
mutex_t lock; /* Protects channel table */
sem_t chansem; /* Count of free channels */
```
just to align with `dma_channel_s` above
##########
arch/arm/src/sama5/sam_ssc.c:
##########
@@ -2434,7 +2387,7 @@ static int ssc_send(struct i2s_dev_s *dev, struct
ap_buffer_s *apb,
{
i2serr("ERROR: SSC%d has no transmitter\n", priv->sscno);
ret = -EAGAIN;
- goto errout_with_exclsem;
+ goto errout_with_excllock;
Review Comment:
```suggestion
goto errout_with_lock;
```
##########
arch/arm/src/lpc54xx/lpc54_dma.c:
##########
@@ -307,7 +308,7 @@ int lpc54_dma_setup(int ch, uint32_t cfg, uint32_t xfrcfg,
uint8_t trigsrc,
if (dmach->inuse)
{
ret = -EBUSY;
- goto errout_with_exclsem;
+ goto errout_with_excllock;
Review Comment:
```suggestion
goto errout_with_lock;
```
##########
arch/arm/src/stm32/stm32_dma2d.c:
##########
@@ -111,7 +111,7 @@ struct stm32_dma2d_s
uint32_t *clut; /* Color lookup table */
#endif
- sem_t *lock; /* Ensure mutually exclusive access */
+ mutex_t *lock; /* Ensure mutually exclusive access */
Review Comment:
```suggestion
mutex_t *lock; /* Ensure mutually exclusive access */
```
##########
arch/arm/src/stm32f7/stm32_i2c.c:
##########
@@ -2869,43 +2817,36 @@ struct i2c_master_s *stm32_i2cbus_initialize(int port)
*
****************************************************************************/
-int stm32_i2cbus_uninitialize(struct i2c_master_s * dev)
+int stm32_i2cbus_uninitialize(struct i2c_master_s *dev)
{
- irqstate_t irqs;
+ struct stm32_i2c_priv_s *priv = ((struct stm32_i2c_inst_s *)dev)->priv;
Review Comment:
let's fix this again
##########
arch/arm/src/sama5/sam_ssc.c:
##########
@@ -2462,11 +2415,11 @@ static int ssc_send(struct i2s_dev_s *dev, struct
ap_buffer_s *apb,
ret = ssc_txdma_setup(priv);
DEBUGASSERT(ret == OK);
leave_critical_section(flags);
- ssc_exclsem_give(priv);
+ nxmutex_unlock(&priv->lock);
return OK;
-errout_with_exclsem:
- ssc_exclsem_give(priv);
+errout_with_excllock:
Review Comment:
```suggestion
errout_with_lock:
```
##########
arch/arm/src/sama5/sam_ssc.c:
##########
@@ -2239,11 +2192,11 @@ static int ssc_receive(struct i2s_dev_s *dev, struct
ap_buffer_s *apb,
ret = ssc_rxdma_setup(priv);
DEBUGASSERT(ret == OK);
leave_critical_section(flags);
- ssc_exclsem_give(priv);
+ nxmutex_unlock(&priv->lock);
return OK;
-errout_with_exclsem:
- ssc_exclsem_give(priv);
+errout_with_excllock:
Review Comment:
```suggestion
errout_with_lock:
```
##########
arch/arm/src/sama5/sam_ssc.c:
##########
@@ -2211,7 +2164,7 @@ static int ssc_receive(struct i2s_dev_s *dev, struct
ap_buffer_s *apb,
{
i2serr("ERROR: SSC%d has no receiver\n", priv->sscno);
ret = -EAGAIN;
- goto errout_with_exclsem;
+ goto errout_with_excllock;
Review Comment:
```suggestion
goto errout_with_lock;
```
##########
arch/arm/src/rp2040/rp2040_dmac.c:
##########
@@ -61,8 +62,8 @@ struct dma_channel_s
struct dma_controller_s
{
- sem_t exclsem; /* Protects channel table */
- sem_t chansem; /* Count of free channels */
+ mutex_t lock; /* Protects channel table */
+ sem_t chansem; /* Count of free channels */
Review Comment:
let's align with comments in `struct dma_channel_s` above
##########
arch/arm/src/samv7/sam_ssc.c:
##########
@@ -2222,11 +2175,11 @@ static int ssc_receive(struct i2s_dev_s *dev, struct
ap_buffer_s *apb,
ret = ssc_rxdma_setup(priv);
DEBUGASSERT(ret == OK);
leave_critical_section(flags);
- ssc_exclsem_give(priv);
+ nxmutex_unlock(&priv->lock);
return OK;
-errout_with_exclsem:
- ssc_exclsem_give(priv);
+errout_with_excllock:
Review Comment:
```suggestion
errout_with_lock:
```
##########
arch/arm/src/stm32f0l0g0/stm32_i2c.c:
##########
@@ -2845,43 +2778,36 @@ struct i2c_master_s *stm32_i2cbus_initialize(int port)
*
****************************************************************************/
-int stm32_i2cbus_uninitialize(struct i2c_master_s * dev)
+int stm32_i2cbus_uninitialize(struct i2c_master_s *dev)
{
- irqstate_t irqs;
+ struct stm32_i2c_priv_s *priv = ((struct stm32_i2c_inst_s *)dev)->priv;
Review Comment:
Let's fix this again
##########
arch/arm/src/samd5e5/sam_usb.c:
##########
@@ -8566,10 +8531,10 @@ static inline void sam_sw_initialize(struct
sam_usbhost_s *priv)
sam_reset_pipes(priv, false);
- /* Initialize semaphores */
+ /* Initialize semaphore & mutex */
Review Comment:
In some places `&` is used and in other `and`. So maybe we can use similar
style in all the places?
--
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
