You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/04/04 01:22:13 UTC

[GitHub] [incubator-nuttx] acassis opened a new pull request #719: Kinetis lpc sdcard

acassis opened a new pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405413
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -334,13 +340,16 @@ static void lpc17_40_dmacallback(DMA_HANDLE handle, void *arg, int status);
 /* Data Transfer Helpers ****************************************************/
 
 static uint8_t lpc17_40_log2(uint16_t value);
-static void lpc17_40_dataconfig(uint32_t timeout, uint32_t dlen, uint32_t dctrl);
+static void lpc17_40_dataconfig(uint32_t timeout, uint32_t dlen,
+                                uint32_t dctrl);
 static void lpc17_40_datadisable(void);
 static void lpc17_40_sendfifo(struct lpc17_40_dev_s *priv);
 static void lpc17_40_recvfifo(struct lpc17_40_dev_s *priv);
 static void lpc17_40_eventtimeout(int argc, uint32_t arg);
-static void lpc17_40_endwait(struct lpc17_40_dev_s *priv, sdio_eventset_t wkupevent);
-static void lpc17_40_endtransfer(struct lpc17_40_dev_s *priv, sdio_eventset_t wkupevent);
+static void lpc17_40_endwait(struct lpc17_40_dev_s *priv,
+                             sdio_eventset_t wkupevent);
 
 Review comment:
   Same comment as above.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo merged pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405346
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -305,9 +308,11 @@ struct lpc17_40_sampleregs_s
 static void lpc17_40_takesem(struct lpc17_40_dev_s *priv);
 #define     lpc17_40_givesem(priv) (nxsem_post(&priv->waitsem))
 static inline void lpc17_40_setclock(uint32_t clkcr);
-static void lpc17_40_configwaitints(struct lpc17_40_dev_s *priv, uint32_t waitmask,
+static void lpc17_40_configwaitints(struct lpc17_40_dev_s *priv,
+                                    uint32_t waitmask,
               sdio_eventset_t waitevents, sdio_eventset_t wkupevents);
 
 Review comment:
   This indentation style is inconstistent with the indentation style used elsewhere (see below)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405255
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -317,7 +322,8 @@ static inline uint32_t lpc17_40_getpwrctrl(void);
 static void lpc17_40_sampleinit(void);
 static void lpc17_40_sdcard_sample(struct lpc17_40_sdcard_regs_s *regs);
 static void lpc17_40_sample(struct lpc17_40_dev_s *priv, int index);
-static void lpc17_40_sdcard_dump(struct lpc17_40_sdcard_regs_s *regs, const char *msg);
+static void lpc17_40_sdcard_dump(struct lpc17_40_sdcard_regs_s *regs,
+                                 const char *msg);
 static void lpc17_40_dumpsample(struct lpc17_40_dev_s *priv,
               struct lpc17_40_sampleregs_s *regs, const char *msg);
 static void lpc17_40_dumpsamples(struct lpc17_40_dev_s *priv);
 
 Review comment:
   
   This is the correct alignment to use in this file.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] acassis commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405996
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -317,7 +322,8 @@ static inline uint32_t lpc17_40_getpwrctrl(void);
 static void lpc17_40_sampleinit(void);
 static void lpc17_40_sdcard_sample(struct lpc17_40_sdcard_regs_s *regs);
 static void lpc17_40_sample(struct lpc17_40_dev_s *priv, int index);
-static void lpc17_40_sdcard_dump(struct lpc17_40_sdcard_regs_s *regs, const char *msg);
+static void lpc17_40_sdcard_dump(struct lpc17_40_sdcard_regs_s *regs,
+                                 const char *msg);
 
 Review comment:
   Hmm, I missed the column start position, normally I use as reference the "(" in the previous line. But some cases like ABIGMACRONAMEA | ABIGMACRONAMEB I try to keep it aligned the beginning of the first macro name. I will fix it

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405251
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -317,7 +322,8 @@ static inline uint32_t lpc17_40_getpwrctrl(void);
 static void lpc17_40_sampleinit(void);
 static void lpc17_40_sdcard_sample(struct lpc17_40_sdcard_regs_s *regs);
 static void lpc17_40_sample(struct lpc17_40_dev_s *priv, int index);
