You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/08/20 18:40:10 UTC
[GitHub] [mynewt-core] JustineKH opened a new pull request #2359: hw/mcu/dialog: Add all reset reason checks
JustineKH opened a new pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359
Adding all checks for reboot reasons per da1469x ds.
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-682327396
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### sys/reboot/include/reboot/log_reboot.h
<details>
```diff
@@ -27,15 +27,15 @@
#define REBOOT_REASON_STR(reason) \
(reason == HAL_RESET_POR ? "HARD" : \
- (reason == HAL_RESET_PIN ? "RESET_PIN" : \
- (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
- (reason == HAL_RESET_SOFT ? "SOFT" : \
- (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
- (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
- (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
- (reason == HAL_RESET_DFU ? "DFU" : \
- (reason == HAL_RESET_OTHER ? "OTHER" : \
- "UNKNOWN")))))))))
+ (reason == HAL_RESET_PIN ? "RESET_PIN" : \
+ (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
+ (reason == HAL_RESET_SOFT ? "SOFT" : \
+ (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
+ (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
+ (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
+ (reason == HAL_RESET_DFU ? "DFU" : \
+ (reason == HAL_RESET_OTHER ? "OTHER" : \
+ "UNKNOWN")))))))))
struct log_reboot_info {
enum hal_reset_reason reason;
```
</details>
----------------------------------------------------------------
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] [mynewt-core] andrzej-kaczmarek commented on a change in pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on a change in pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#discussion_r480175863
##########
File path: sys/reboot/src/log_reboot.c
##########
@@ -383,6 +384,50 @@ reboot_conf_export(void (*func)(char *name, char *val),
return 0;
}
+void
Review comment:
`const char *`
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-677871520
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/mcu/dialog/da1469x/src/hal_system.c
<details>
```diff
@@ -53,7 +53,7 @@
if (reg & CRG_TOP_RESET_STAT_REG_PORESET_STAT_Msk) {
if (reg & CRG_TOP_RESET_STAT_REG_HWRESET_STAT_Msk) {
- g_hal_reset_reason = HAL_RESET_PIN;
+ g_hal_reset_reason = HAL_RESET_PIN;
}
if (reg & CRG_TOP_RESET_STAT_REG_HWRESET_STAT_Msk) {
g_hal_reset_reason = HAL_RESET_POR;
```
</details>
#### sys/reboot/include/reboot/log_reboot.h
<details>
```diff
@@ -27,16 +27,16 @@
#define REBOOT_REASON_STR(reason) \
(reason == HAL_RESET_POR ? "HARD" : \
- (reason == HAL_RESET_PIN ? "RESET_PIN" : \
- (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
- (reason == HAL_RESET_SOFT ? "SOFT" : \
- (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
- (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
- (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
- (reason == HAL_RESET_DFU ? "DFU" : \
- (reason == HAL_RESET_WATCHDOG2 ? "WDOG2" : \
- (reason == HAL_RESET_SWD ? "SWD" : \
- "UNKNOWN"))))))))))
+ (reason == HAL_RESET_PIN ? "RESET_PIN" : \
+ (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
+ (reason == HAL_RESET_SOFT ? "SOFT" : \
+ (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
+ (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
+ (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
+ (reason == HAL_RESET_DFU ? "DFU" : \
+ (reason == HAL_RESET_WATCHDOG2 ? "WDOG2" : \
+ (reason == HAL_RESET_SWD ? "SWD" : \
+ "UNKNOWN"))))))))))
struct log_reboot_info {
enum hal_reset_reason reason;
```
</details>
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-682327396
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### sys/reboot/include/reboot/log_reboot.h
<details>
```diff
@@ -27,15 +27,15 @@
#define REBOOT_REASON_STR(reason) \
(reason == HAL_RESET_POR ? "HARD" : \
- (reason == HAL_RESET_PIN ? "RESET_PIN" : \
- (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
- (reason == HAL_RESET_SOFT ? "SOFT" : \
- (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
- (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
- (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
- (reason == HAL_RESET_DFU ? "DFU" : \
- (reason == HAL_RESET_OTHER ? "OTHER" : \
- "UNKNOWN")))))))))
+ (reason == HAL_RESET_PIN ? "RESET_PIN" : \
+ (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
+ (reason == HAL_RESET_SOFT ? "SOFT" : \
+ (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
+ (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
+ (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
+ (reason == HAL_RESET_DFU ? "DFU" : \
+ (reason == HAL_RESET_OTHER ? "OTHER" : \
+ "UNKNOWN")))))))))
struct log_reboot_info {
enum hal_reset_reason reason;
```
</details>
----------------------------------------------------------------
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] [mynewt-core] andrzej-kaczmarek commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-681096565
my proposal to fix this would be as follows:
- add smth like `HAL_RESET_OTHER = 256` for any reset reason that is specific for MCU, that means it neither maps to common list nor it makes sense to add it to that list (e.g. CMAC watchdog)
- `hal_reset_cause` can either just return `HAL_RESET_OTHER` for such reason or if more mappings are requires, it can return `HAL_RESET_OTHER + 1`, `HAL_RESET_OTHER + 2` and so on; the behavior whether to return just `HAL_RESET_OTHER` for all reasons or more detailed values could be potentially configurable via syscfg, although I don't think this is really necessary
- `sys/reboot` code should use some proper function instead of ugly `REBOOT_REASON_STR` to stringify reset reason - for known mappings it will return proper string, for others it can return hex value or smth so it can be logged properly
this way we do not bloat reset reason line with too many symbols, but also allow to return some more info if needed. would like to hear what other thinks about above idea, perhaps there's something smarter to do here
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-685883633
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### sys/reboot/include/reboot/log_reboot.h
<details>
```diff
@@ -27,15 +27,15 @@
#define REBOOT_REASON_STR(reason) \
(reason == HAL_RESET_POR ? "HARD" : \
- (reason == HAL_RESET_PIN ? "RESET_PIN" : \
- (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
- (reason == HAL_RESET_SOFT ? "SOFT" : \
- (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
- (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
- (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
- (reason == HAL_RESET_DFU ? "DFU" : \
- (reason == HAL_RESET_OTHER ? "OTHER" : \
- "UNKNOWN")))))))))
+ (reason == HAL_RESET_PIN ? "RESET_PIN" : \
+ (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
+ (reason == HAL_RESET_SOFT ? "SOFT" : \
+ (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
+ (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
+ (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
+ (reason == HAL_RESET_DFU ? "DFU" : \
+ (reason == HAL_RESET_OTHER ? "OTHER" : \
+ "UNKNOWN")))))))))
struct log_reboot_info {
enum hal_reset_reason reason;
```
</details>
----------------------------------------------------------------
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] [mynewt-core] andrzej-kaczmarek commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-681758676
you do not need to remove `REBOOT_REASON_STR`
----------------------------------------------------------------
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] [mynewt-core] JustineKH commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
JustineKH commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-681173556
@andrzej-kaczmarek
I agree that this list of reboot reasons could get out of control if we keep adding non-standard reasons. Could we just add a callback function for the reason and SYSCFG to enable it? Seems that some users could be directly calling `REBOOT_REASON_STR`, and expect it to exist.
----------------------------------------------------------------
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] [mynewt-core] andrzej-kaczmarek commented on a change in pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on a change in pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#discussion_r480176190
##########
File path: sys/reboot/src/log_reboot.c
##########
@@ -383,6 +384,50 @@ reboot_conf_export(void (*func)(char *name, char *val),
return 0;
}
+void
+log_reboot_reason_str(enum hal_reset_reason reason, char *buf)
+{
+ char str_reason[MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE)];
Review comment:
use `static` buffer with fixed size
##########
File path: sys/reboot/src/log_reboot.c
##########
@@ -383,6 +384,50 @@ reboot_conf_export(void (*func)(char *name, char *val),
return 0;
}
+void
+log_reboot_reason_str(enum hal_reset_reason reason, char *buf)
+{
+ char str_reason[MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE)];
+
+ if (reason >= HAL_RESET_OTHER) {
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"OTHER: 0x%X",reason - HAL_RESET_OTHER);
+ strcpy(buf, str_reason);
+ return;
+ }
+
+ switch (reason) {
+ case HAL_RESET_POR:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"HARD");
+ break;
+ case HAL_RESET_PIN:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"RESET_PIN");
+ break;
+ case HAL_RESET_WATCHDOG:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"WDOG");
+ break;
+ case HAL_RESET_SOFT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SOFT");
+ break;
+ case HAL_RESET_BROWNOUT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"BROWNOUT");
+ break;
+ case HAL_RESET_REQUESTED:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"REQUESTED");
+ break;
+ case HAL_RESET_SYS_OFF_INT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SYSTEM_OFF_INT");
+ break;
+ case HAL_RESET_DFU:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"DFU");
+ break;
+ default:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"UNKNOWN");
Review comment:
would be better to stringify this like `"UNKNOWN (%d)"` so actual value is also logged
##########
File path: sys/reboot/src/log_reboot.c
##########
@@ -383,6 +384,50 @@ reboot_conf_export(void (*func)(char *name, char *val),
return 0;
}
+void
+log_reboot_reason_str(enum hal_reset_reason reason, char *buf)
+{
+ char str_reason[MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE)];
+
+ if (reason >= HAL_RESET_OTHER) {
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"OTHER: 0x%X",reason - HAL_RESET_OTHER);
+ strcpy(buf, str_reason);
+ return;
+ }
+
+ switch (reason) {
+ case HAL_RESET_POR:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"HARD");
Review comment:
you can just return e.g. `"HARD"` for plain strings
for values that have to be converted to string just do `snprintf` into static buffer of fixed size which is large enough to hold that string and return pointer to that buffer - it's safe as long as it is 'static`
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-682273941
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/mcu/dialog/da1469x/src/hal_system.c
<details>
```diff
@@ -53,7 +53,7 @@
if (reg & CRG_TOP_RESET_STAT_REG_PORESET_STAT_Msk) {
if (reg & CRG_TOP_RESET_STAT_REG_SWD_HWRESET_STAT_Msk) {
- g_hal_reset_reason = HAL_RESET_PIN;
+ g_hal_reset_reason = HAL_RESET_PIN;
}
if (reg & CRG_TOP_RESET_STAT_REG_HWRESET_STAT_Msk) {
g_hal_reset_reason = HAL_RESET_POR;
```
</details>
#### sys/reboot/include/reboot/log_reboot.h
<details>
```diff
@@ -27,15 +27,15 @@
#define REBOOT_REASON_STR(reason) \
(reason == HAL_RESET_POR ? "HARD" : \
- (reason == HAL_RESET_PIN ? "RESET_PIN" : \
- (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
- (reason == HAL_RESET_SOFT ? "SOFT" : \
- (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
- (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
- (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
- (reason == HAL_RESET_DFU ? "DFU" : \
- (reason == HAL_RESET_OTHER ? "OTHER" : \
- "UNKNOWN")))))))))
+ (reason == HAL_RESET_PIN ? "RESET_PIN" : \
+ (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
+ (reason == HAL_RESET_SOFT ? "SOFT" : \
+ (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
+ (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
+ (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
+ (reason == HAL_RESET_DFU ? "DFU" : \
+ (reason == HAL_RESET_OTHER ? "OTHER" : \
+ "UNKNOWN")))))))))
struct log_reboot_info {
enum hal_reset_reason reason;
```
</details>
#### sys/reboot/src/log_reboot.c
<details>
```diff
@@ -396,33 +396,33 @@
}
switch (reason) {
- case HAL_RESET_POR:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"HARD");
- break;
- case HAL_RESET_PIN:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"RESET_PIN");
- break;
- case HAL_RESET_WATCHDOG:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"WDOG");
- break;
- case HAL_RESET_SOFT:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SOFT");
- break;
- case HAL_RESET_BROWNOUT:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"BROWNOUT");
- break;
- case HAL_RESET_REQUESTED:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"REQUESTED");
- break;
- case HAL_RESET_SYS_OFF_INT:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SYSTEM_OFF_INT");
- break;
- case HAL_RESET_DFU:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"DFU");
- break;
- default:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"UNKNOWN");
- break;
+ case HAL_RESET_POR:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"HARD");
+ break;
+ case HAL_RESET_PIN:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"RESET_PIN");
+ break;
+ case HAL_RESET_WATCHDOG:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"WDOG");
+ break;
+ case HAL_RESET_SOFT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SOFT");
+ break;
+ case HAL_RESET_BROWNOUT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"BROWNOUT");
+ break;
+ case HAL_RESET_REQUESTED:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"REQUESTED");
+ break;
+ case HAL_RESET_SYS_OFF_INT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SYSTEM_OFF_INT");
+ break;
+ case HAL_RESET_DFU:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"DFU");
+ break;
+ default:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"UNKNOWN");
+ break;
}
strcpy(buf, str_reason);
```
</details>
----------------------------------------------------------------
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] [mynewt-core] JustineKH edited a comment on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
JustineKH edited a comment on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-681173556
@andrzej-kaczmarek
I agree that this list of reboot reasons could get out of control if we keep adding non-standard reasons. Could we just add a callback function for returning the reason string, and a SYSCFG to enable it? Seems that some users could be directly calling `REBOOT_REASON_STR`, and expect it to exist.
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-677833505
<!-- style-bot -->
## Style check summary
#### No suggestions at this 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] [mynewt-core] apache-mynewt-bot commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-682273941
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/mcu/dialog/da1469x/src/hal_system.c
<details>
```diff
@@ -53,7 +53,7 @@
if (reg & CRG_TOP_RESET_STAT_REG_PORESET_STAT_Msk) {
if (reg & CRG_TOP_RESET_STAT_REG_SWD_HWRESET_STAT_Msk) {
- g_hal_reset_reason = HAL_RESET_PIN;
+ g_hal_reset_reason = HAL_RESET_PIN;
}
if (reg & CRG_TOP_RESET_STAT_REG_HWRESET_STAT_Msk) {
g_hal_reset_reason = HAL_RESET_POR;
```
</details>
#### sys/reboot/include/reboot/log_reboot.h
<details>
```diff
@@ -27,15 +27,15 @@
#define REBOOT_REASON_STR(reason) \
(reason == HAL_RESET_POR ? "HARD" : \
- (reason == HAL_RESET_PIN ? "RESET_PIN" : \
- (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
- (reason == HAL_RESET_SOFT ? "SOFT" : \
- (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
- (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
- (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
- (reason == HAL_RESET_DFU ? "DFU" : \
- (reason == HAL_RESET_OTHER ? "OTHER" : \
- "UNKNOWN")))))))))
+ (reason == HAL_RESET_PIN ? "RESET_PIN" : \
+ (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
+ (reason == HAL_RESET_SOFT ? "SOFT" : \
+ (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
+ (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
+ (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
+ (reason == HAL_RESET_DFU ? "DFU" : \
+ (reason == HAL_RESET_OTHER ? "OTHER" : \
+ "UNKNOWN")))))))))
struct log_reboot_info {
enum hal_reset_reason reason;
```
</details>
#### sys/reboot/src/log_reboot.c
<details>
```diff
@@ -396,33 +396,33 @@
}
switch (reason) {
- case HAL_RESET_POR:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"HARD");
- break;
- case HAL_RESET_PIN:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"RESET_PIN");
- break;
- case HAL_RESET_WATCHDOG:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"WDOG");
- break;
- case HAL_RESET_SOFT:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SOFT");
- break;
- case HAL_RESET_BROWNOUT:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"BROWNOUT");
- break;
- case HAL_RESET_REQUESTED:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"REQUESTED");
- break;
- case HAL_RESET_SYS_OFF_INT:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SYSTEM_OFF_INT");
- break;
- case HAL_RESET_DFU:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"DFU");
- break;
- default:
- snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"UNKNOWN");
- break;
+ case HAL_RESET_POR:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"HARD");
+ break;
+ case HAL_RESET_PIN:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"RESET_PIN");
+ break;
+ case HAL_RESET_WATCHDOG:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"WDOG");
+ break;
+ case HAL_RESET_SOFT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SOFT");
+ break;
+ case HAL_RESET_BROWNOUT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"BROWNOUT");
+ break;
+ case HAL_RESET_REQUESTED:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"REQUESTED");
+ break;
+ case HAL_RESET_SYS_OFF_INT:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"SYSTEM_OFF_INT");
+ break;
+ case HAL_RESET_DFU:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"DFU");
+ break;
+ default:
+ snprintf(str_reason,MYNEWT_VAL(REBOOT_LOG_REBOOT_REASON_SIZE),"UNKNOWN");
+ break;
}
strcpy(buf, str_reason);
```
</details>
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-677833505
<!-- style-bot -->
## Style check summary
#### No suggestions at this 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] [mynewt-core] andrzej-kaczmarek merged pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek merged pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#issuecomment-677871520
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/mcu/dialog/da1469x/src/hal_system.c
<details>
```diff
@@ -53,7 +53,7 @@
if (reg & CRG_TOP_RESET_STAT_REG_PORESET_STAT_Msk) {
if (reg & CRG_TOP_RESET_STAT_REG_HWRESET_STAT_Msk) {
- g_hal_reset_reason = HAL_RESET_PIN;
+ g_hal_reset_reason = HAL_RESET_PIN;
}
if (reg & CRG_TOP_RESET_STAT_REG_HWRESET_STAT_Msk) {
g_hal_reset_reason = HAL_RESET_POR;
```
</details>
#### sys/reboot/include/reboot/log_reboot.h
<details>
```diff
@@ -27,16 +27,16 @@
#define REBOOT_REASON_STR(reason) \
(reason == HAL_RESET_POR ? "HARD" : \
- (reason == HAL_RESET_PIN ? "RESET_PIN" : \
- (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
- (reason == HAL_RESET_SOFT ? "SOFT" : \
- (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
- (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
- (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
- (reason == HAL_RESET_DFU ? "DFU" : \
- (reason == HAL_RESET_WATCHDOG2 ? "WDOG2" : \
- (reason == HAL_RESET_SWD ? "SWD" : \
- "UNKNOWN"))))))))))
+ (reason == HAL_RESET_PIN ? "RESET_PIN" : \
+ (reason == HAL_RESET_WATCHDOG ? "WDOG" : \
+ (reason == HAL_RESET_SOFT ? "SOFT" : \
+ (reason == HAL_RESET_BROWNOUT ? "BROWNOUT" : \
+ (reason == HAL_RESET_REQUESTED ? "REQUESTED" : \
+ (reason == HAL_RESET_SYS_OFF_INT ? "SYSTEM_OFF_INT" : \
+ (reason == HAL_RESET_DFU ? "DFU" : \
+ (reason == HAL_RESET_WATCHDOG2 ? "WDOG2" : \
+ (reason == HAL_RESET_SWD ? "SWD" : \
+ "UNKNOWN"))))))))))
struct log_reboot_info {
enum hal_reset_reason reason;
```
</details>
----------------------------------------------------------------
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] [mynewt-core] andrzej-kaczmarek commented on a change in pull request #2359: hw/mcu/dialog: Add all reset reason checks
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on a change in pull request #2359:
URL: https://github.com/apache/mynewt-core/pull/2359#discussion_r474240221
##########
File path: hw/hal/include/hal/hal_system.h
##########
@@ -72,6 +72,10 @@ enum hal_reset_reason {
HAL_RESET_SYS_OFF_INT = 7,
/** Restart due to DFU */
HAL_RESET_DFU = 8,
+ /** Reserved 1 */
+ HAL_RESET_WATCHDOG2 = 9,
Review comment:
this file is common for all MCUs and imo it does not really need to have unique mapping for every single reset reason for each MCU. in this case, CMAC watchdog could be just mapped to `HAL_RESET_WATCHDOG`. in generic case, it would be just better to add `HAL_RESET_OTHER` and map any reset reason that does not map to defined value - MCU can provide `mcu_reset_cause` to read actual reset reason and appropriate `#define`s in `mcu.h`.
----------------------------------------------------------------
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