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/24 09:01:33 UTC
[GitHub] [incubator-nuttx] jlaitine opened a new pull request #5323: Fix opensbi build
jlaitine opened a new pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323
This reverts commits which break opensbi building for PX4; our build environment doesn't run patches on top of it; also, we want to use upstream opensbi and not execute any patches on it.
Also, update the opensbi to the newer upstream version, which allows compiling with more strict compiler warning settings.
--
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 #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323
--
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] eenurkka commented on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
eenurkka commented on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1020048687
I guess what happens is that if one modifies mpfs_opensbi.c -file, and builds, the OpenSBI patch has no #defined values yet, so the conflict is opposite: those #ifndefs would need to be in both headers...
--
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] pkarashchenko commented on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1019984215
Ok. I will work with opensbi team to apply changes and restore compatibility.
--
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] jlaitine commented on a change in pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#discussion_r790755306
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
#include <sbi/riscv_io.h>
+#include <sbi/riscv_encoding.h>
Review comment:
sbi mode macros such as PRV_S come from there
--
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] pkarashchenko commented on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1020057641
I filled https://github.com/riscv-software-src/opensbi/issues/243
--
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] jlaitine commented on a change in pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#discussion_r790755704
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
Review comment:
true
##########
File path: arch/risc-v/src/opensbi/Kconfig
##########
@@ -9,4 +9,4 @@ config OPENSBI
default n
---help---
Enable or disable Open Source Supervisor Binary Interface (OpenSBI) features
- for RISC-V.
+ for RISC-V.
Review comment:
ok
##########
File path: arch/risc-v/src/Makefile
##########
@@ -38,10 +38,9 @@ INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)chip}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)common}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)$(ARCH_SUBDIR)}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(TOPDIR)$(DELIM)sched}
-ifeq ($(CONFIG_OPENSBI),y)
-INCLUDES += $(shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)opensbi)
+#ifeq ($(CONFIG_OPENSBI),y)
INCLUDES += $(shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)opensbi$(DELIM)opensbi-3rdparty$(DELIM)include)
-endif
+#endif
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] jlaitine commented on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
jlaitine commented on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1020116670
I will remove the unnecessary includes, and push a cleanup patch on top.
Let's continue the cleanup after reaction from opensbi people
--
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] eenurkka edited a comment on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
eenurkka edited a comment on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1020048687
I guess what happens is that if one modifies mpfs_opensbi.c -file, and builds, the OpenSBI patch has no #defined values yet, so the conflict is opposite: those #ifndefs would need to be in both headers... non-clean build fails or the warnings are present also with my setup.
--
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] eenurkka edited a comment on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
eenurkka edited a comment on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1020048687
I guess what happens is that if one modifies mpfs_opensbi.c -file, and builds, the OpenSBI patch has no #defined values yet, so the conflict is opposite: those #ifndefs would need to be in both headers... non-clean build fails also with my setup.
--
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] pkarashchenko edited a comment on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
pkarashchenko edited a comment on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1020051647
> I guess what happens is that if one modifies mpfs_opensbi.c -file, and builds, the OpenSBI patch has no #defined values yet, so the conflict is opposite: those #ifndefs would need to be in both headers... non-clean build fails or the warnings are present also with my setup.
Yes. I'm filling the issue in opensbi github and will start a discussion. Probably the best way will be to add `SBI_` or `OPEN_SBI_` prefix, so the build will be clean.
What I'm pointing here in comments there seems like there are many headers included that really are not needed for `mpfs_opensbi.c` to build
--
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] pkarashchenko commented on pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#issuecomment-1020051647
>
Yes. I'm filling the issue in opensbi github and will start a discussion. Probably the best way will be to add `SBI_` or `OPEN_SBI_` prefix, so the build will be clean.
What I'm pointing here in comments there seems like there are many headers included that really are not needed for `mpfs_opensbi.c` to build
--
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] jlaitine commented on a change in pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#discussion_r790752876
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
#include <sbi/riscv_io.h>
+#include <sbi/riscv_encoding.h>
#include <sbi/sbi_hart.h>
#include <sbi/sbi_console.h>
#include <sbi/sbi_platform.h>
+#include <sbi/sbi_domain.h>
#include <sbi/sbi_timer.h>
#include <sbi/sbi_init.h>
+#include <sbi/sbi_scratch.h>
Review comment:
struct sbi_scratch comes from there
--
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] jlaitine commented on a change in pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#discussion_r790776997
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
Review comment:
sure
--
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] pkarashchenko commented on a change in pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#discussion_r790640923
##########
File path: arch/risc-v/src/Makefile
##########
@@ -38,10 +38,9 @@ INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)chip}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)common}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)$(ARCH_SUBDIR)}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(TOPDIR)$(DELIM)sched}
-ifeq ($(CONFIG_OPENSBI),y)
-INCLUDES += $(shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)opensbi)
+#ifeq ($(CONFIG_OPENSBI),y)
INCLUDES += $(shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)opensbi$(DELIM)opensbi-3rdparty$(DELIM)include)
-endif
+#endif
Review comment:
```suggestion
endif
```
##########
File path: arch/risc-v/src/Makefile
##########
@@ -38,10 +38,9 @@ INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)chip}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)common}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)$(ARCH_SUBDIR)}
INCLUDES += ${shell $(INCDIR) "$(CC)" $(TOPDIR)$(DELIM)sched}
-ifeq ($(CONFIG_OPENSBI),y)
-INCLUDES += $(shell $(INCDIR) "$(CC)" $(ARCH_SRCDIR)$(DELIM)opensbi)
+#ifeq ($(CONFIG_OPENSBI),y)
Review comment:
```suggestion
ifeq ($(CONFIG_OPENSBI),y)
```
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
Review comment:
please check, but I think it is not needed here
##########
File path: arch/risc-v/src/opensbi/Kconfig
##########
@@ -9,4 +9,4 @@ config OPENSBI
default n
---help---
Enable or disable Open Source Supervisor Binary Interface (OpenSBI) features
- for RISC-V.
+ for RISC-V.
Review comment:
Lets keep this change
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
#include <sbi/riscv_io.h>
+#include <sbi/riscv_encoding.h>
#include <sbi/sbi_hart.h>
#include <sbi/sbi_console.h>
#include <sbi/sbi_platform.h>
+#include <sbi/sbi_domain.h>
Review comment:
please check, but I think it is not needed here
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
#include <sbi/riscv_io.h>
+#include <sbi/riscv_encoding.h>
#include <sbi/sbi_hart.h>
#include <sbi/sbi_console.h>
#include <sbi/sbi_platform.h>
+#include <sbi/sbi_domain.h>
#include <sbi/sbi_timer.h>
#include <sbi/sbi_init.h>
+#include <sbi/sbi_scratch.h>
Review comment:
please check, but I think it is not needed here
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
#include <sbi/riscv_io.h>
+#include <sbi/riscv_encoding.h>
Review comment:
please check, but I think it is not needed here
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
Review comment:
```suggestion
#include "riscv_arch.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.
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] jlaitine commented on a change in pull request #5323: Fix opensbi build
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5323:
URL: https://github.com/apache/incubator-nuttx/pull/5323#discussion_r790753788
##########
File path: arch/risc-v/src/mpfs/mpfs_opensbi.c
##########
@@ -23,27 +23,36 @@
****************************************************************************/
#include <nuttx/config.h>
-
#include <assert.h>
#include <errno.h>
#include <stdint.h>
-
-#include "riscv_internal.h"
-#include "riscv_arch.h"
+#include <riscv_arch.h>
#include <hardware/mpfs_plic.h>
#include <hardware/mpfs_memorymap.h>
#include <hardware/mpfs_clint.h>
#include <hardware/mpfs_sysreg.h>
+/* OpenSBI will also define NULL. Undefine NULL in order to avoid warning:
+ * 'warning: "NULL" redefined'
+ */
+
+#ifdef NULL
+ #undef NULL
+#endif
+
#include <sbi/sbi_types.h>
#include <sbi/riscv_atomic.h>
+#include <sbi/riscv_asm.h>
#include <sbi/riscv_io.h>
+#include <sbi/riscv_encoding.h>
#include <sbi/sbi_hart.h>
#include <sbi/sbi_console.h>
#include <sbi/sbi_platform.h>
+#include <sbi/sbi_domain.h>
Review comment:
that's right, not needed
--
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