masayuki2009 commented on code in PR #7472:
URL: https://github.com/apache/incubator-nuttx/pull/7472#discussion_r1010159691


##########
arch/arm/src/sama5/sam_twi.c:
##########
@@ -1248,15 +1268,6 @@ struct i2c_master_s *sam_i2cbus_initialize(int bus)
       goto errout_with_lock;
     }
 
-  /* Initialize the TWI driver structure */
-
-  priv->dev.ops = &g_twiops;

Review Comment:
   @xiaoxiang781216 
   Does this change relate to the global mutex/sem initialization?
   



##########
arch/arm/src/sama5/sam_xdmac.c:
##########
@@ -732,24 +738,6 @@ static struct sam_xdmac_s g_xdmac1 =
  * Private Functions
  ****************************************************************************/
 
-/****************************************************************************
- * Name: sam_takedsem() and sam_givedsem()
- *
- * Description:
- *   Used to wait for availability of descriptors in the descriptor table.
- *
- ****************************************************************************/
-
-static int sam_takedsem(struct sam_xdmac_s *xdmac)
-{
-  return nxsem_wait_uninterruptible(&xdmac->dsem);
-}
-
-static inline void sam_givedsem(struct sam_xdmac_s *xdmac)
-{
-  nxsem_post(&xdmac->dsem);
-}
-

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_dmac.c:
##########
@@ -158,26 +154,6 @@ static struct dma_desc_s 
g_dma_desc[CONFIG_SAMD2L2_DMAC_NDESC]
  * Private Functions
  ****************************************************************************/
 