-static void lpc17_40_sdcard_dump(struct lpc17_40_sdcard_regs_s *regs, const char *msg);
+static void lpc17_40_sdcard_dump(struct lpc17_40_sdcard_regs_s *regs,
+                                 const char *msg);
 
 Review comment:
   Indentation style is not consistent.  In all other prototypes, the argument that continue on the same coleumn.  Like the following.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405395
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -334,13 +340,16 @@ static void lpc17_40_dmacallback(DMA_HANDLE handle, void *arg, int status);
 /* Data Transfer Helpers ****************************************************/
 
 static uint8_t lpc17_40_log2(uint16_t value);
-static void lpc17_40_dataconfig(uint32_t timeout, uint32_t dlen, uint32_t dctrl);
+static void lpc17_40_dataconfig(uint32_t timeout, uint32_t dlen,
+                                uint32_t dctrl);
 static void lpc17_40_datadisable(void);
 static void lpc17_40_sendfifo(struct lpc17_40_dev_s *priv);
 static void lpc17_40_recvfifo(struct lpc17_40_dev_s *priv);
 static void lpc17_40_eventtimeout(int argc, uint32_t arg);
-static void lpc17_40_endwait(struct lpc17_40_dev_s *priv, sdio_eventset_t wkupevent);
-static void lpc17_40_endtransfer(struct lpc17_40_dev_s *priv, sdio_eventset_t wkupevent);
+static void lpc17_40_endwait(struct lpc17_40_dev_s *priv,
+                             sdio_eventset_t wkupevent);
 
 Review comment:
   Same comment as above

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405854
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -2290,32 +2338,44 @@ static sdio_eventset_t lpc17_40_eventwait(FAR struct sdio_dev_s *dev,
       /* Start the watchdog timer */
 
       delay = MSEC2TICK(timeout);
-      ret   = wd_start(priv->waitwdog, delay, (wdentry_t)lpc17_40_eventtimeout,
+      ret   = wd_start(priv->waitwdog, delay,
+                       (wdentry_t) lpc17_40_eventtimeout,
                        1, (uint32_t)priv);
       if (ret < 0)
         {
           mcerr("ERROR: wd_start failed: %d\n", ret);
         }
     }
 
-  /* Loop until the event (or the timeout occurs). Race conditions are avoided
-   * by calling lpc17_40_waitenable prior to triggering the logic that will cause
-   * the wait to terminate.  Under certain race conditions, the waited-for
-   * may have already occurred before this function was called!
+  /* Loop until the event (or the timeout occurs). Race conditions are
+   * avoided by calling lpc17_40_waitenable prior to triggering the logic
+   * that will cause the wait to terminate.  Under certain race conditions,
+   * the waited-for may have already occurred before this function was
+   * called!
    */
 
   for (; ; )
     {
-      /* Wait for an event in event set to occur.  If this the event has already
-       * occurred, then the semaphore will already have been incremented and
-       * there will be no wait.
+      /* Wait for an event in event set to occur.  If this the event has
+       * already occurred, then the semaphore will already have been
+       * incremented and there will be no wait.
        */
 
-      lpc17_40_takesem(priv);
+      ret = lpc17_40_takesem(priv);
+      if (ret < 0)
+        {
+          wd_cancel(priv->waitwdog);
+
+          leave_critical_section(flags);
+
+          return SDIOWAIT_ERROR;
+        }
+
       wkupevent = priv->wkupevent;
 
 Review comment:
   LGTM

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #719: Kinetis lpc sdcard

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #719: Kinetis lpc sdcard
URL: https://github.com/apache/incubator-nuttx/pull/719#discussion_r403405436
 
 

 ##########
 File path: arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
 ##########
 @@ -368,8 +377,9 @@ static int  lpc17_40_attach(FAR struct sdio_dev_s *dev);
 
 static int  lpc17_40_sendcmd(FAR struct sdio_dev_s *dev, uint32_t cmd,
               uint32_t arg);
-static int  lpc17_40_recvsetup(FAR struct sdio_dev_s *dev, FAR uint8_t *buffer,
-              size_t nbytes);
+static int  lpc17_40_recvsetup(FAR struct sdio_dev_s *dev,
+                               FAR uint8_t *buffer,
+                               size_t nbytes);
 static int  lpc17_40_sendsetup(FAR struct sdio_dev_s *dev,
 
 Review comment:
   Same comment as above.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services