Merged in ziggurat29/nuttx/stm32l4_i2c_lcd_mjkdz_001 (pull request #35)

correct bugs and add enhancements to pcf8574 lcd backpack driver

drivers/lcd/pcf8574_lcd_backpack.c

@@ -63,9 +63,9 @@
 
 /* timing characteristics of the LCD interface */
 
-#define DELAY_US_NYBBLE0    200
+#define DELAY_US_NYBBLE0    20
-#define DELAY_US_NYBBLE1    100
+#define DELAY_US_NYBBLE1    10
-#define DELAY_US_WRITE      35
+#define DELAY_US_WRITE      40
 #define DELAY_US_HOMECLEAR  1500
 
 /* HD44780 commands */
@@ -86,6 +86,8 @@
 #  define lcdvdbg(x...)
 #endif
 
+#define MAX_OPENCNT     (255)                  /* Limit of uint8_t */
+
 /****************************************************************************
  * Private Types
  ****************************************************************************/
@@ -95,6 +97,8 @@ struct pcf8574_lcd_dev_s
   FAR struct i2c_master_s *i2c;               /* I2C interface */
   struct pcf8574_lcd_backpack_config_s cfg;   /* gpio configuration */
   uint8_t bl_bit;                             /* current backlight bit */
+  uint8_t refs;                               /* Number of references */
+  uint8_t unlinked;                           /* We are unlinked, so teardown on last close */
   sem_t sem_excl;                             /* mutex */
 };
 
@@ -117,12 +121,16 @@ static ssize_t pcf8574_lcd_read(FAR struct file *filep, FAR char *buffer,
                                 size_t buflen);
 static ssize_t pcf8574_lcd_write(FAR struct file *filep,
                                  FAR const char *buffer, size_t buflen);
+static off_t pcf8574_lcd_seek(FAR struct file *filep, off_t offset, int whence);
 static int pcf8574_lcd_ioctl(FAR struct file *filep, int cmd,
                              unsigned long arg);
 #ifndef CONFIG_DISABLE_POLL
 static int pcf8574lcd_poll(FAR struct file *filep, FAR struct pollfd *fds,
                            bool setup);
 #endif
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int pcf8574_lcd_unlink(FAR struct inode *inode);
+#endif
 
 /****************************************************************************
  * Private Data
@@ -134,12 +142,14 @@ static const struct file_operations g_pcf8574_lcd_fops =
   pcf8574_lcd_close,           /* close */
   pcf8574_lcd_read,            /* read */
   pcf8574_lcd_write,           /* write */
-  0,                           /* seek */
+  pcf8574_lcd_seek,            /* seek */
   pcf8574_lcd_ioctl,           /* ioctl */
 #ifndef CONFIG_DISABLE_POLL
   pcf8574lcd_poll,             /* poll */
 #endif
-  0                            /* unlink */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  pcf8574_lcd_unlink           /* unlink */
+#endif
 };
 
 /****************************************************************************
@@ -259,7 +269,7 @@ static inline uint8_t rc2addr(FAR struct pcf8574_lcd_dev_s *priv,
        * of first line, and fourth line is a continuation of second.
        */
 
-      return (row - 2) * 0x40 + (col - priv->cfg.cols);
+      return (row - 2) * 0x40 + (col + priv->cfg.cols);
     }
 }
 
