Fishwaldo commented on code in PR #19878:
URL: https://github.com/apache/nuttx/pull/19878#discussion_r3801030354


##########
drivers/ioexpander/gpio.c:
##########
@@ -75,10 +125,391 @@ static const struct file_operations g_gpio_drvrops =
   gpio_poll,   /* poll */
 };
 
+#ifdef CONFIG_GPIO_PROCFS
+
+static struct list_node g_gpio_list = LIST_INITIAL_VALUE(g_gpio_list);
+static mutex_t g_gpio_lock = NXMUTEX_INITIALIZER;
+static bool g_gpio_procfs_added;
+
+/* Indexed by enum gpio_pintype_e.  The enum's own comment warns that a
+ * table like this has to be extended with it; the assertion below turns
+ * forgetting into a build error rather than a pin type with no name.
+ */
+
+static const FAR char *g_gpio_typename[] =
+{
+  "INPUT",            /* GPIO_INPUT_PIN                  */
+  "INPUT_PU",         /* GPIO_INPUT_PIN_PULLUP           */
+  "INPUT_PD",         /* GPIO_INPUT_PIN_PULLDOWN         */
+  "OUTPUT",           /* GPIO_OUTPUT_PIN                 */
+  "OUTPUT_OD",        /* GPIO_OUTPUT_PIN_OPENDRAIN       */
+  "INT",              /* GPIO_INTERRUPT_PIN              */
+  "INT_HIGH",         /* GPIO_INTERRUPT_HIGH_PIN         */
+  "INT_LOW",          /* GPIO_INTERRUPT_LOW_PIN          */
+  "INT_RISING",       /* GPIO_INTERRUPT_RISING_PIN       */
+  "INT_FALLING",      /* GPIO_INTERRUPT_FALLING_PIN      */
+  "INT_BOTH",         /* GPIO_INTERRUPT_BOTH_PIN         */
+  "INT_WAKE",         /* GPIO_INTERRUPT_PIN_WAKEUP       */
+  "INT_HIGH_WAKE",    /* GPIO_INTERRUPT_HIGH_PIN_WAKEUP  */
+  "INT_LOW_WAKE",     /* GPIO_INTERRUPT_LOW_PIN_WAKEUP   */
+  "INT_RISING_WAKE",  /* GPIO_INTERRUPT_RISING_PIN_WAKEUP */
+  "INT_FALLING_WAKE", /* GPIO_INTERRUPT_FALLING_PIN_WAKEUP */
+  "INT_BOTH_WAKE",    /* GPIO_INTERRUPT_BOTH_PIN_WAKEUP  */
+};
+
+static_assert(nitems(g_gpio_typename) == GPIO_NPINTYPES,
+              "pin type name table does not match enum gpio_pintype_e");
+
+static const struct procfs_operations g_gpio_procfs_ops =
+{
+  gpio_procfs_open,   /* open */
+  gpio_procfs_close,  /* close */
+  gpio_procfs_read,   /* read */
+  NULL,               /* write */
+  NULL,               /* poll */
+
+  gpio_procfs_dup,    /* dup */
+
+  NULL,               /* opendir */
+  NULL,               /* closedir */
+  NULL,               /* readdir */
+  NULL,               /* rewinddir */
+
+  gpio_procfs_stat,   /* stat */
+};
+
+static const struct procfs_entry_s g_gpio_procfs =
+{
+  "gpio", &g_gpio_procfs_ops, PROCFS_FILE_TYPE
+};
+
+#endif /* CONFIG_GPIO_PROCFS */
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
+#ifdef CONFIG_GPIO_PROCFS
+
+/****************************************************************************
+ * Name: gpio_procfs_open
+ *
+ * Description:
+ *   Open /proc/gpio.  The entry is read only, and holds no state of its
+ *   own beyond the position accounting procfs does for every file.
+ *
+ * Input Parameters:
+ *   filep   - The file structure to attach the open file to
+ *   relpath - The path below /proc being opened
+ *   oflags  - Open flags; anything but read only is refused
+ *   mode    - Ignored, the entry cannot be created
+ *
+ * Returned Value:
+ *   Zero on success, or a negated errno on failure.
+ *
+ ****************************************************************************/
+
+static int gpio_procfs_open(FAR struct file *filep, FAR const char *relpath,
+                            int oflags, mode_t mode)
+{
+  FAR struct procfs_file_s *priv;
+
+  if ((oflags & O_ACCMODE) != O_RDONLY)
+    {
+      return -EACCES;
+    }
+
+  priv = kmm_zalloc(sizeof(struct procfs_file_s));
+  if (priv == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  filep->f_priv = priv;
+  return OK;
+}
+
+/****************************************************************************
+ * Name: gpio_procfs_close
+ *
+ * Description:
+ *   Close /proc/gpio and free what open() allocated.
+ *
+ * Input Parameters:
+ *   filep - The open file
+ *
+ * Returned Value:
+ *   Zero on success, or a negated errno on failure.
+ *
+ ****************************************************************************/
+
+static int gpio_procfs_close(FAR struct file *filep)
+{
+  kmm_free(filep->f_priv);
+  filep->f_priv = NULL;
+  return OK;
+}
+
+/****************************************************************************
+ * Name: gpio_procfs_read
+ *
+ * Description:
+ *   List every registered pin, in registration order, with what the upper
+ *   half knows about it.  A lower half that supplies go_describe adds its
+ *   own fields to the same line.
+ *
+ *   Each read renders from the start and skips what earlier reads already
+ *   returned, so a file longer than the caller's buffer still comes out
+ *   whole across successive reads.
+ *
+ * Input Parameters:
+ *   filep  - The open file, carrying the offset reached so far
+ *   buffer - Where to return the text
+ *   buflen - Size of buffer
+ *
+ * Returned Value:
+ *   The number of bytes returned, zero at end of file, or a negated errno
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static ssize_t gpio_procfs_read(FAR struct file *filep, FAR char *buffer,
+                                size_t buflen)
+{
+  FAR struct gpio_entry_s *entry;
+  size_t remaining = buflen;
+  FAR char *dest = buffer;
+  off_t pos = filep->f_pos;
+  char extra[48];
+  char line[128];
+  bool value;
+  size_t n;
+  int ret;
+
+  ret = nxmutex_lock(&g_gpio_lock);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  list_for_every_entry(&g_gpio_list, entry, struct gpio_entry_s, node)
+    {
+      FAR const char *type = "-";
+
+      if (remaining == 0)
+        {
+          break;
+        }
+
+      /* A lower half is free to invent a pin type this table has never
+       * heard of, so the index is bounded here rather than trusted.
+       */
+
+      if (entry->dev->gp_pintype < nitems(g_gpio_typename))
+        {
+          type = g_gpio_typename[entry->dev->gp_pintype];
+        }
+
+      /* A pin that cannot be read reports -, so that a failed read is
+       * not shown as a low level.
+       */
+
+      if (entry->dev->gp_ops->go_read != NULL &&
+          entry->dev->gp_ops->go_read(entry->dev, &value) >= 0)
+        {
+          n = snprintf(line, sizeof(line), "%-12s type:%-18s val:%u",
+                       entry->name, type, (unsigned int)value);

Review Comment:
   Fixed



##########
drivers/ioexpander/gpio.c:
##########
@@ -75,10 +125,391 @@ static const struct file_operations g_gpio_drvrops =
   gpio_poll,   /* poll */
 };
 
+#ifdef CONFIG_GPIO_PROCFS
+
+static struct list_node g_gpio_list = LIST_INITIAL_VALUE(g_gpio_list);
+static mutex_t g_gpio_lock = NXMUTEX_INITIALIZER;
+static bool g_gpio_procfs_added;
+
+/* Indexed by enum gpio_pintype_e.  The enum's own comment warns that a
+ * table like this has to be extended with it; the assertion below turns
+ * forgetting into a build error rather than a pin type with no name.
+ */
+
+static const FAR char *g_gpio_typename[] =
+{
+  "INPUT",            /* GPIO_INPUT_PIN                  */
+  "INPUT_PU",         /* GPIO_INPUT_PIN_PULLUP           */
+  "INPUT_PD",         /* GPIO_INPUT_PIN_PULLDOWN         */
+  "OUTPUT",           /* GPIO_OUTPUT_PIN                 */
+  "OUTPUT_OD",        /* GPIO_OUTPUT_PIN_OPENDRAIN       */
+  "INT",              /* GPIO_INTERRUPT_PIN              */
+  "INT_HIGH",         /* GPIO_INTERRUPT_HIGH_PIN         */
+  "INT_LOW",          /* GPIO_INTERRUPT_LOW_PIN          */
+  "INT_RISING",       /* GPIO_INTERRUPT_RISING_PIN       */
+  "INT_FALLING",      /* GPIO_INTERRUPT_FALLING_PIN      */
+  "INT_BOTH",         /* GPIO_INTERRUPT_BOTH_PIN         */
+  "INT_WAKE",         /* GPIO_INTERRUPT_PIN_WAKEUP       */
+  "INT_HIGH_WAKE",    /* GPIO_INTERRUPT_HIGH_PIN_WAKEUP  */
+  "INT_LOW_WAKE",     /* GPIO_INTERRUPT_LOW_PIN_WAKEUP   */
+  "INT_RISING_WAKE",  /* GPIO_INTERRUPT_RISING_PIN_WAKEUP */
+  "INT_FALLING_WAKE", /* GPIO_INTERRUPT_FALLING_PIN_WAKEUP */
+  "INT_BOTH_WAKE",    /* GPIO_INTERRUPT_BOTH_PIN_WAKEUP  */
+};
+
+static_assert(nitems(g_gpio_typename) == GPIO_NPINTYPES,
+              "pin type name table does not match enum gpio_pintype_e");
+
+static const struct procfs_operations g_gpio_procfs_ops =
+{
+  gpio_procfs_open,   /* open */
+  gpio_procfs_close,  /* close */
+  gpio_procfs_read,   /* read */
+  NULL,               /* write */
+  NULL,               /* poll */
+
+  gpio_procfs_dup,    /* dup */
+
+  NULL,               /* opendir */
+  NULL,               /* closedir */
+  NULL,               /* readdir */
+  NULL,               /* rewinddir */
+
+  gpio_procfs_stat,   /* stat */
+};
+
+static const struct procfs_entry_s g_gpio_procfs =
+{
+  "gpio", &g_gpio_procfs_ops, PROCFS_FILE_TYPE
+};
+
+#endif /* CONFIG_GPIO_PROCFS */
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
+#ifdef CONFIG_GPIO_PROCFS
+
+/****************************************************************************
+ * Name: gpio_procfs_open
+ *
+ * Description:
+ *   Open /proc/gpio.  The entry is read only, and holds no state of its
+ *   own beyond the position accounting procfs does for every file.
+ *
+ * Input Parameters:
+ *   filep   - The file structure to attach the open file to
+ *   relpath - The path below /proc being opened
+ *   oflags  - Open flags; anything but read only is refused
+ *   mode    - Ignored, the entry cannot be created
+ *
+ * Returned Value:
+ *   Zero on success, or a negated errno on failure.
+ *
+ ****************************************************************************/
+
+static int gpio_procfs_open(FAR struct file *filep, FAR const char *relpath,
+                            int oflags, mode_t mode)
+{
+  FAR struct procfs_file_s *priv;
+
+  if ((oflags & O_ACCMODE) != O_RDONLY)
+    {
+      return -EACCES;
+    }
+
+  priv = kmm_zalloc(sizeof(struct procfs_file_s));
+  if (priv == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  filep->f_priv = priv;
+  return OK;
+}
+
+/****************************************************************************
+ * Name: gpio_procfs_close
+ *
+ * Description:
+ *   Close /proc/gpio and free what open() allocated.
+ *
+ * Input Parameters:
+ *   filep - The open file
+ *
+ * Returned Value:
+ *   Zero on success, or a negated errno on failure.
+ *
+ ****************************************************************************/
+
+static int gpio_procfs_close(FAR struct file *filep)
+{
+  kmm_free(filep->f_priv);
+  filep->f_priv = NULL;
+  return OK;
+}
+
+/****************************************************************************
+ * Name: gpio_procfs_read
+ *
+ * Description:
+ *   List every registered pin, in registration order, with what the upper
+ *   half knows about it.  A lower half that supplies go_describe adds its
+ *   own fields to the same line.
+ *
+ *   Each read renders from the start and skips what earlier reads already
+ *   returned, so a file longer than the caller's buffer still comes out
+ *   whole across successive reads.
+ *
+ * Input Parameters:
+ *   filep  - The open file, carrying the offset reached so far
+ *   buffer - Where to return the text
+ *   buflen - Size of buffer
+ *
+ * Returned Value:
+ *   The number of bytes returned, zero at end of file, or a negated errno
+ *   on failure.
+ *
+ ****************************************************************************/
+
+static ssize_t gpio_procfs_read(FAR struct file *filep, FAR char *buffer,
+                                size_t buflen)
+{
+  FAR struct gpio_entry_s *entry;
+  size_t remaining = buflen;
+  FAR char *dest = buffer;
+  off_t pos = filep->f_pos;
+  char extra[48];
+  char line[128];
+  bool value;
+  size_t n;
+  int ret;
+
+  ret = nxmutex_lock(&g_gpio_lock);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  list_for_every_entry(&g_gpio_list, entry, struct gpio_entry_s, node)
+    {
+      FAR const char *type = "-";
+
+      if (remaining == 0)
+        {
+          break;
+        }
+
+      /* A lower half is free to invent a pin type this table has never
+       * heard of, so the index is bounded here rather than trusted.
+       */
+
+      if (entry->dev->gp_pintype < nitems(g_gpio_typename))
+        {
+          type = g_gpio_typename[entry->dev->gp_pintype];
+        }
+
+      /* A pin that cannot be read reports -, so that a failed read is
+       * not shown as a low level.
+       */
+
+      if (entry->dev->gp_ops->go_read != NULL &&
+          entry->dev->gp_ops->go_read(entry->dev, &value) >= 0)
+        {
+          n = snprintf(line, sizeof(line), "%-12s type:%-18s val:%u",
+                       entry->name, type, (unsigned int)value);
+        }
+      else
+        {
+          n = snprintf(line, sizeof(line), "%-12s type:%-18s val:-",
+                       entry->name, type);
+        }
+
+      n += snprintf(line + n, sizeof(line) - n,
+                    " regs:%u ints:%lu",
+                    (unsigned int)entry->dev->register_count,
+                    (unsigned long)entry->dev->int_count);

Review Comment:
   Fixed



-- 
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