drivers/sensors/sensor: fix poll()/read() for fetch()-only sensors

drivers/sensors/Kconfig

@@ -861,17 +861,19 @@ config SENSORS_L3GD20
 	bool "STMicro L3GD20 Gyroscope Sensor support"
 	default n
 	select SPI
-	select SCHED_HPWORK if SENSORS_L3GD20_BUFFER_SIZE > 0
+	select SCHED_HPWORK
 	---help---
 		Enable driver support for the STMicro L3GD20 gyroscope sensor.
 
 config SENSORS_L3GD20_BUFFER_SIZE
 	int "size of buffer"
 	default 1
+	range 1 32
 	depends on SENSORS_L3GD20
 	---help---
-		The size of the circular buffer used. If the value equal to zero,
-		indicates that the circular buffer is disabled.
+		The number of events that the circular buffer can hold. The data
+		ready interrupt pushes each sample into it, so at least one event
+		is required.
 
 config SENSOR_KXTJ9
 	bool "Kionix KXTJ9 Accelerometer support"

drivers/sensors/l3gd20_uorb.c

@@ -69,12 +69,10 @@ struct l3gd20_dev_s
                                        * L3GD20 sensor */
   uint64_t timestamp;                 /* Units is microseconds */
   struct sensor_lowerhalf_s lower;    /* The struct of lower half driver */
-#if CONFIG_SENSORS_L3GD20_BUFFER_SIZE > 0
   struct work_s work;                 /* The work queue is responsible for
                                        * retrieving the data from the sensor
                                        * after the arrival of new data was
                                        * signalled in an interrupt */
-#endif
 };
 
 /****************************************************************************
@@ -100,13 +98,7 @@ static int l3gd20_interrupt_handler(int irq, FAR void *context,
                                     FAR void *arg);
 static int l3gd20_activate(FAR struct sensor_lowerhalf_s *lower,
                            FAR struct file *filep, bool enable);
-#if CONFIG_SENSORS_L3GD20_BUFFER_SIZE > 0
 static void l3gd20_worker(FAR void *arg);
-#else
-static int l3gd20_fetch(FAR struct sensor_lowerhalf_s *lower,
-                        FAR struct file *filep,
-                        FAR char *buffer, size_t buflen);
-#endif
 
 /****************************************************************************
  * Private Data
@@ -119,11 +111,7 @@ static const struct sensor_ops_s g_l2gd20_ops =
   .activate = l3gd20_activate,
   .set_interval = NULL,
   .batch = NULL,
-#if CONFIG_SENSORS_L3GD20_BUFFER_SIZE > 0
   .fetch = NULL,
-#else
-  .fetch = l3gd20_fetch,
-#endif
   .control = NULL
 };
 
@@ -359,7 +347,6 @@ static int l3gd20_interrupt_handler(int irq, FAR void *context,
 
   priv->timestamp = sensor_get_timestamp();
 
-#if CONFIG_SENSORS_L3GD20_BUFFER_SIZE > 0
   /* Task the worker with retrieving the latest sensor data. We should not do
    * this in a interrupt since it might take too long. Also we cannot lock
    * the SPI bus from within an interrupt.
@@ -373,17 +360,10 @@ static int l3gd20_interrupt_handler(int irq, FAR void *context,
       snerr("ERROR: Failed to queue work: %d\n", ret);
       return ret;
     }
-#else
-
-  /* notify event to upper half driver */
 
-  priv->lower.notify_event(priv->lower.priv);
-
-#endif
   return OK;
 }
 
-#if CONFIG_SENSORS_L3GD20_BUFFER_SIZE > 0
 /****************************************************************************
  * Name: l3gd20_worker
  ****************************************************************************/
@@ -405,33 +385,6 @@ static void l3gd20_worker(FAR void *arg)
                          sizeof(struct sensor_gyro_uncal));
 }
 
-#else
-
-/****************************************************************************
- * Name: l3gd20_fetch
- ****************************************************************************/
-
-static int l3gd20_fetch(FAR struct sensor_lowerhalf_s *lower,
-                        FAR struct file *filep,
-                        FAR char *buffer, size_t buflen)
-{
-  FAR struct l3gd20_dev_s *priv = container_of(lower,
-                                               FAR struct l3gd20_dev_s,
-                                               lower);
-
-  if (buflen != sizeof(struct sensor_gyro_uncal))
-      return 0;
-
-  DEBUGASSERT(priv != NULL);
-
-  /* Read out the latest sensor data */
-
-  l3gd20_read_measurement_data(priv, (FAR struct sensor_gyro_uncal *)buffer);
-
-  return sizeof(struct sensor_gyro_uncal);
-}
-#endif
-
 /****************************************************************************
  * Name: l3gd20_activate
  ****************************************************************************/
