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/10/23 08:54:19 UTC

[GitHub] [incubator-nuttx] donghengqaz opened a new pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

donghengqaz opened a new pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074


   ## Summary
   
   Add real-time timer support for WiFi, and remove "disable modem sleep" function.
   
   ## Impact
   
   1. time accuracy is microsecond
   2. do callback in high priority task
   
   ## Testing
   
   Use WAPI exmaple and "ping" command to test.
   


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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074#discussion_r510825562



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -665,4 +671,21 @@ config ESP32_WIFI_FS_MOUNTPT
 
 endmenu # ESP32_WIRELESS
 
+menu "Real-Time Timer"
+	depends on ESP32_RT_TIMER
+
+config ESP32_RT_TIMER_TASK_NAME
+	string "Timer task name"
+	default "rt_timer"
+
+config ESP32_RT_TIMER_TASK_PRIORITY
+	int "Timer task priority"
+	default 10

Review comment:
       Yes, it is not suitable. I see the high priority workqueue's thread priority is 224 default, far away from max value 255. Is there document introduce exiting task's priority in NuttX.




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



[GitHub] [incubator-nuttx] saramonteiro commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
saramonteiro commented on a change in pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074#discussion_r511070304



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -259,6 +263,8 @@ config ESP32_WIRELESS
 	select NET
 	select ARCH_PHY_INTERRUPT
 	select ESP32_RNG
+	select ESP32_RT_TIMER
+	select ESP32_TIMER0

Review comment:
       I think we should hide the Timer 0 instance  in case the user selected the WiFi. (and disable it in case it was already enabled). 




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



[GitHub] [incubator-nuttx] acassis commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

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



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -259,6 +263,8 @@ config ESP32_WIRELESS
 	select NET
 	select ARCH_PHY_INTERRUPT
 	select ESP32_RNG
+	select ESP32_RT_TIMER
+	select ESP32_TIMER0

Review comment:
       Will it always be hard-coded to Timer0? Also I think in this case we need forbid the user from selecting Timer0 as general purpose timer!




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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074#discussion_r510794880



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -665,4 +671,21 @@ config ESP32_WIFI_FS_MOUNTPT
 
 endmenu # ESP32_WIRELESS
 
+menu "Real-Time Timer"
+	depends on ESP32_RT_TIMER
+
+config ESP32_RT_TIMER_TASK_NAME
+	string "Timer task name"
+	default "rt_timer"
+
+config ESP32_RT_TIMER_TASK_PRIORITY
+	int "Timer task priority"
+	default 10

Review comment:
       I find this default value a bit misleading for a "high priority" task.

