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/13 06:50:43 UTC
[GitHub] [incubator-nuttx] cwespressif opened a new pull request #1978: xtensa/esp32: Add power management of deep-sleep
cwespressif opened a new pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978
## Summary
Add power management of deep-sleep.
## Impact
Device enters deep-sleep to effectively save power consumption.
## Testing
ESP32 ULP Kit (with ESP-WROOM-32 module) && multimeter (Test current value and power consumption).
Test with pmconfig, commands like:
```
pmconfig relax idle
pmconfig relax standby
```
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#issuecomment-709995360
LGTM. @acassis Please take a look and merge if everything is okay. Thanks.
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#discussion_r505489388
##########
File path: arch/xtensa/src/esp32/esp32_pm.c
##########
@@ -180,8 +215,28 @@ static struct esp32_sleep_config_t s_config =
* Private Functions
****************************************************************************/
+/* CPU do while loop for some time. */
+
extern void ets_delay_us(uint32_t us);
+/* Set CRC of Fast RTC memory 0-0x7ff into RTC STORE7. */
+
+extern void set_rtc_memory_crc(void);
+
+/* Set the real CPU ticks per us to the ets,
+ * so that ets_delay_us will be accurate.
+ */
+
+extern void ets_update_cpu_frequency_rom(uint32_t ticks_per_us);
+
+/* Get xtal_freq value, If value not stored in RTC_STORE5, than store. */
+
+extern uint32_t ets_get_detected_xtal_freq(void);
+
+/* Get the reset reason for CPU. */
+
+extern enum esp32_reset_reason_e rtc_get_reset_reason(int cpu_no);
Review comment:
OK, I have replaced rtc_get_reset_reason with esp32_resetcause.
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#discussion_r504357332
##########
File path: arch/xtensa/src/esp32/esp32_pm.c
##########
@@ -180,8 +215,28 @@ static struct esp32_sleep_config_t s_config =
* Private Functions
****************************************************************************/
+/* CPU do while loop for some time. */
+
extern void ets_delay_us(uint32_t us);
+/* Set CRC of Fast RTC memory 0-0x7ff into RTC STORE7. */
+
+extern void set_rtc_memory_crc(void);
+
+/* Set the real CPU ticks per us to the ets,
+ * so that ets_delay_us will be accurate.
+ */
+
+extern void ets_update_cpu_frequency_rom(uint32_t ticks_per_us);
+
+/* Get xtal_freq value, If value not stored in RTC_STORE5, than store. */
+
+extern uint32_t ets_get_detected_xtal_freq(void);
+
+/* Get the reset reason for CPU. */
+
+extern enum esp32_reset_reason_e rtc_get_reset_reason(int cpu_no);
Review comment:
Hi @Ouss4
We may need to continue to improve this enum type：
https://github.com/apache/incubator-nuttx/blob/fff15f1b93359c17c920cd79b01b24dc1900b583/arch/xtensa/src/esp32/esp32_resetcause.h#L25-L39
You can refer to this：
https://github.com/cwespressif/incubator-nuttx/blob/feature/add_esp32_deep_sleep/arch/xtensa/src/esp32/esp32_pm.c#L123-L141
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
acassis merged pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#discussion_r504354776
##########
File path: arch/xtensa/src/esp32/esp32_pm.c
##########
@@ -180,8 +215,28 @@ static struct esp32_sleep_config_t s_config =
* Private Functions
****************************************************************************/
+/* CPU do while loop for some time. */
+
extern void ets_delay_us(uint32_t us);
+/* Set CRC of Fast RTC memory 0-0x7ff into RTC STORE7. */
+
+extern void set_rtc_memory_crc(void);
+
+/* Set the real CPU ticks per us to the ets,
+ * so that ets_delay_us will be accurate.
+ */
+
+extern void ets_update_cpu_frequency_rom(uint32_t ticks_per_us);
+
+/* Get xtal_freq value, If value not stored in RTC_STORE5, than store. */
+
+extern uint32_t ets_get_detected_xtal_freq(void);
+
+/* Get the reset reason for CPU. */
+
+extern enum esp32_reset_reason_e rtc_get_reset_reason(int cpu_no);
Review comment:
Hi @Ouss4 , Yes, this function is in the ROM, it is the same as
```
enum esp32_resetcause_e esp32_resetcause(int cpu);
```
Maybe this file: esp32_resetcause.c can be remove.
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#discussion_r504883097
##########
File path: arch/xtensa/src/esp32/esp32_pm.c
##########
@@ -180,8 +215,28 @@ static struct esp32_sleep_config_t s_config =
* Private Functions
****************************************************************************/
+/* CPU do while loop for some time. */
+
extern void ets_delay_us(uint32_t us);
+/* Set CRC of Fast RTC memory 0-0x7ff into RTC STORE7. */
+
+extern void set_rtc_memory_crc(void);
+
+/* Set the real CPU ticks per us to the ets,
+ * so that ets_delay_us will be accurate.
+ */
+
+extern void ets_update_cpu_frequency_rom(uint32_t ticks_per_us);
+
+/* Get xtal_freq value, If value not stored in RTC_STORE5, than store. */
+
+extern uint32_t ets_get_detected_xtal_freq(void);
+
+/* Get the reset reason for CPU. */
+
+extern enum esp32_reset_reason_e rtc_get_reset_reason(int cpu_no);
Review comment:
> Hi @Ouss4
> We may need to continue to improve this enum type：
>
> https://github.com/apache/incubator-nuttx/blob/fff15f1b93359c17c920cd79b01b24dc1900b583/arch/xtensa/src/esp32/esp32_resetcause.h#L25-L39
>
> You can refer to this：
> https://github.com/cwespressif/incubator-nuttx/blob/feature/add_esp32_deep_sleep/arch/xtensa/src/esp32/esp32_pm.c#L123-L141
Hi @cwespressif I think it is a good idea to keep ESP32_* in these enums instead of using generic names like "NO_MEAN", because it has "no meaning", no pun intended.
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#discussion_r503947296
##########
File path: arch/xtensa/src/esp32/esp32_pm.c
##########
@@ -180,8 +215,28 @@ static struct esp32_sleep_config_t s_config =
* Private Functions
****************************************************************************/
+/* CPU do while loop for some time. */
+
extern void ets_delay_us(uint32_t us);
+/* Set CRC of Fast RTC memory 0-0x7ff into RTC STORE7. */
+
+extern void set_rtc_memory_crc(void);
+
+/* Set the real CPU ticks per us to the ets,
+ * so that ets_delay_us will be accurate.
+ */
+
+extern void ets_update_cpu_frequency_rom(uint32_t ticks_per_us);
+
+/* Get xtal_freq value, If value not stored in RTC_STORE5, than store. */
+
+extern uint32_t ets_get_detected_xtal_freq(void);
+
+/* Get the reset reason for CPU. */
+
+extern enum esp32_reset_reason_e rtc_get_reset_reason(int cpu_no);
Review comment:
@cwespressif is this function present in ROM?
There is the same function here: https://github.com/apache/incubator-nuttx/blob/fff15f1b93359c17c920cd79b01b24dc1900b583/arch/xtensa/src/esp32/esp32_resetcause.h#L53
Since it exists already, should that file be removed?
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#discussion_r504898752
##########
File path: arch/xtensa/src/esp32/esp32_pm.c
##########
@@ -180,8 +215,28 @@ static struct esp32_sleep_config_t s_config =
* Private Functions
****************************************************************************/
+/* CPU do while loop for some time. */
+
extern void ets_delay_us(uint32_t us);
+/* Set CRC of Fast RTC memory 0-0x7ff into RTC STORE7. */
+
+extern void set_rtc_memory_crc(void);
+
+/* Set the real CPU ticks per us to the ets,
+ * so that ets_delay_us will be accurate.
+ */
+
+extern void ets_update_cpu_frequency_rom(uint32_t ticks_per_us);
+
+/* Get xtal_freq value, If value not stored in RTC_STORE5, than store. */
+
+extern uint32_t ets_get_detected_xtal_freq(void);
+
+/* Get the reset reason for CPU. */
+
+extern enum esp32_reset_reason_e rtc_get_reset_reason(int cpu_no);
Review comment:
The `esp32_resetcause` returns the value read from the register with no conversion. `rtc_get_reset_reason` in the other had seems to do some conversion before returning. This is why the values are different. If we take only the ROM version of resetreason we'll have to adapt the values as Chen Wen did.
As Alan stated, we just need to add the ESP32_ prefix.
----------------------------------------------------------------
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 #1978: xtensa/esp32: Add power management of deep-sleep
Posted by GitBox <gi...@apache.org>.
cwespressif commented on a change in pull request #1978:
URL: https://github.com/apache/incubator-nuttx/pull/1978#discussion_r504357332
##########
File path: arch/xtensa/src/esp32/esp32_pm.c
##########
@@ -180,8 +215,28 @@ static struct esp32_sleep_config_t s_config =
* Private Functions
****************************************************************************/
+/* CPU do while loop for some time. */
+
extern void ets_delay_us(uint32_t us);
+/* Set CRC of Fast RTC memory 0-0x7ff into RTC STORE7. */
+
+extern void set_rtc_memory_crc(void);
+
+/* Set the real CPU ticks per us to the ets,
+ * so that ets_delay_us will be accurate.
+ */
+
+extern void ets_update_cpu_frequency_rom(uint32_t ticks_per_us);
+
+/* Get xtal_freq value, If value not stored in RTC_STORE5, than store. */
+
+extern uint32_t ets_get_detected_xtal_freq(void);
+
+/* Get the reset reason for CPU. */
+
+extern enum esp32_reset_reason_e rtc_get_reset_reason(int cpu_no);
Review comment:
Hi @Ouss4
We may need to continue to improve this enum type：
https://github.com/apache/incubator-nuttx/blob/fff15f1b93359c17c920cd79b01b24dc1900b583/arch/xtensa/src/esp32/esp32_resetcause.h#L25-L39
You can refer to this：
https://github.com/apache/incubator-nuttx/blob/fff15f1b93359c17c920cd79b01b24dc1900b583/arch/xtensa/src/esp32/esp32_resetcause.h#L25-L39
----------------------------------------------------------------
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