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/01/03 15:24:37 UTC
[GitHub] [incubator-nuttx] no1wudi opened a new pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
no1wudi opened a new pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148
## Summary
## Impact
## Testing
--
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] no1wudi commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777804499
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
OK, but I thinks it's better to keep this Symbol for some reason :
1. SoC still need to specific it's register width
2. This symbol from nuttx/config.h can provide better support for modern editor (like language server in vscode) since '__LP64__' series definition only available in riscv relative toolchain
--
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] no1wudi commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777805844
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -71,9 +79,9 @@
* Name: riscv_stackdump
****************************************************************************/
-static void riscv_stackdump(uint64_t sp, uintptr_t stack_top)
+static void riscv_stackdump(uintptr_t sp, uintptr_t stack_top)
{
- uint64_t stack;
+ uintptr_t stack;
Review comment:
Done
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -98,35 +106,35 @@ static inline void riscv_registerdump(volatile uintptr_t *regs)
{
/* Are user registers available from interrupt processing? */
- _alert("EPC:%016" PRIx64 "\n", regs[REG_EPC]);
- _alert("A0:%016" PRIx64 " A1:%01" PRIx64 "6 A2:%016" PRIx64
- " A3:%016" PRIx64 "\n",
+ _alert("EPC:%0" PRIxREG "\n", regs[REG_EPC]);
+ _alert("A0:%0" PRIxREG " A1:%0" PRIxREG "A2:%0" PRIxREG
+ " A3:%0" PRIxREG "\n",
regs[REG_A0], regs[REG_A1], regs[REG_A2], regs[REG_A3]);
- _alert("A4:%016" PRIx64 " A5:%016" PRIx64 "A6:%016" PRIx64
- " A7:%016" PRIx64 "\n",
+ _alert("A4:%0" PRIxREG " A5:%0" PRIxREG "A6:%0" PRIxREG
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.
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] xiaoxiang781216 commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777803567
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -98,35 +106,35 @@ static inline void riscv_registerdump(volatile uintptr_t *regs)
{
/* Are user registers available from interrupt processing? */
- _alert("EPC:%016" PRIx64 "\n", regs[REG_EPC]);
- _alert("A0:%016" PRIx64 " A1:%01" PRIx64 "6 A2:%016" PRIx64
- " A3:%016" PRIx64 "\n",
+ _alert("EPC:%0" PRIxREG "\n", regs[REG_EPC]);
+ _alert("A0:%0" PRIxREG " A1:%0" PRIxREG "A2:%0" PRIxREG
+ " A3:%0" PRIxREG "\n",
regs[REG_A0], regs[REG_A1], regs[REG_A2], regs[REG_A3]);
- _alert("A4:%016" PRIx64 " A5:%016" PRIx64 "A6:%016" PRIx64
- " A7:%016" PRIx64 "\n",
+ _alert("A4:%0" PRIxREG " A5:%0" PRIxREG "A6:%0" PRIxREG
Review comment:
let's move %0 into PRIxREG?
--
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] no1wudi commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777794145
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
But we still need to specific the output width for rv32 (8) and rv64 (16) by it.
--
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] xiaoxiang781216 commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777802724
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
yes, but we can check `__LP64__` like what's is done in nuttx/arch/risc-v/include/types.h. The final goal is to remove CONFIG_ARCH_RV32 from Kconfig.
And PRIxPTR is better than PRIx32/PRIx64 since the type pass to _alert is uinptr_t.
--
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] xiaoxiang781216 commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777648007
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
let's use PRIxPTR to avoid chceck CONFIG_ARCH_RV32
--
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] xiaoxiang781216 commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777802724
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
yes, but we can check `__LP64__` like what's is done in nuttx/arch/risc-v/include/types.h. The final goal is to remove CONFIG_ARCH_RV32 from Kconfig.
--
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] xiaoxiang781216 commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777802724
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
yes, but we can check __LP64__ like what's is done in nuttx/arch/risc-v/include/types.h. The final goal is to remove CONFIG_ARCH_RV32 from Kconfig.
--
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] xiaoxiang781216 commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777803402
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -71,9 +79,9 @@
* Name: riscv_stackdump
****************************************************************************/
-static void riscv_stackdump(uint64_t sp, uintptr_t stack_top)
+static void riscv_stackdump(uintptr_t sp, uintptr_t stack_top)
{
- uint64_t stack;
+ uintptr_t stack;
Review comment:
need update %016" PRIx64 " at line 93
--
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] no1wudi commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777804499
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
OK
--
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] no1wudi commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777804499
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
OK, but I thinks it's better to keep this Symbol for some reason :
1. SoC still need to specific it's register width
2. This symbol from nuttx/config.h can provide better support for modern editor (like language server in vscode) since `__LP64__` series definition only available in riscv relative toolchain
--
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] no1wudi commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777794145
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
Yes, but we still need to specific the output width for rv32 (8) and rv64 (16) by it.
--
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] xiaoxiang781216 commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777856908
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
OK.
--
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] xiaoxiang781216 merged pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148
--
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] no1wudi commented on a change in pull request #5148: arch/risc-v: Refine riscv_assert.c and arch.h
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5148:
URL: https://github.com/apache/incubator-nuttx/pull/5148#discussion_r777804499
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -57,6 +57,14 @@
# define CONFIG_BOARD_RESET_ON_ASSERT 0
#endif
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "8"PRIx32
Review comment:
OK, but I thinks it's better to keep this Symbol for some reason :
1. SoC still need to specific it's register width
2. This symbol from nuttx/config.h can provide better support for modern editor (like language server in vscode) since __LP64__ series definition only available in riscv relative toolchain
--
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