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 2022/03/30 07:46:54 UTC
[GitHub] [incubator-nuttx-apps] XinStellaris opened a new pull request #1111: apps/nsh:add resetcause command
XinStellaris opened a new pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111
Signed-off-by: 田昕 <ti...@xiaomi.com>
## Summary
add resetcause nsh command, this will call boardctl to read last reset cause
## Impact
N/A
## Testing
Vela tested
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] pkarashchenko commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838298703
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
I think Mr. @xiaoxiang781216 means that it is expected that all fields of the structure are filled by `boardctl(BOARDIOC_RESET_CAUSE, &cause);` call in case of success
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] pkarashchenko commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838375781
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -319,6 +319,26 @@ int cmd_reboot(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
}
#endif
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+
+ memset(&cause, 0, sizeof(cause));
+
+ ret = boardctl(BOARDIOC_RESET_CAUSE, &cause);
Review comment:
minor style
```suggestion
memset(&cause, 0, sizeof(cause));
ret = boardctl(BOARDIOC_RESET_CAUSE, &cause);
```
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -319,6 +319,26 @@ int cmd_reboot(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
}
#endif
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+
+ memset(&cause, 0, sizeof(cause));
+
+ ret = boardctl(BOARDIOC_RESET_CAUSE, &cause);
+ if (ret < 0)
+ {
+ nsh_error(vtbl, g_fmtcmdfailed, argv[0], "boardctl", NSH_ERRNO);
+ return ERROR;
+ }
+
+ nsh_output(vtbl, "cause:0x%x, flag:0x%x\n", cause.cause, cause.flag);
Review comment:
```suggestion
nsh_output(vtbl, "cause:0x%x, flag:0x%" PRIx32 "\n", cause.cause, cause.flag);
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838236258
##########
File path: nshlib/nsh_command.c
##########
@@ -586,6 +586,10 @@ static const struct cmdmap_s g_cmdmap[] =
#ifndef CONFIG_NSH_DISABLE_XD
{ "xd", cmd_xd, 3, 3, "<hex-address> <byte-count>" },
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+ { "resetcause", cmd_reset_cause, 1, 1, NULL },
Review comment:
let's keep the command in the order.
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
Review comment:
keep the order too.
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
+
+ ret = boardctl(BOARDIOC_RESET_CAUSE, &cause);
+ if (ret < 0)
+ {
+ nsh_error(vtbl, g_fmtcmdfailed, argv[0], "boardctl", NSH_ERRNO);
+ return ERROR;
+ }
+ nsh_output(vtbl, "cause:%x, flag:%x\n", cause.cause, cause.flag);
Review comment:
should we map cause to string xxx(yy)?
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
don't need memset
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838311142
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
Is it right to assume board-logic implementation will meet our expectation of filling all the fields?
I would suggest any application developer to memset to all-zero before calling api.
Need your advices, dont know what 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838359850
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
+
+ ret = boardctl(BOARDIOC_RESET_CAUSE, &cause);
+ if (ret < 0)
+ {
+ nsh_error(vtbl, g_fmtcmdfailed, argv[0], "boardctl", NSH_ERRNO);
+ return ERROR;
+ }
+ nsh_output(vtbl, "cause:%x, flag:%x\n", cause.cause, cause.flag);
Review comment:
I'd prefer to make it as thin as possible.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#issuecomment-1083135655
@xiaoxiang781216 Need your review and approval.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] pkarashchenko merged pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838357362
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
+
+ ret = boardctl(BOARDIOC_RESET_CAUSE, &cause);
+ if (ret < 0)
+ {
+ nsh_error(vtbl, g_fmtcmdfailed, argv[0], "boardctl", NSH_ERRNO);
+ return ERROR;
+ }
+ nsh_output(vtbl, "cause:%x, flag:%x\n", cause.cause, cause.flag);
Review comment:
My intention is: this is just a helper cmd to test whether reset cause works.
It may be useful in developing stage, And a developer should know those causes and flags.
I don't want to increase .rodata size or code size for this helper cmd.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838286894
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
I dont get it.cause is an instance of struct boardioc_reset_cause_s, and some struct members are not used in some cases.
Shouldn't I memset it to 0 to avoid uninitialized value ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838720350
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -319,6 +319,26 @@ int cmd_reboot(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
}
#endif
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+
+ memset(&cause, 0, sizeof(cause));
+ ret = boardctl(BOARDIOC_RESET_CAUSE, &cause);
+ if (ret < 0)
+ {
+ nsh_error(vtbl, g_fmtcmdfailed, argv[0], "boardctl", NSH_ERRNO);
+ return ERROR;
+ }
+
+ nsh_output(vtbl, "cause:0x%x, flag:0x" PRIx32 "\n", cause.cause,
+ cause.flag);
Review comment:
could you format to:
```
nsh_output(vtbl, "cause: 0x%x, flag: 0x" PRIx32 "\n",
cause.cause, cause.flag);
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] pkarashchenko commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838298703
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
I think Mr. @xiaoxiang781216 means that it is expected that all fields of the structure are filled by `boardctl(BOARDIOC_RESET_CAUSE, &cause);` call.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838311142
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
Is it right to assume board-logic implementation to meet our expectation of filling all the fields?
I would suggest any application developer to memset to all-zero before calling api.
Need your advices, dont know what 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] pkarashchenko commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838324608
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
Let's keep the `memset` to follow the same is it is done for `BOARDIOC_UNIQUEID` https://github.com/apache/incubator-nuttx-apps/blob/850607862473be636e2f02dd36fcc779b1bebefe/system/adb/adb_banner.c#L46-L47
Also the struct is fully inited before calling `boardctl(BOARDIOC_NXTERM, (uintptr_t)&nxcreate);`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#discussion_r838321485
##########
File path: nshlib/nsh_syscmds.c
##########
@@ -518,3 +518,21 @@ int cmd_uname(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
return OK;
}
#endif
+
+#if defined(CONFIG_BOARDCTL_RESET_CAUSE) && !defined(CONFIG_NSH_DISABLE_RESET_CAUSE)
+int cmd_reset_cause(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
+{
+ int ret;
+ struct boardioc_reset_cause_s cause;
+ memset(&cause, 0, sizeof(cause));
Review comment:
Ok, it's fine to keep zero, but please add a blank line between the variable declaration and the execute statement.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] pkarashchenko commented on pull request #1111: apps/nsh:add resetcause command
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #1111:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1111#issuecomment-1083158844
I approved the CI run. Let's wait it to finish.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org