@@ -563,9 +516,7 @@ int l3gd20_register(int devno, FAR struct spi_dev_s *spi,
 
   priv->spi              = spi;
   priv->config           = config;
-#if CONFIG_SENSORS_L3GD20_BUFFER_SIZE > 0
   priv->work.worker      = NULL;
-#endif
   priv->timestamp        = 0;
 
   priv->lower.type = SENSOR_TYPE_GYROSCOPE_UNCALIBRATED;

drivers/sensors/sensor.c

@@ -43,6 +43,7 @@
 #include <nuttx/mutex.h>
 #include <nuttx/sensors/sensor.h>
 #include <nuttx/lib/lib.h>
+#include <nuttx/wdog.h>
 
 /****************************************************************************
  * Pre-processor Definitions
@@ -96,12 +97,13 @@ struct sensor_user_s
   struct list_node node;       /* Node of users list */
   struct pollfd   *fds;        /* The poll structure of thread waiting events */
   sensor_role_t    role;       /* The is used to indicate user's role based on open flags */
+  struct wdog_s    wdog;       /* Paces POLLIN at the requested interval */
+  uint64_t         fetched;    /* When POLLIN was last reported, in usec */
   bool             changed;    /* This is used to indicate event happens and need to
                                 * asynchronous notify other users
                                 */
   unsigned int     event;      /* The event of this sensor, eg: SENSOR_EVENT_FLUSH_COMPLETE. */
   bool             flushing;   /* The is used to indicate user is flushing */
-  sem_t            buffersem;  /* Wakeup user waiting for data in circular buffer */
   size_t           bufferpos;  /* The index of user generation in buffer */
 
   /* The subscriber info
@@ -143,6 +145,7 @@ static int     sensor_poll(FAR struct file *filep, FAR struct pollfd *fds,
                            bool setup);
 static ssize_t sensor_push_event(FAR void *priv, FAR const void *data,
                                  size_t bytes);
+static void    sensor_fetch_expired(wdparm_t arg);
 
 /****************************************************************************
  * Private Data
@@ -688,6 +691,23 @@ static void sensor_pollnotify_one(FAR struct sensor_user_s *user,
   poll_notify(&user->fds, 1, eventset);
 }
 
+static void sensor_fetch_expired(wdparm_t arg)
+{
+  FAR struct sensor_user_s *user = (FAR struct sensor_user_s *)arg;
+
+  /* Timer context, so no lock: a teardown that raced us cleared fds, which
+   * makes both the notify and the re-arm below no-ops.
+   */
+
+  if (user->fds != NULL)
+    {
+      user->fetched = sensor_get_timestamp();
+      sensor_pollnotify_one(user, POLLIN, SENSOR_ROLE_RD);
+      wd_start(&user->wdog, USEC2TICK(user->state.interval),
+               sensor_fetch_expired, arg);
+    }
+}
+
 static void sensor_pollnotify(FAR struct sensor_upperhalf_s *upper,
                               pollevent_t eventset, sensor_role_t role)
 {
@@ -774,7 +794,6 @@ static int sensor_open(FAR struct file *filep)
   user->state.interval = UINT32_MAX;
   user->state.esize = upper->state.esize;
   user->state.nonwakeup = true;
-  nxsem_init(&user->buffersem, 0, 0);
   list_add_tail(&upper->userlist, &user->node);
 
   /* The new user generation, notify to other users */
@@ -835,7 +854,6 @@ static int sensor_close(FAR struct file *filep)
     }
 
   list_delete(&user->node);
-  nxsem_destroy(&user->buffersem);
 
   /* The user is closed, notify to other users */
 
@@ -871,24 +889,15 @@ static ssize_t sensor_read(FAR struct file *filep, FAR char *buffer,
           return -EINVAL;
         }
 
-      if (!(filep->f_oflags & O_NONBLOCK))
-        {
-          nxrmutex_unlock(&upper->lock);
-          ret = nxsem_wait_uninterruptible(&user->buffersem);
-          if (ret < 0)
-            {
-              return ret;
-            }
+      /* Read the device directly, there is nothing to wait for */
 
-          nxrmutex_lock(&upper->lock);
-        }
-      else if (!upper->state.nsubscribers)
+      if (!upper->state.nsubscribers)
         {
           ret = -EAGAIN;
           goto out;
         }
 
