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/12/30 08:33:23 UTC
[GitHub] [incubator-nuttx] SChina-CSC-SDD-Dev opened a new pull request #2625: Add time ticket related APIs to support hot sleep function
SChina-CSC-SDD-Dev opened a new pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625
## Summary
Add time ticket related APIs to support hot sleep function
## Impact
For watchdog, add two new APIs with below functions
- Get idle time for hot sleep
- Update watchdogs' lag after resume
For system clock, add a new API to sync system time with idle ticks after resume
## Testing
Built in with these three APIs
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r560693468
##########
File path: sched/wdog/wd_getidletime.c
##########
@@ -0,0 +1,64 @@
+/****************************************************************************
+ * sched/wdog/wd_getidletime.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_getidletime
+ *
+ * Description:
+ * This function returns the idle time.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * The time in system ticks remaining for idle.
+ * Zero means system is busy.
+ *
+ ****************************************************************************/
+
+int wd_getidletime(void)
+{
+ int idletime = -1;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
+
+ FAR struct wdog_s *curr = (FAR struct wdog_s *)g_wdactivelist.head;
+
+ idletime = (curr ? curr->lag : 0);
Review comment:
But it isn't the right value, because it ignore the RR interval.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r556410633
##########
File path: sched/wdog/wd_getidletime.c
##########
@@ -0,0 +1,64 @@
+/****************************************************************************
+ * sched/wdog/wd_getidletime.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_getidletime
+ *
+ * Description:
+ * This function returns the idle time.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * The time in system ticks remaining for idle.
+ * Zero means system is busy.
+ *
+ ****************************************************************************/
+
+int wd_getidletime(void)
+{
+ int idletime = -1;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
+
+ FAR struct wdog_s *curr = (FAR struct wdog_s *)g_wdactivelist.head;
+
+ idletime = (curr ? curr->lag : 0);
Review comment:
Do you want to use the returned value to decide how long the device can sleep?
##########
File path: sched/wdog/wd_setsleepticks.c
##########
@@ -0,0 +1,65 @@
+/****************************************************************************
+ * sched/wdog/wd_setsleepticks.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_setsleepticks
+ *
+ * Description:
+ * This function will update watchdogs' lag.
+ *
+ * Input Parameters:
+ * ticks - sleep ticks
+ *
+ * Returned Value:
+ * Zero means success.
+ *
+ ****************************************************************************/
+
+int wd_setsleepticks(clock_t ticks)
+{
+ irqstate_t flags;
+ FAR struct wdog_s * curr;
+
+ flags = enter_critical_section();
+
+ for (curr = (FAR struct wdog_s *)(g_wdactivelist.head);
+ curr; curr = curr->next)
+ {
+ curr->lag -= ticks;
Review comment:
Why not directly increase the system tick by nxsched_process_timer? it's better to update the system time to math the real elapse.
##########
File path: include/nuttx/clock.h
##########
@@ -436,6 +436,22 @@ int clock_systime_timespec(FAR struct timespec *ts);
int clock_cpuload(int pid, FAR struct cpuload_s *cpuload);
#endif
+/****************************************************************************
+ * Name: clock_systime_steps
+ *
+ * Description:
+ * Add system time by idle ticks.
+ *
+ * Input Parameters:
+ * ticks - Idle ticks
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void clock_systime_steps(sclock_t ticks);
Review comment:
should we move it to arch.h like nxsched_process_timer?
##########
File path: include/nuttx/clock.h
##########
@@ -436,6 +436,22 @@ int clock_systime_timespec(FAR struct timespec *ts);
int clock_cpuload(int pid, FAR struct cpuload_s *cpuload);
#endif
+/****************************************************************************
+ * Name: clock_systime_steps
+ *
+ * Description:
+ * Add system time by idle ticks.
+ *
+ * Input Parameters:
+ * ticks - Idle ticks
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void clock_systime_steps(sclock_t ticks);
Review comment:
I think from sched level, OS don't need to know whether this wakeup come from the deep sleep or not. Why not reuse nxsched_process_timer 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] xiaoxiang781216 commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759443692
> > > @xiaoxiang781216 could you take a look here please.
> >
> >
> > Sorry, I thought this PR is for esp32 specific. So this patch want to sync the system time after wake up from RTC timer?
>
> Hi @xiaoxiang781216 Yes, after entering auto-sleep(pm standby mode), the system clock interruption will stop, when exiting auto-sleep, the idletime tick obtained through watchdog monitoring (by calling wd_getidletime) will be inaccurate, so after exiting auto-sleep, the obtained system tick needs to be compensated (by calling wd_setsleepticks).
But, the compensation isn't completed there are other places depend on the correct timing:
1. systime need to update
2. sched::timeslice need to update
2. Many field in sporadic_s need to update
To sync the timing information, the better method is:
1. Call nxsched_process_timer with enough times after wakeup for fixed tick mode
2. up_timer_gettime return the compensated time for tickless mode
The above method can ensure all timing related component get updated correctly.
One improvement, we can change nxsched_process_timer prototype to accept the elasped ticks:
void nxsched_process_timer(uint32_t ticks);
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r560694031
##########
File path: sched/wdog/wd_getidletime.c
##########
@@ -0,0 +1,64 @@
+/****************************************************************************
+ * sched/wdog/wd_getidletime.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_getidletime
+ *
+ * Description:
+ * This function returns the idle time.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * The time in system ticks remaining for idle.
+ * Zero means system is busy.
+ *
+ ****************************************************************************/
+
+int wd_getidletime(void)
+{
+ int idletime = -1;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
+
+ FAR struct wdog_s *curr = (FAR struct wdog_s *)g_wdactivelist.head;
+
+ idletime = (curr ? curr->lag : 0);
Review comment:
To optimize the power consumption, the first step is switching to the tickless mode, and then you can get the accurate idle time from up_alarm_start or up_timer_start. you can:
1. Enter the different low power level base on the length of idle time
2. Or combine include/nuttx/power/pm.h to support the more complex scenario
We can achieve this goal without modifying the common code at all.
----------------------------------------------------------------
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] xiaoxiang781216 commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759356180
> @xiaoxiang781216 could you take a look here please.
Sorry, I thought this PR is for esp32 specific. So this patch want to sync the system time after wake up from RTC 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] cwespressif commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r550065852
##########
File path: sched/wdog/wd_setsleepticks.c
##########
@@ -0,0 +1,67 @@
+/****************************************************************************
+ * sched/wdog/wd_setsleepticks.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_setsleepticks
+ *
+ * Description:
+ * This function will update watchdogs' lag.
+ *
+ * Input Parameters:
+ * ticks - sleep ticks
+ *
+ * Returned Value:
+ * Zero means success.
+ *
+ ****************************************************************************/
+
+
Review comment:
Please use nuttx/tools/nxstyle.c to check styles.
----------------------------------------------------------------
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 pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759330012
@xiaoxiang781216 could you take a look here please.
----------------------------------------------------------------
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] cwespressif commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r550110410
##########
File path: sched/clock/clock_systime_ticks.c
##########
@@ -154,3 +154,25 @@ clock_t clock_systime_ticks(void)
# endif /* CONFIG_SYSTEM_TIME64 */
#endif /* CONFIG_SCHED_TICKLESS */
}
+
+/****************************************************************************
+ * Name: clock_systime_steps
+ *
+ * Description:
+ * Add system time by idle ticks.
+ *
+ * Input Parameters:
+ * ticks - Idle ticks
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void clock_systime_steps(sclock_t ticks)
+{
+ irqstate_t flags;
+ flags = enter_critical_section();
+ g_system_timer += ticks;
Review comment:
If enable CONFIG_SCHED_TICKLESS, g_system_timer cannot be used as system tick.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r560694031
##########
File path: sched/wdog/wd_getidletime.c
##########
@@ -0,0 +1,64 @@
+/****************************************************************************
+ * sched/wdog/wd_getidletime.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_getidletime
+ *
+ * Description:
+ * This function returns the idle time.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * The time in system ticks remaining for idle.
+ * Zero means system is busy.
+ *
+ ****************************************************************************/
+
+int wd_getidletime(void)
+{
+ int idletime = -1;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
+
+ FAR struct wdog_s *curr = (FAR struct wdog_s *)g_wdactivelist.head;
+
+ idletime = (curr ? curr->lag : 0);
Review comment:
To optimize the power consumption, the first step is switching to the tickless mode, and then you can get the accurate idle time from up_alarm_start or up_timer_start.
----------------------------------------------------------------
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] SChina-CSC-SDD-Dev commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
SChina-CSC-SDD-Dev commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-768100795
We will propose the three below solutions as above suggestion.
- Solution One
We will create a new API like nxsched_process_timer_ext(ticks) called by HW platform.
When resuming, HW platform will update ticks about shed, systime and sporadic etc
separately through this API. And this API will update ticks through these corresponding
module API. If the called module API not found, we will create the new module API to
update ticks. If this solution can be acceptable and merged into mainline without other
issues, we will follow this solution.
- Solution Two
Similar with solution One, we will create a new API like nxched_process_timer_ext(ticks)
but called by nxched_process_timer(). But it seems overlying.
- Solution Three
We will create the mechanism. When the related module will update ticks by itself when
the event has been triggered.
----------------------------------------------------------------
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 pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-769023606
> I agree this simple API will have minimal impact.
Should this PR be changed to Draft ?
----------------------------------------------------------------
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] SChina-CSC-SDD-Dev commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
SChina-CSC-SDD-Dev commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-763350514
I agree this simple API will have minimal impact.
----------------------------------------------------------------
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] SChina-CSC-SDD-Dev removed a comment on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
SChina-CSC-SDD-Dev removed a comment on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-768100795
We will propose the three below solutions as above suggestion.
- Solution One
We will create a new API like nxsched_process_timer_ext(ticks) called by HW platform.
When resuming, HW platform will update ticks about shed, systime and sporadic etc
separately through this API. And this API will update ticks through these corresponding
module API. If the called module API not found, we will create the new module API to
update ticks. If this solution can be acceptable and merged into mainline without other
issues, we will follow this solution.
- Solution Two
Similar with solution One, we will create a new API like nxched_process_timer_ext(ticks)
but called by nxched_process_timer(). But it seems overlying.
- Solution Three
We will create the mechanism. When the related module will update ticks by itself when
the event has been triggered.
----------------------------------------------------------------
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] xiaoxiang781216 edited a comment on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759443692
> > > @xiaoxiang781216 could you take a look here please.
> >
> >
> > Sorry, I thought this PR is for esp32 specific. So this patch want to sync the system time after wake up from RTC timer?
>
> Hi @xiaoxiang781216 Yes, after entering auto-sleep(pm standby mode), the system clock interruption will stop, when exiting auto-sleep, the idletime tick obtained through watchdog monitoring (by calling wd_getidletime) will be inaccurate, so after exiting auto-sleep, the obtained system tick needs to be compensated (by calling wd_setsleepticks).
But, the compensation isn't completed there are other places depend on the correct timing, at least:
1. systime need to update
2. sched::timeslice need to update
2. Many field in sporadic_s need to update
To sync the timing information, the better method is:
1. Call nxsched_process_timer with enough times after wakeup for fixed tick mode
2. up_timer_gettime return the compensated time for tickless mode
The above method can ensure all timing related component get updated correctly.
One improvement, we can change nxsched_process_timer prototype to accept the elasped ticks:
```
void nxsched_process_timer(uint32_t ticks);
```
So, we can call nxsched_process_timer once.
Of course, the best method is using a timer which still couting in all low power mode, then you don't need any special handling after wakeup.
----------------------------------------------------------------
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] xiaoxiang781216 commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-782225159
Let's close it since the tickless mode is the right direction to go after the offline discussion.
----------------------------------------------------------------
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] xiaoxiang781216 edited a comment on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759443692
> > > @xiaoxiang781216 could you take a look here please.
> >
> >
> > Sorry, I thought this PR is for esp32 specific. So this patch want to sync the system time after wake up from RTC timer?
>
> Hi @xiaoxiang781216 Yes, after entering auto-sleep(pm standby mode), the system clock interruption will stop, when exiting auto-sleep, the idletime tick obtained through watchdog monitoring (by calling wd_getidletime) will be inaccurate, so after exiting auto-sleep, the obtained system tick needs to be compensated (by calling wd_setsleepticks).
But, the compensation isn't completed there are other places depend on the correct timing. At least:
1. systime need to update
2. sched::timeslice need to update
2. Many field in sporadic_s need to update
To sync the timing information, the better method is:
1. Call nxsched_process_timer with enough times after wakeup for fixed tick mode
2. up_timer_gettime return the compensated time for tickless mode
The above method can ensure all timing related component get updated correctly.
One improvement, we can change nxsched_process_timer prototype to accept the elasped ticks:
void nxsched_process_timer(uint32_t ticks);
----------------------------------------------------------------
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] xiaoxiang781216 edited a comment on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759443692
> > > @xiaoxiang781216 could you take a look here please.
> >
> >
> > Sorry, I thought this PR is for esp32 specific. So this patch want to sync the system time after wake up from RTC timer?
>
> Hi @xiaoxiang781216 Yes, after entering auto-sleep(pm standby mode), the system clock interruption will stop, when exiting auto-sleep, the idletime tick obtained through watchdog monitoring (by calling wd_getidletime) will be inaccurate, so after exiting auto-sleep, the obtained system tick needs to be compensated (by calling wd_setsleepticks).
But, the compensation isn't completed there are other places depend on the correct timing. At least:
1. systime need to update
2. sched::timeslice need to update
2. Many field in sporadic_s need to update
To sync the timing information, the better method is:
1. Call nxsched_process_timer with enough times after wakeup for fixed tick mode
2. up_timer_gettime return the compensated time for tickless mode
The above method can ensure all timing related component get updated correctly.
One improvement, we can change nxsched_process_timer prototype to accept the elasped ticks:
```
void nxsched_process_timer(uint32_t ticks);
```
So, we can call nxsched_process_timer once.
----------------------------------------------------------------
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] SChina-CSC-SDD-Dev commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
SChina-CSC-SDD-Dev commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r560689178
##########
File path: sched/wdog/wd_getidletime.c
##########
@@ -0,0 +1,64 @@
+/****************************************************************************
+ * sched/wdog/wd_getidletime.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_getidletime
+ *
+ * Description:
+ * This function returns the idle time.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * The time in system ticks remaining for idle.
+ * Zero means system is busy.
+ *
+ ****************************************************************************/
+
+int wd_getidletime(void)
+{
+ int idletime = -1;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
+
+ FAR struct wdog_s *curr = (FAR struct wdog_s *)g_wdactivelist.head;
+
+ idletime = (curr ? curr->lag : 0);
Review comment:
Yes, we will use this value for power save.
----------------------------------------------------------------
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] SChina-CSC-SDD-Dev commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
SChina-CSC-SDD-Dev commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r553163678
##########
File path: sched/wdog/wd_setsleepticks.c
##########
@@ -0,0 +1,67 @@
+/****************************************************************************
+ * sched/wdog/wd_setsleepticks.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_setsleepticks
+ *
+ * Description:
+ * This function will update watchdogs' lag.
+ *
+ * Input Parameters:
+ * ticks - sleep ticks
+ *
+ * Returned Value:
+ * Zero means success.
+ *
+ ****************************************************************************/
+
+
Review comment:
Done
##########
File path: sched/clock/clock_systime_ticks.c
##########
@@ -154,3 +154,25 @@ clock_t clock_systime_ticks(void)
# endif /* CONFIG_SYSTEM_TIME64 */
#endif /* CONFIG_SCHED_TICKLESS */
}
+
+/****************************************************************************
+ * Name: clock_systime_steps
+ *
+ * Description:
+ * Add system time by idle ticks.
+ *
+ * Input Parameters:
+ * ticks - Idle ticks
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void clock_systime_steps(sclock_t ticks)
+{
+ irqstate_t flags;
+ flags = enter_critical_section();
+ g_system_timer += ticks;
Review comment:
Done
----------------------------------------------------------------
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] SChina-CSC-SDD-Dev commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
SChina-CSC-SDD-Dev commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r554662339
##########
File path: sched/clock/clock_systime_ticks.c
##########
@@ -154,3 +154,25 @@ clock_t clock_systime_ticks(void)
# endif /* CONFIG_SYSTEM_TIME64 */
#endif /* CONFIG_SCHED_TICKLESS */
}
+
+/****************************************************************************
+ * Name: clock_systime_steps
+ *
+ * Description:
+ * Add system time by idle ticks.
+ *
+ * Input Parameters:
+ * ticks - Idle ticks
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void clock_systime_steps(sclock_t ticks)
+{
+ irqstate_t flags;
+ flags = enter_critical_section();
+ g_system_timer += ticks;
Review comment:
I have changed this PR into ready status for merge.
----------------------------------------------------------------
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] cwespressif commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
cwespressif commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759915998
> nxsched_process_timer
The RTC timer still works in all low power mode, we compensate the system clock according to the time of low power mode counted by RTC timer.
If change nxsched_ process_ timer prototype to accept the elasped ticks:
`void nxsched_process_timer(uint32_t ticks);`
it would be better, in idle task, the default call is
`nxsched_ process_ timer(1);`
----------------------------------------------------------------
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] cwespressif commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r554663666
##########
File path: sched/clock/clock_systime_ticks.c
##########
@@ -154,3 +154,25 @@ clock_t clock_systime_ticks(void)
# endif /* CONFIG_SYSTEM_TIME64 */
#endif /* CONFIG_SCHED_TICKLESS */
}
+
+/****************************************************************************
+ * Name: clock_systime_steps
+ *
+ * Description:
+ * Add system time by idle ticks.
+ *
+ * Input Parameters:
+ * ticks - Idle ticks
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void clock_systime_steps(sclock_t ticks)
+{
+ irqstate_t flags;
+ flags = enter_critical_section();
+ g_system_timer += ticks;
Review comment:
LGTM
----------------------------------------------------------------
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] xiaoxiang781216 commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759939687
Yes, I think:
1. Change nxsched_process_timer to accept the elapsed ticks
2. Call nxsched_process_timer with the correct ticks after wakeup
it's more simpler and clear than to add the special wd/systime functions.
----------------------------------------------------------------
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] cwespressif commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r554659725
##########
File path: sched/clock/clock_systime_ticks.c
##########
@@ -154,3 +154,25 @@ clock_t clock_systime_ticks(void)
# endif /* CONFIG_SYSTEM_TIME64 */
#endif /* CONFIG_SCHED_TICKLESS */
}
+
+/****************************************************************************
+ * Name: clock_systime_steps
+ *
+ * Description:
+ * Add system time by idle ticks.
+ *
+ * Input Parameters:
+ * ticks - Idle ticks
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void clock_systime_steps(sclock_t ticks)
+{
+ irqstate_t flags;
+ flags = enter_critical_section();
+ g_system_timer += ticks;
Review comment:
I see this PR is still in draft status, could you change it so that it can be merged into the NuttX mainline, thank you.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r560694031
##########
File path: sched/wdog/wd_getidletime.c
##########
@@ -0,0 +1,64 @@
+/****************************************************************************
+ * sched/wdog/wd_getidletime.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_getidletime
+ *
+ * Description:
+ * This function returns the idle time.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * The time in system ticks remaining for idle.
+ * Zero means system is busy.
+ *
+ ****************************************************************************/
+
+int wd_getidletime(void)
+{
+ int idletime = -1;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
+
+ FAR struct wdog_s *curr = (FAR struct wdog_s *)g_wdactivelist.head;
+
+ idletime = (curr ? curr->lag : 0);
Review comment:
To optimize the power consumption, the first step is switching to the tickless mode, and then you can get the accurate idle time from up_alarm_start or up_timer_start. you can:
1. Enter the different low power level base on the length of idle time
2. Or combine include/nuttx/power/pm.h to support the more complex scenario
We can achieve the great power saving without modifying the common code at all.
----------------------------------------------------------------
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] SChina-CSC-SDD-Dev commented on a change in pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
SChina-CSC-SDD-Dev commented on a change in pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#discussion_r560690115
##########
File path: sched/wdog/wd_setsleepticks.c
##########
@@ -0,0 +1,65 @@
+/****************************************************************************
+ * sched/wdog/wd_setsleepticks.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work 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 <nuttx/wdog.h>
+#include <nuttx/irq.h>
+
+#include "wdog/wdog.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wd_setsleepticks
+ *
+ * Description:
+ * This function will update watchdogs' lag.
+ *
+ * Input Parameters:
+ * ticks - sleep ticks
+ *
+ * Returned Value:
+ * Zero means success.
+ *
+ ****************************************************************************/
+
+int wd_setsleepticks(clock_t ticks)
+{
+ irqstate_t flags;
+ FAR struct wdog_s * curr;
+
+ flags = enter_critical_section();
+
+ for (curr = (FAR struct wdog_s *)(g_wdactivelist.head);
+ curr; curr = curr->next)
+ {
+ curr->lag -= ticks;
Review comment:
This is hotline solution for ESP nuttx, maybe not fit for mainline. And we also think that update systime with elapse is a better choice for mainline.
----------------------------------------------------------------
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] cwespressif commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
cwespressif commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-759415008
> > @xiaoxiang781216 could you take a look here please.
>
> Sorry, I thought this PR is for esp32 specific. So this patch want to sync the system time after wake up from RTC timer?
Hi @xiaoxiang781216 Yes, after entering auto-sleep(pm standby mode), the system clock interruption will stop, when exiting auto-sleep, the idletime tick obtained through watchdog monitoring (by calling wd_getidletime) will be inaccurate, so after exiting auto-sleep, the obtained system tick needs to be compensated (by calling wd_setsleepticks).
----------------------------------------------------------------
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] xiaoxiang781216 commented on pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625#issuecomment-769044096
Yes, I think the right solution is enable the tickless mode, and then get the accurate timeout value from new arch API automatically.
----------------------------------------------------------------
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] xiaoxiang781216 closed pull request #2625: Add time ticket related APIs to support hot sleep function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 closed pull request #2625:
URL: https://github.com/apache/incubator-nuttx/pull/2625
----------------------------------------------------------------
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