@@ -383,9 +393,14 @@ static void latch_nybble(FAR struct pcf8574_lcd_dev_s *priv, uint8_t nybble,
   en_bit = 1 << priv->cfg.en;
   rs_bit = rs ? (1 << priv->cfg.rs) : 0;
 
-  /* Put the nybble, preserving backlight, reset R/~W and set EN and maybe RS */
+  /* Put the nybble, preserving backlight, reset R/~W and maybe RS */
 
-  lcddata = prepare_nybble(priv, nybble) | priv->bl_bit | en_bit | rs_bit;
+  lcddata = prepare_nybble(priv, nybble) | priv->bl_bit | rs_bit;
+  pca8574_write(priv, lcddata);
+
+  /* Now set EN */
+
+  lcddata |= en_bit;
   pca8574_write(priv, lcddata);
   up_udelay(DELAY_US_NYBBLE0);  /* setup */
 
@@ -419,9 +434,14 @@ static uint8_t load_nybble(FAR struct pcf8574_lcd_dev_s *priv, bool rs)
   rs_bit = rs ? (1 << priv->cfg.rs) : 0;
   rw_bit = 1 << priv->cfg.rw;
 
-  /* Put highs on the data lines, preserve, set R/~W and set EN and maybe RS */
+  /* Put highs on the data lines, preserve, set R/~W and maybe RS */
 
-  lcddata = prepare_nybble(priv, 0x0f) | priv->bl_bit | en_bit | rw_bit | rs_bit;
+  lcddata = prepare_nybble(priv, 0x0f) | priv->bl_bit | rw_bit | rs_bit;
+  pca8574_write(priv, lcddata);
+
+  /* Now set EN */
+
+  lcddata |= en_bit;
   pca8574_write(priv, lcddata);
   up_udelay(DELAY_US_NYBBLE0);  /* setup */
 
@@ -538,7 +558,7 @@ static void lcd_init(FAR struct pcf8574_lcd_dev_s *priv)
 {
   /* Wait for more than 15 ms after Vcc for the LCD to stabilize */
 
-  usleep(20000);
+  usleep(50000);
 
   /* Perform the init sequence.  This sequence of commands is constructed so
    * that it will get the device into nybble mode irrespective of what state
@@ -548,7 +568,7 @@ static void lcd_init(FAR struct pcf8574_lcd_dev_s *priv)
    * the remainder of operations.
    */
 
-  /* Send Command 0x30, set 8-bit mode, and wait > 4.1 ms*/
+  /* Send Command 0x30, set 8-bit mode, and wait > 4.1 ms */
 
   latch_nybble(priv, 0x30>>4, false);
   usleep(5000);
@@ -556,15 +576,17 @@ static void lcd_init(FAR struct pcf8574_lcd_dev_s *priv)
   /* Send Command 0x30, set 8-bit mode, and wait > 100 us */
 
   latch_nybble(priv, 0x30>>4, false);
-  usleep(200);
+  usleep(5000);
 
   /* Send Command 0x30, set 8-bit mode */
 
   latch_nybble(priv, 0x30>>4, false);
+  usleep(200);
 
   /* now Function set: Set interface to be 4 bits long (only 1 cycle write for the first time). */
 
   latch_nybble(priv, 0x20>>4, false);
+  usleep(5000);
 
   /* Function set: DL=0;Interface is 4 bits, N=1 (2 Lines), F=0 (5x8 dots font) */
 
@@ -988,6 +1010,53 @@ static int lcd_getstream(FAR struct lib_instream_s *instream)
   return EOF;
 }
 
+/****************************************************************************
+ * Name: lcd_fpos_to_curpos
+ *
+ * Description:
+ *   Convert a file logical offset to a screen cursor pos (row,col).  This
+ *   discounts 'synthesized' line feeds at the end of screen lines.
+ *
+ ****************************************************************************/
+
+static void lcd_fpos_to_curpos(FAR struct pcf8574_lcd_dev_s *priv,
+                              off_t fpos, uint8_t *row, uint8_t *col, bool* onlf)
+{
+  int virtcols;
+  
+  virtcols = (priv->cfg.cols + 1);
+  
+  /* Determine if this is a 'virtual' position (on the synthetic LF) */
+
+  *onlf = (priv->cfg.cols == fpos % virtcols);
+
+  /* Adjust off any preceding synthetic LF's to get linear position */
+  
+  fpos -= fpos / virtcols;
+  
+  /* Compute row/col from linear position */
+  
+  *row = fpos / priv->cfg.cols;
+  *col = fpos % priv->cfg.cols;
+}
+
+/****************************************************************************
+ * Name: lcd_curpos_to_fpos
+ *
+ * Description:
+ *   Convert a screen cursor pos (row,col) to a file logical offset.  This
+ *   includes 'synthesized' line feeds at the end of screen lines.
+ *
+ ****************************************************************************/
+
+static void lcd_curpos_to_fpos(FAR struct pcf8574_lcd_dev_s *priv,
+                              uint8_t row, uint8_t col, off_t* fpos)
+{
+  /* the logical file position is the linear position plus any synthetic LF */
+  
+  *fpos = (row * priv->cfg.cols) + col + row;
+}
+
 /****************************************************************************
  * Name: pcf8574_lcd_open
  *
@@ -998,6 +1067,22 @@ static int lcd_getstream(FAR struct lib_instream_s *instream)
 
 static int pcf8574_lcd_open(FAR struct file *filep)
 {
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct pcf8574_lcd_dev_s *priv = (FAR struct pcf8574_lcd_dev_s *)inode->i_private;
+
+  /* Increment the reference count */
+
+  sem_wait(&priv->sem_excl);
+  if (priv->refs == MAX_OPENCNT)
+    {
+      return -EMFILE;
+    }
+  else
+    {
+      priv->refs++;
+    }
+
+  sem_post(&priv->sem_excl);
   return OK;
 }
 