-/****************************************************************************
- * Name: sam_takedsem() and sam_givedsem()
- *
- * Description:
- *   Used to wait for availability of descriptors in the descriptor table.
- *
- ****************************************************************************/
-
-#if CONFIG_SAMD2L2_DMAC_NDESC > 0
-static void sam_takedsem(void)
-{
-  nxsem_wait_uninterruptible(&g_dsem);
-}
-
-static inline void sam_givedsem(void)
-{
-  nxsem_post(&g_dsem);
-}
-#endif
-

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/sama5/sam_xdmac.c:
##########
@@ -1407,7 +1395,7 @@ sam_allocdesc(struct sam_xdmach_s *xdmach, struct 
chnext_view1_s *prev,
       ret = nxmutex_lock(&xdmac->chlock);
       if (ret < 0)
         {
-          sam_givedsem(xdmac);
+          nxsem_post(&xdmac->dsem);

Review Comment:
   @xiaoxiang781216 
   Does this change relate to the global mutex/sem initialization?



##########
arch/arm/src/sama5/sam_xdmac.c:
##########
@@ -1392,7 +1380,7 @@ sam_allocdesc(struct sam_xdmach_s *xdmach, struct 
chnext_view1_s *prev,
        * there is at least one free descriptor in the table and it is ours.
        */
 
-      ret = sam_takedsem(xdmac);
+      ret = nxsem_wait_uninterruptible(&xdmac->dsem);

Review Comment:
   @xiaoxiang781216 
   Does this change relate to the global mutex/sem initialization?



##########
arch/arm/src/samd2l2/sam_dmac.c:
##########
@@ -99,10 +99,6 @@ struct sam_dmach_s
  * Private Function Prototypes
  ****************************************************************************/
 
-#if CONFIG_SAMD2L2_DMAC_NDESC > 0
-static void   sam_takedsem(void);
-static inline void sam_givedsem(void);
-#endif

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_dmac.c:
##########
@@ -512,7 +488,7 @@ static void sam_free_desc(struct sam_dmach_s *dmach)
 
       next = (struct dma_desc_s *)desc->descaddr;
       memset(desc, 0, sizeof(struct dma_desc_s));
-      sam_givedsem();
+      nxsem_post(&g_dsem);

Review Comment:
   @xiaoxiang781216 
   Does this change relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -287,7 +304,16 @@ static const struct i2c_attr_s g_i2c1attr =
   .base      = SAM_SERCOM1_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c1;
+static struct sam_i2c_dev_s g_i2c1 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c1attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -323,7 +358,16 @@ static const struct i2c_attr_s g_i2c3attr =
   .base      = SAM_SERCOM3_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c3;
+static struct sam_i2c_dev_s g_i2c3 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c3attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the mutex/sem initialization?



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -305,7 +331,16 @@ static const struct i2c_attr_s g_i2c2attr =
   .base      = SAM_SERCOM2_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c2;
+static struct sam_i2c_dev_s g_i2c2 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c2attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the mutex/sem initialization?



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -255,6 +255,14 @@ static void i2c_pad_configure(struct sam_i2c_dev_s *priv);
  * Private Data
  ****************************************************************************/
 
+static const struct i2c_ops_s g_i2cops =
+{
+  .transfer = sam_i2c_transfer,
+#ifdef CONFIG_I2C_RESET
+  .reset = sam_i2c_reset,
+#endif
+};
+

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -270,7 +278,16 @@ static const struct i2c_attr_s g_i2c0attr =
   .base      = SAM_SERCOM0_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c0;
+static struct sam_i2c_dev_s g_i2c0 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c0attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_dmac.c:
##########
@@ -356,7 +332,7 @@ static struct dma_desc_s *sam_alloc_desc(struct sam_dmach_s 
*dmach)
        * it is ours.
        */
 
-      sam_takedsem();
+      nxsem_wait_uninterruptible(&g_dsem);

Review Comment:
   @xiaoxiang781216 
   Does this change relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -359,16 +412,17 @@ static const struct i2c_attr_s g_i2c5attr =
   .base      = SAM_SERCOM5_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c5;
-#endif
-
-struct i2c_ops_s g_i2cops =
+static struct sam_i2c_dev_s g_i2c5 =
 {
-  .transfer = sam_i2c_transfer,
-#ifdef CONFIG_I2C_RESET
-  .reset = sam_i2c_reset,
-#endif
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c5attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -1253,83 +1307,59 @@ struct i2c_master_s *sam_i2c_master_initialize(int bus)
 #ifdef SAMD2L2_HAVE_I2C0
   if (bus == 0)
     {
-      /* Select up I2C0 and setup invariant attributes */
+      /* Select up I2C0 and the (initial) I2C frequency */
 
       priv = &g_i2c0;
-      priv->attr = &g_i2c0attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C0_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C1
   if (bus == 1)
     {
-      /* Select up I2C1 and setup invariant attributes */
+      /* Select up I2C1 and the (initial) I2C frequency */
 
       priv = &g_i2c1;
-      priv->attr = &g_i2c1attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C1_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C2
   if (bus == 2)
     {
-      /* Select up I2C2 and setup invariant attributes */
+      /* Select up I2C2 and the (initial) I2C frequency */
 
       priv = &g_i2c2;
-      priv->attr = &g_i2c2attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C2_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C3
   if (bus == 3)
     {
-      /* Select up I2C3 and setup invariant attributes */
+      /* Select up I2C3 and the (initial) I2C frequency */
 
       priv = &g_i2c3;
-      priv->attr = &g_i2c3attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C3_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C4
   if (bus == 4)
     {
-      /* Select up I2C4 and setup invariant attributes */
+      /* Select up I2C4 and the (initial) I2C frequency */
 
       priv = &g_i2c4;
-      priv->attr = &g_i2c4attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C4_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C5
   if (bus == 5)
     {
-      /* Select up I2C5 and setup invariant attributes */
+      /* Select up I2C5 and the (initial) I2C frequency */
 
       priv = &g_i2c5;
-      priv->attr = &g_i2c5attr;

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -341,7 +385,16 @@ static const struct i2c_attr_s g_i2c4attr =
   .base      = SAM_SERCOM4_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c4;
+static struct sam_i2c_dev_s g_i2c4 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c4attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -1353,14 +1383,6 @@ struct i2c_master_s *sam_i2c_master_initialize(int bus)
       return NULL;
     }
 
-  /* Initialize the I2C driver structure */
-
-  priv->dev.ops = &g_i2cops;
-  priv->flags = 0;

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -1253,83 +1307,59 @@ struct i2c_master_s *sam_i2c_master_initialize(int bus)
 #ifdef SAMD2L2_HAVE_I2C0
   if (bus == 0)
     {
-      /* Select up I2C0 and setup invariant attributes */
+      /* Select up I2C0 and the (initial) I2C frequency */
 
       priv = &g_i2c0;
-      priv->attr = &g_i2c0attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C0_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C1
   if (bus == 1)
     {
-      /* Select up I2C1 and setup invariant attributes */
+      /* Select up I2C1 and the (initial) I2C frequency */
 
       priv = &g_i2c1;
-      priv->attr = &g_i2c1attr;

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd5e5/sam_dmac.c:
##########
@@ -157,26 +153,6 @@ static struct dma_desc_s 
g_dma_desc[CONFIG_SAMD5E5_DMAC_NDESC]
  * Private Functions
  ****************************************************************************/
 
-/****************************************************************************
- * Name: sam_takedsem() and sam_givedsem()
- *
- * Description:
- *   Used to wait for availability of descriptors in the descriptor table.
- *
- ****************************************************************************/
-
-#if CONFIG_SAMD5E5_DMAC_NDESC > 0
-static void sam_takedsem(void)
-{
-  nxsem_wait_uninterruptible(&g_dsem);
-}
-
-static inline void sam_givedsem(void)
-{
-  nxsem_post(&g_dsem);
-}
-#endif

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -1253,83 +1307,59 @@ struct i2c_master_s *sam_i2c_master_initialize(int bus)
 #ifdef SAMD2L2_HAVE_I2C0
   if (bus == 0)
     {
-      /* Select up I2C0 and setup invariant attributes */
+      /* Select up I2C0 and the (initial) I2C frequency */
 
       priv = &g_i2c0;
-      priv->attr = &g_i2c0attr;

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -1253,83 +1307,59 @@ struct i2c_master_s *sam_i2c_master_initialize(int bus)
 #ifdef SAMD2L2_HAVE_I2C0
   if (bus == 0)
     {
-      /* Select up I2C0 and setup invariant attributes */
+      /* Select up I2C0 and the (initial) I2C frequency */
 
       priv = &g_i2c0;
-      priv->attr = &g_i2c0attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C0_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C1
   if (bus == 1)
     {
-      /* Select up I2C1 and setup invariant attributes */
+      /* Select up I2C1 and the (initial) I2C frequency */
 
       priv = &g_i2c1;
-      priv->attr = &g_i2c1attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C1_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C2
   if (bus == 2)
     {
-      /* Select up I2C2 and setup invariant attributes */
+      /* Select up I2C2 and the (initial) I2C frequency */
 
       priv = &g_i2c2;
-      priv->attr = &g_i2c2attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C2_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C3
   if (bus == 3)
     {
-      /* Select up I2C3 and setup invariant attributes */
+      /* Select up I2C3 and the (initial) I2C frequency */
 
       priv = &g_i2c3;
-      priv->attr = &g_i2c3attr;

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd5e5/sam_i2c_master.c:
##########
@@ -243,6 +243,14 @@ static void i2c_pad_configure(struct sam_i2c_dev_s *priv);
  * Private Data
  ****************************************************************************/
 
+static static struct i2c_ops_s g_i2cops =
+{
+  .transfer = sam_i2c_transfer,
+#ifdef CONFIG_I2C_RESET
+  .reset = sam_i2c_reset,
+#endif
+};
+

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd5e5/sam_i2c_master.c:
##########
@@ -276,7 +293,16 @@ static const struct i2c_attr_s g_i2c1attr =
   .base      = SAM_SERCOM1_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c1;
+static struct sam_i2c_dev_s g_i2c1 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c1attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd2l2/sam_i2c_master.c:
##########
@@ -1253,83 +1307,59 @@ struct i2c_master_s *sam_i2c_master_initialize(int bus)
 #ifdef SAMD2L2_HAVE_I2C0
   if (bus == 0)
     {
-      /* Select up I2C0 and setup invariant attributes */
+      /* Select up I2C0 and the (initial) I2C frequency */
 
       priv = &g_i2c0;
-      priv->attr = &g_i2c0attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C0_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C1
   if (bus == 1)
     {
-      /* Select up I2C1 and setup invariant attributes */
+      /* Select up I2C1 and the (initial) I2C frequency */
 
       priv = &g_i2c1;
-      priv->attr = &g_i2c1attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C1_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C2
   if (bus == 2)
     {
-      /* Select up I2C2 and setup invariant attributes */
+      /* Select up I2C2 and the (initial) I2C frequency */
 
       priv = &g_i2c2;
-      priv->attr = &g_i2c2attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C2_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C3
   if (bus == 3)
     {
-      /* Select up I2C3 and setup invariant attributes */
+      /* Select up I2C3 and the (initial) I2C frequency */
 
       priv = &g_i2c3;
-      priv->attr = &g_i2c3attr;
-
-      /* Select the (initial) I2C frequency */
-
       frequency = CONFIG_SAM_I2C3_FREQUENCY;
     }
   else
 #endif
 #ifdef SAMD2L2_HAVE_I2C4
   if (bus == 4)
     {
-      /* Select up I2C4 and setup invariant attributes */
+      /* Select up I2C4 and the (initial) I2C frequency */
 
       priv = &g_i2c4;
-      priv->attr = &g_i2c4attr;

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd5e5/sam_dmac.c:
##########
@@ -328,7 +304,7 @@ static struct dma_desc_s *sam_alloc_desc(struct sam_dmach_s 
*dmach)
        * it is ours.
        */
 
-      sam_takedsem();
+      nxsem_wait_uninterruptible(&g_dsem);

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd5e5/sam_dmac.c:
##########
@@ -100,10 +100,6 @@ struct sam_dmach_s
  * Private Function Prototypes
  ****************************************************************************/
 
-#if CONFIG_SAMD5E5_DMAC_NDESC > 0
-static void   sam_takedsem(void);
-static inline void sam_givedsem(void);
-#endif

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?
   



##########
arch/arm/src/samd5e5/sam_i2c_master.c:
##########
@@ -258,7 +266,16 @@ static const struct i2c_attr_s g_i2c0attr =
   .base      = SAM_SERCOM0_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c0;
+static struct sam_i2c_dev_s g_i2c0 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c0attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd5e5/sam_i2c_master.c:
##########
@@ -312,7 +347,16 @@ static const struct i2c_attr_s g_i2c3attr =
   .base      = SAM_SERCOM3_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c3;
+static struct sam_i2c_dev_s g_i2c3 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c3attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd5e5/sam_dmac.c:
##########
@@ -484,7 +460,7 @@ static void sam_free_desc(struct sam_dmach_s *dmach)
 
       next = (struct dma_desc_s *)desc->descaddr;
       memset(desc, 0, sizeof(struct dma_desc_s));
-      sam_givedsem();
+      nxsem_post(&g_dsem);

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd5e5/sam_i2c_master.c:
##########
@@ -294,7 +320,16 @@ static const struct i2c_attr_s g_i2c2attr =
   .base      = SAM_SERCOM2_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c2;
+static struct sam_i2c_dev_s g_i2c2 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c2attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



##########
arch/arm/src/samd5e5/sam_i2c_master.c:
##########
@@ -330,7 +374,16 @@ static const struct i2c_attr_s g_i2c4attr =
   .base      = SAM_SERCOM4_BASE,
 };
 
-static struct sam_i2c_dev_s g_i2c4;
+static struct sam_i2c_dev_s g_i2c4 =
+{
+  .dev       =
+  {
+    .ops     = &g_i2cops,
+  },
+  .attr      = &g_i2c4attr,

Review Comment:
   @xiaoxiang781216 
   Do these changes relate to the global mutex/sem initialization?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to