[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6510: Support for 2.13 inch display v2

[GitBox <gi...@apache.org>]

pkarashchenko commented on code in PR #6510:
URL: https://github.com/apache/incubator-nuttx/pull/6510#discussion_r906665867


##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,30 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_ticks = 200;
   if ((priv->board_priv != NULL) && (priv->board_priv->check_busy != NULL))
     {
-      while (priv->board_priv->check_busy())
+      while (priv->board_priv->check_busy() && maks_wait_ticks > 0)
         {
-          nxsig_usleep(1);
+          nxsig_usleep(1000);
+          maks_wait_ticks--;
         }
     }
   else
     {
-      nxsig_usleep(2000000);
+      nxsig_usleep(maks_wait_ticks);

Review Comment:
   maybe
   ```suggestion
         nxsig_usleep(maks_wait_ticks * 1000);
   ```
   ?



##########
drivers/lcd/ssd1680.c:
##########
@@ -567,49 +567,96 @@ static int ssd1680_configuredisplay(struct ssd1680_dev_s *priv)
 
   /* Busy wait */
 
-  ssd1680_busy_wait(priv);
-  lcdinfo("SSD1680 is ready\n");
+  ret = ssd1680_busy_wait(priv);
+  if (ret != OK)
+    {
+      goto exit_err;

Review Comment:
   IMO duplicating `lcderr("Configuration is not ready\n");` here + `return ret;` is better than `goto`. Anyway if the mesage is the same, then compiler till optimize and generate the code similar to goto.



##########
drivers/lcd/ssd1680.c:
##########
@@ -878,19 +896,30 @@ static void ssd1680_select(FAR struct ssd1680_dev_s *priv, bool cs)
     }
 }
 
-static void ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
+static int ssd1680_busy_wait(FAR struct ssd1680_dev_s *priv)
 {
+  int maks_wait_ticks = 200;
   if ((priv->board_priv != NULL) && (priv->board_priv->check_busy != NULL))
     {
-      while (priv->board_priv->check_busy())
+      while (priv->board_priv->check_busy() && maks_wait_ticks > 0)

Review Comment:
   ```suggestion
         while (priv->board_priv->check_busy() && maks_wait_ticks-- > 0)
   ```
   and remove code below.



##########
drivers/lcd/ssd1680.c:
##########
@@ -1269,11 +1299,16 @@ FAR struct lcd_dev_s *ssd1680_initialize(FAR struct spi_dev_s *dev,
   /* Initialize the framebuffer */
 
   memset(priv->shadow_fb, SSD1680_Y1_BLACK & 1 ?
-    0xff : 0x00, SSD1680_DEV_FBSIZE);
+         0xff : 0x00, SSD1680_DEV_FBSIZE);
 
   /* Power on and configure display */
 
-  ssd1680_setpower(&priv->dev, true);
+  ret = ssd1680_setpower(&priv->dev, true);
+

Review Comment:
   Optional
   ```suggestion
   ```



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org