##########
File path: arch/xtensa/src/esp32/esp32_rt_timer.c
##########
@@ -0,0 +1,570 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rt_timer.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this args for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <string.h>
+#include <fcntl.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/irq.h>
+#include <nuttx/kthread.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include "hardware/esp32_soc.h"
+#include "esp32_tim.h"
+#include "esp32_rt_timer.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define RT_TIMER_TASK_NAME        CONFIG_ESP32_RT_TIMER_TASK_NAME
+#define RT_TIMER_TASK_PRIORITY    CONFIG_ESP32_RT_TIMER_TASK_PRIORITY
+#define RT_TIMER_TASK_STACK_SIZE  CONFIG_ESP32_RT_TIMER_TASK_STACK_SIZE
+
+#define ESP32_TIMER_PRESCALER     (APB_CLK_FREQ / (1000 * 1000))
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static int s_pid;
+
+static sem_t s_toutsem;
+
+static struct list_node s_runlist;
+static struct list_node s_toutlist;
+
+static struct esp32_tim_dev_s *s_esp32_tim_dev;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: start_rt_timer
+ *
+ * Description:
+ *   Start timer by inserting it into running list and reset hardware timer
+ *   alarm value if this timer in head of list.
+ *
+ * Input Parameters:
+ *   timer - RT timer pointer
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+static void start_rt_timer(FAR struct rt_timer_s *timer)
+{
+  irqstate_t flags;
+  struct rt_timer_s *p;
+  bool inserted = false;
+  uint64_t counter;
+  struct esp32_tim_dev_s *tim = s_esp32_tim_dev;
+
+  flags = enter_critical_section();
+
+  /* Check if timer is "not started" */
+
+  if (!timer->started)
+    {
+      /* Calculate the timer's alarm value */
+
+      ESP32_TIM_GETCTR(tim, &counter);
+      timer->alarm = timer->timeout + counter;
+
+      /** Scan timer list and insert the new timer into previous
+       *  node of timer whose alarm value is larger than new one
+       */
+
+      list_for_every_entry(&s_runlist, p, struct rt_timer_s, list)
+        {
+          if (p->alarm > timer->alarm)
+            {
+              list_add_before(&p->list, &timer->list);
+              inserted = true;
+              break;
+            }
+        }
+
+      /* If not find a larger one, insert new timer into tail of list */
+
+      if (!inserted)
+        {
+          list_add_tail(&s_runlist, &timer->list);
+        }
+
+      timer->started = true;
+
+      /* If this timer is in head of list */
+
+      if (timer == container_of(s_runlist.next, struct rt_timer_s, list))
+        {
+          /* Reset hardware timer alarm */
+
+          ESP32_TIM_SETALRVL(tim, timer->alarm);
+          ESP32_TIM_SETALRM(tim, true);
+        }
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: stop_rt_timer
+ *
+ * Description:
+ *   Stop timer by removing it from running list and reset hardware timer
+ *   alarm value if this timer is in head of list.
+ *
+ * Input Parameters:
+ *   timer - RT timer pointer
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+static void stop_rt_timer(FAR struct rt_timer_s *timer)
+{
+  irqstate_t flags;
+  bool ishead;
+  struct rt_timer_s *next_timer;
+  uint64_t alarm;
+  struct esp32_tim_dev_s *tim = s_esp32_tim_dev;
+
+  flags = enter_critical_section();
+
+  /* Check if timer is "started" */
+
+  if (timer->started)
+    {
+      /* Check if timer is in head of list */
+
+      if (timer == container_of(s_runlist.next, struct rt_timer_s, list))
+        {
+          ishead = true;
+        }
+      else
+        {
+          ishead = false;
+        }
+
+      list_delete(&timer->list);
+      timer->started = false;
+
+      /* If timer is in in head of list */
+
+      if (ishead)
+        {
+          /* If list is not empty */
+
+          if (!list_is_empty(&s_runlist))
+            {
+              /* Reset hardware timer alarm value to be next timer's */
+
+              next_timer = container_of(s_runlist.next,
+                                        struct rt_timer_s,
+                                        list);
+              alarm = next_timer->alarm;
+
+              ESP32_TIM_SETALRVL(tim, alarm);
+              ESP32_TIM_SETALRM(tim, true);
+            }
+        }
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: rt_timer_thread
+ *
+ * Description:
+ *   RT timer working thread, it wait for a timeout semaphore, scan
+ *   the timeout list and process all timers in this list.
+ *
+ * Input Parameters:
+ *   argc - Not used
+ *   argv - Not used
+ *
+ * Returned Value:
+ *   0.
+ *
+ ****************************************************************************/
+
+static int rt_timer_thread(int argc, FAR char *argv[])
+{
+  int ret;
+  irqstate_t flags;
+  struct rt_timer_s *timer;
+
+  while (1)
+    {
+      /* Waiting for timers timeout */
+
+      ret = nxsem_wait(&s_toutsem);
+      if (ret)
+        {
+          tmrerr("ERROR: Wait s_toutsem error=%d\n", ret);
+          assert(0);
+        }
+
+      /* Enter critical to check global timer timeout list */
+
+      flags = enter_critical_section();
+
+      /* Process all timers in list */
+
+      while (!list_is_empty(&s_toutlist))
+        {
+          /* Get first timer in list */
+
+          timer = container_of(s_toutlist.next, struct rt_timer_s, list);
+
+          /* Delete timer from list */
+
+          list_delete(&timer->list);
+          timer->started = false;
+
+          /* Leave from critical to start to call "callback" function */
+
+          leave_critical_section(flags);
+
+          timer->callback(timer->arg);
+
+          /* Check if timer is repeat */
+
+          if (timer->flags & RT_TIMER_REPEAT)
+            {
+              /* Check if timer is "not started" */
+
+              if (timer->started == false)
+                {
+                  /* Restart timer */
+
+                  start_rt_timer(timer);
+                }
+            }
+
+          /* Enter critical for next scaning list */
+
+          flags = enter_critical_section();
+        }
+
+      leave_critical_section(flags);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: rt_timer_isr
+ *
+ * Description:
+ *   Hardware timer interrupt service function.
+ *
+ * Input Parameters:
+ *   irq     - Not used
+ *   context - Not used
+ *   arg     - Not used
+ *
+ * Returned Value:
+ *   0.
+ *
+ ****************************************************************************/
+
+static int rt_timer_isr(int irq, void *context, void *arg)
+{
+  irqstate_t flags;
+  struct rt_timer_s *timer;
+  uint64_t alarm;
+  struct esp32_tim_dev_s *tim = s_esp32_tim_dev;
+
+  /* Clear interrupt register status */
+
+  ESP32_TIM_ACKINT(tim);
+
+  /* Wake up thread to process timeout timers */
+
+  nxsem_post(&s_toutsem);
+
+  flags = enter_critical_section();
+
+  /* Check if there is timer running */
+
+  if (!list_is_empty(&s_runlist))
+    {
+      /* Remove first timer in running list and add it into timeout list */
+
+      timer = container_of(s_runlist.next, struct rt_timer_s, list);
+      list_delete(&timer->list);
+      timer->started = false;
+      list_add_after(&s_toutlist, &timer->list);
+
+      /* Check if thers is timer running */
+
+      if (!list_is_empty(&s_runlist))
+        {
+          /* Reset hardware timer alarm with next timer's alarm value */
+
+          timer = container_of(s_runlist.next, struct rt_timer_s, list);
+          alarm = timer->alarm;
+
+          ESP32_TIM_SETALRVL(tim, alarm);
+          ESP32_TIM_SETALRM(tim, true);
+        }
+    }
+
+  leave_critical_section(flags);
+
+  return 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: rt_timer_create
+ *
+ * Description:
+ *   Create RT timer by into timer creation arguments
+ *
+ * Input Parameters:
+ *   args         - Input RT timer creation arguments
+ *   timer_handle - Output RT timer handle pointer
+ *
+ * Returned Value:
+ *   0 is returned on success. Otherwise, a negated errno value is returned.
+ *
+ ****************************************************************************/
+
+int rt_timer_create(FAR const struct rt_timer_args_s *args,
+                    FAR struct rt_timer_s **timer_handle)
+{
+  struct rt_timer_s *timer;
+
+  timer = (struct rt_timer_s *)kmm_malloc(sizeof(*timer));
+  if (!timer)
+    {
+      tmrerr("ERROR: Failed to allocate %d bytes\n", sizeof(*timer));
+      return -ENOMEM;
+    }
+
+  timer->callback = args->callback;
+  timer->arg      = args->arg;
+  timer->flags    = RT_TIMER_NOFLAGS;
+  timer->started  = false;
+  list_initialize(&timer->list);
+
+  *timer_handle = timer;
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: rt_timer_start
+ *
+ * Description:
+ *   Start RT timer.
+ *
+ * Input Parameters:
+ *   timer   - RT timer pointer
+ *   timeout - Timeout value
+ *   repeat  - If the timer run repeat
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void rt_timer_start(FAR struct rt_timer_s *timer,
+                    uint32_t timeout,
+                    bool repeat)
+{
+  stop_rt_timer(timer);
+
+  if (repeat)
+    {
+      timer->flags |= RT_TIMER_REPEAT;
+    }
+  else
+    {
+      timer->flags &= ~RT_TIMER_REPEAT;
+    }
+
+  timer->timeout = timeout;
+
+  start_rt_timer(timer);
+}
+
+/****************************************************************************
+ * Name: rt_timer_stop
+ *
+ * Description:
+ *   Stop RT timer.
+ *
+ * Input Parameters:
+ *   timer - RT timer pointer
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void rt_timer_stop(FAR struct rt_timer_s *timer)
+{
+  stop_rt_timer(timer);
+}
+
+/****************************************************************************
+ * Name: rt_timer_delete
+ *
+ * Description:
+ *   Stop and deleta RT timer.
+ *
+ * Input Parameters:
+ *   timer - RT timer pointer
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void rt_timer_delete(FAR struct rt_timer_s *timer)
+{
+  stop_rt_timer(timer);
+
+  kmm_free(timer);
+}
+
+/****************************************************************************
+ * Name: esp32_rt_timer_init
+ *
+ * Description:
+ *   Initialize ESP32 RT timer.
+ *
+ * Input Parameters:
+ *   timer_no - Hardware timer number
+ *
+ * Returned Value:
+ *   0 is returned on success. Otherwise, a negated errno value is returned.
+ *
+ ****************************************************************************/
+
+int esp32_rt_timer_init(int timer_no)
+{
+  int pid;
+  irqstate_t flags;
+  struct esp32_tim_dev_s *tim;
+
+  tim = esp32_tim_init(timer_no);
+  if (!tim)
+    {
+      tmrerr("ERROR: Failed to initialize ESP32 timer%d\n", timer_no);
+      return -EINVAL;
+    }
+
+  pid = nxtask_create(RT_TIMER_TASK_NAME,

Review comment:
       I think this one should be `kthread_create` instead.




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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074#discussion_r510853430



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -665,4 +671,21 @@ config ESP32_WIFI_FS_MOUNTPT
 
 endmenu # ESP32_WIRELESS
 
+menu "Real-Time Timer"
+	depends on ESP32_RT_TIMER
+
+config ESP32_RT_TIMER_TASK_NAME
+	string "Timer task name"
+	default "rt_timer"
+
+config ESP32_RT_TIMER_TASK_PRIORITY
+	int "Timer task priority"
+	default 10

Review comment:
       I'm not aware of such document, but I was expecting something higher.  For examples in apps/ the default priority is 100.
   So I was expecting to see something between 100 and HP workqueue priority.




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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074#discussion_r511764286



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -259,6 +263,8 @@ config ESP32_WIRELESS
 	select NET
 	select ARCH_PHY_INTERRUPT
 	select ESP32_RNG
+	select ESP32_RT_TIMER
+	select ESP32_TIMER0

Review comment:
       I think we can add extra API to get timer0, then there will be 2 APIs like `esp32_tim_init` and `esp32_tim0_init`. When WiFi is enable, `esp32_tim_init` can't return timer0, and only `esp32_tim0_init` return timer0.




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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074#discussion_r511764888



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -665,4 +671,21 @@ config ESP32_WIFI_FS_MOUNTPT
 
 endmenu # ESP32_WIRELESS
 
+menu "Real-Time Timer"
+	depends on ESP32_RT_TIMER
+
+config ESP32_RT_TIMER_TASK_NAME
+	string "Timer task name"
+	default "rt_timer"
+
+config ESP32_RT_TIMER_TASK_PRIORITY
+	int "Timer task priority"
+	default 10

Review comment:
       Yes I think it should be only a little lower than HP workqueue, but it also should process real-time.




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



[GitHub] [incubator-nuttx] acassis merged pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
acassis merged pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074


   


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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2074: xtensa/esp32: Add real-time timer support for WiFi

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2074:
URL: https://github.com/apache/incubator-nuttx/pull/2074#discussion_r510826829



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -259,6 +263,8 @@ config ESP32_WIRELESS
 	select NET
 	select ARCH_PHY_INTERRUPT
 	select ESP32_RNG
+	select ESP32_RT_TIMER
+	select ESP32_TIMER0

Review comment:
       Because WiFi should use 1 hardware timer, so I think using timer0 directly and tell users that timer0 is used by WiFi and they should use other timers, this is also a good way.




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