-        ret = lower->ops->fetch(lower, filep, buffer, len);
+      ret = lower->ops->fetch(lower, filep, buffer, len);
     }
   else if (circbuf_is_empty(&upper->buffer))
     {
@@ -1166,7 +1175,6 @@ static int sensor_poll(FAR struct file *filep,
   FAR struct sensor_lowerhalf_s *lower = upper->lower;
   FAR struct sensor_user_s *user = filep->f_priv;
   pollevent_t eventset = 0;
-  int semcount;
   int ret = 0;
 
   nxrmutex_lock(&upper->lock);
@@ -1184,19 +1192,30 @@ static int sensor_poll(FAR struct file *filep,
       fds->priv = filep;
       if (lower->ops->fetch)
         {
-          /* Always return POLLIN for fetch data directly(non-block) */
+          /* Always ready, unless a rate was requested: then once per
+           * interval, woken by sensor_fetch_expired().
+           */
 
-          if (filep->f_oflags & O_NONBLOCK)
+          if (user->state.interval == UINT32_MAX)
             {
               eventset |= POLLIN;
             }
           else
             {
-              nxsem_get_value(&user->buffersem, &semcount);
-              if (semcount > 0)
+              uint64_t now = sensor_get_timestamp();
+              uint64_t elapsed = now - user->fetched;
+
+              if (elapsed >= user->state.interval)
                 {
+                  user->fetched = now;
                   eventset |= POLLIN;
                 }
+              else
+                {
+                  wd_start(&user->wdog,
+                           USEC2TICK(user->state.interval - elapsed),
+                           sensor_fetch_expired, (wdparm_t)user);
+                }
             }
         }
       else if (sensor_is_updated(upper, user))
@@ -1215,6 +1234,7 @@ static int sensor_poll(FAR struct file *filep,
     {
       user->fds = NULL;
       fds->priv = NULL;
+      wd_cancel(&user->wdog);
     }
 
 errout:
@@ -1229,7 +1249,6 @@ static ssize_t sensor_push_event(FAR void *priv, FAR const void *data,
   FAR struct sensor_lowerhalf_s *lower = upper->lower;
   FAR struct sensor_user_s *user;
   unsigned long envcount;
-  int semcount;
   int ret;
 
   nxrmutex_lock(&upper->lock);
@@ -1287,12 +1306,6 @@ static ssize_t sensor_push_event(FAR void *priv, FAR const void *data,
     {
       if (sensor_is_updated(upper, user))
         {
-          nxsem_get_value(&user->buffersem, &semcount);
-          if (semcount < 1)
-            {
-              nxsem_post(&user->buffersem);
-            }
-
           sensor_pollnotify_one(user, POLLIN, SENSOR_ROLE_RD);
         }
     }
@@ -1305,17 +1318,10 @@ static void sensor_notify_event(FAR void *priv)
 {
   FAR struct sensor_upperhalf_s *upper = priv;
   FAR struct sensor_user_s *user;
-  int semcount;
 
   nxrmutex_lock(&upper->lock);
   list_for_every_entry(&upper->userlist, user, struct sensor_user_s, node)
     {
-      nxsem_get_value(&user->buffersem, &semcount);
-      if (semcount < 1)
-        {
-          nxsem_post(&user->buffersem);
-        }
-
       sensor_pollnotify_one(user, POLLIN, SENSOR_ROLE_RD);
     }
 

include/nuttx/sensors/sensor.h

@@ -353,10 +353,10 @@ struct sensor_ops_s
    * If fetch isn't NULL, upper half driver will disable intermediate
    * buffer and userspace can't set buffer size by ioctl.
    *
-   * You can call this function to read sensor register data by I2C/SPI bus
-   * when open mode is non-block, and poll are always successful.
-   * When you call this function and open mode is block, you will wait
-   * until sensor data ready, then read sensor data.
+   * You can call this function to read sensor register data by I2C/SPI
+   * bus. The data is read from the device on demand, so it is always
+   * available: poll() always reports POLLIN and read() never blocks,
+   * whether or not the open mode is non-block.
    *
    * Input Parameters:
    *   lower      - The instance of lower half sensor driver.