@@ -1011,7 +1096,35 @@ static int pcf8574_lcd_open(FAR struct file *filep)
 
 static int pcf8574_lcd_close(FAR struct file *filep)
 {
-  return OK;
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct pcf8574_lcd_dev_s *priv = (FAR struct pcf8574_lcd_dev_s *)inode->i_private;
+  int ret;
+
+  /* Decrement the reference count */
+
+  sem_wait(&priv->sem_excl);
+
+  if (priv->refs == 0)
+    {
+      ret = -EIO;
+    }
+  else
+    {
+      priv->refs--;
+
+      /* If we had previously unlinked, but there were open references at the
+       * time, we need to do the final teardown now.
+       */
+      
+      if (priv->refs == 0 && priv->unlinked)
+        {
+          /* We have no real teardown at present */
+        }
+      ret = OK;
+    }
+
+  sem_post(&priv->sem_excl);
+  return ret;
 }
 
 /****************************************************************************
@@ -1019,7 +1132,8 @@ static int pcf8574_lcd_close(FAR struct file *filep)
  *
  * Description:
  *  This simply reads as much of the display memory as possible.  This is
- *  probably not very interesting.
+ *  generally not very interesting, but we do it in a way that allows us to
+ *  'cat' the LCD contents via the shell.
  *
  ****************************************************************************/
 
@@ -1032,22 +1146,44 @@ static ssize_t pcf8574_lcd_read(FAR struct file *filep, FAR char *buffer,
   uint8_t addr;
   uint8_t row;
   uint8_t col;
+  bool onlf;
 
   sem_wait(&priv->sem_excl);
 
   /* Get current cursor position so we can restore it */
 
   (void)lcd_read_busy_addr(priv, &addr);
-  addr2rc(priv, addr, &row, &col);
 
-  /* Just read the entire display into the given buffer, as much as possible */
+  /* Convert file position to row/col address and position DDADDR there */
+
+  lcd_fpos_to_curpos(priv, filep->f_pos, &row, &col, &onlf);
+  lcd_set_curpos(priv, row, col);
+
+  /* Read as much of the display as possible */
 
   nIdx = 0;
-  row = 0;
-  col = 0;
-
   while (nIdx < buflen && row < priv->cfg.rows)
   {
+    /* Synthesize end-of-line LF and advance to start of next row */
+    
+    if (onlf)
+      {
+        /* Synthesize LF for all but last row */
+        
+        if ( row < priv->cfg.rows-1)
+          {
+            buffer[nIdx] = '\x0a';
+            onlf = false;
+            ++filep->f_pos;
+            ++nIdx;
+          }
+        ++row;
+        col = 0;
+        continue;
+      }
+
+    /* If we are at start of line we will need to update DDRAM address */
+
     if (0 == col)
       {
         lcd_set_curpos(priv, row, 0);
@@ -1055,12 +1191,15 @@ static ssize_t pcf8574_lcd_read(FAR struct file *filep, FAR char *buffer,
 
     buffer[nIdx] = lcd_getdata(priv);
 
+    ++filep->f_pos;
     ++nIdx;
     ++col;
+
+    /* If we are now at the end of a line, we setup for the synthetic LF */
+
     if (priv->cfg.cols == col)
       {
-        ++row;
+        onlf = true;
-        col = 0;
       }
   }
 
@@ -1214,10 +1353,62 @@ static ssize_t pcf8574_lcd_write(FAR struct file *filep,
         }
     }
 
+  /* Wherever we wound up, update our logical file pos to reflect it */
+
+  lcd_curpos_to_fpos(priv, row, col, &filep->f_pos);
+
   sem_post(&priv->sem_excl);
   return buflen;
 }
 
+/****************************************************************************
+ * Name: pcf8574_lcd_seek
+ *
+ * Description:
+ *   Seek the logical file pointer to the specified position.  This is
+ *   probably not very interesting except possibly for (SEEK_SET, 0) to
+ *   rewind the pointer for a subsequent read().
+ *   The file pointer is logical, and includes synthesized LF chars at the
+ *   end of the display lines.
+ *
+ ****************************************************************************/
+
+static off_t pcf8574_lcd_seek(FAR struct file *filep, off_t offset, int whence)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct pcf8574_lcd_dev_s *priv = (FAR struct pcf8574_lcd_dev_s *)inode->i_private;
+  int maxpos;
+
+  sem_wait(&priv->sem_excl);
+
+  maxpos = priv->cfg.rows * priv->cfg.cols + (priv->cfg.rows - 1);
+  switch (whence)
+    {
+    case SEEK_CUR:
+      filep->f_pos += offset;
+      if (filep->f_pos > maxpos)
+        filep->f_pos = maxpos;
+      break;
+
+    case SEEK_SET:
+      filep->f_pos = offset;
+      if (filep->f_pos > maxpos)
+        filep->f_pos = maxpos;
+      break;
+
+    case SEEK_END:
+      filep->f_pos = maxpos;
+      break;
+
+    default:
+      /* Return EINVAL if the whence argument is invalid */
+      filep->f_pos = -EINVAL;
+    }
+
+  sem_post(&priv->sem_excl);
+  return filep->f_pos;
+}
+
 /****************************************************************************
  * Name: pcf8574_lcd_ioctl
  *
@@ -1337,6 +1528,32 @@ static int pcf8574lcd_poll(FAR struct file *filep, FAR struct pollfd *fds,
 }
 #endif
 
+/****************************************************************************
+ * Name: pcf8574_lcd_unlink
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int pcf8574_lcd_unlink(FAR struct inode *inode)
+{
+  FAR struct pcf8574_lcd_dev_s *priv = (FAR struct pcf8574_lcd_dev_s *)inode->i_private;
+  int ret = OK;
+
+  sem_wait(&priv->sem_excl);
+
+  priv->unlinked = true;
+
+  /* If there are no open references to the driver then tear it down now */
+  if (priv->refs == 0)
+    {
+      /* We have no real teardown at present */
+      ret = OK;
+    }
+
+  sem_post(&priv->sem_excl);
+  return ret;
+}
+#endif
+
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -1383,6 +1600,8 @@ int pcf8574_lcd_backpack_register(FAR const char *devpath,
   priv->i2c = i2c;
   priv->cfg = *cfg;
   priv->bl_bit = priv->cfg.bl_active_high ? 0 : (1 << priv->cfg.bl);
+  priv->refs = 0;
+  priv->unlinked = false;
   sem_init(&priv->sem_excl, 0, 1);
 
   /* Initialize */

drivers/lcd/pcf8574_lcd_backpack_readme.txt

@@ -225,7 +225,7 @@ Example of programming a character image:
 Now character '\x04' will display as an 'up arrow'.
 
 Note, you might consider avoiding the use of code point 0x00 unless
-absolutely needed, because the embedded nul character can cause
+you absolutely need it, because the embedded nul character can cause
 problems.  The driver, and write() apis are binary, and unaffected,
 but things like printf() and puts() assume C-style strings, and are
 affected.
@@ -237,9 +237,16 @@ Troubleshooting
   bus timeouts that suggest a non-responsive slave.
 * Check your board wiring and configuration specification.  Buzz
   out the lines if you have to.
-* Con't forget to check the 'contrast' potentiometer.  The voltage
+* Remember to set the (ros,cols) geometry in pcf8574_lcd_backpack_config_s
-  at the central wiper should be approximately 0.29 V.  The useful
+  before registration of the driver, since this cannot be determined
-  range of voltages at this pin is very narrow, and outside that
+  programmatically.
-  range there will be nothing visible on the display, so most of the
+* If the driver registration step seems to 'hang' it could be the I2C
-  turn range of the pot is non-useful.  Much of human life has been
+  driver performing retries due to no response from the LCD backpack.  Check
-  wasted in the rediscovery of this farcically idiotic
+  the address.  Turning on debug output for I2C can help make this visible.
+* Don't forget to check the 'contrast' potentiometer.  The voltage at the
+  central wiper should be approximately 0.3 V - 2.4 V, but the actual value
+  is is dependent on the physics of the attached LCD module.  The useful
+  range of voltages at this pin for any given LCD is quite narrow, and
+  outside that range there will be nothing visible on the display, so most
+  of the turn range of the pot is non-useful.  It's less 'contrast' and
+  more 'LCD segment drive bias'.

include/nuttx/lcd/pcf8574_lcd_backpack.h

@@ -58,12 +58,15 @@
  * Pretty much anything on the market except 4x40, which really consists of two
  * separate 2x40 controllers, and the I2C backpack doesn't support those due
  * to the second 'E' line being needed.
+ * Additionally, you still need to set the (row,col) geometry explicitly, since
+ * there is not a means of determining this dynamically.
  * Consider these 'informative'.
- * XXX Note, actual testing has been done on LCD_I2C_BACKPACK_CFG_MJKDZ only,
+ * XXX Note, actual testing has been done on LCD_I2C_BACKPACK_CFG_MJKDZ
- * the others come from online research.
+ * and LCD_I2C_BACKPACK_CFG_SAINSMART only, the others come from online
+ * research.
  */
 
-/* board marked 'mjkdz' */
+/* board marked 'mjkdz' and Arduino-IIC-LCD GY-LCD-V1 */
 
 #define LCD_I2C_BACKPACK_CFG_MJKDZ {0x20,4,5,6,0,1,2,3,7,false,0,0}