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 2021/03/08 15:39:57 UTC

[GitHub] [incubator-nuttx] hotislandn opened a new pull request #3001: arch:riscv64:basic porting for C906.

hotislandn opened a new pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001


   ## Summary
   This patch enables Nuttx basic OS functionalities on C906, a RISC-V 64bit SoC that implements RV64GCVX extensions from T-HEAD.
   
   ## Impact
   RV64GC targets.
   
   ## Testing
   1. need to build with the toolchain from T-HEAD website, which enables the specific extentisons
   2. 'ostest' runs on C906 within QEMU(cskysim)
   


----------------------------------------------------------------
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] hotislandn commented on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
hotislandn commented on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793230304


   > I'm a little hesitant to bring in the overhead of the ISA vector extensions that are pre 1.0 (Linux does not support them for this reason). Are there other ISA extensions that are causing issues. To be clear I am not saying no, I just want to make sure we are not signing up for what will be legacy baggage from the start.
   > 
   > When I designed that docker image I did it with the idea in mind that we might want to have multiple targets to keep the size from getting crazy. macOS we probably do not want to burden with a lot of special toolchains, but we already have some fairly reasonable coverage there.
   
   The reason for C906 has to use the vendor-specific toolchain is that T-HEAD adds their own instructions to the "I" extension, not the "V" variant. And, in fact, the "V" spec supported by C906 is 0.7.1.
   I guess we are not going to bring the "V" extension into NuttX at this stage. Anyway, I will try to keep it in accordance with the standard toolchain for this patch.
   Have a nice day!


----------------------------------------------------------------
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] btashton commented on a change in pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#discussion_r591159689



##########
File path: arch/risc-v/src/c906/c906_clockconfig.c
##########
@@ -0,0 +1,86 @@
+/****************************************************************************
+ * arch/risc-v/src/c906/c906_clockconfig.c
+ *
+ * Derives from software originally provided by Canaan Inc
+ *
+ *   Copyright 2018 Canaan Inc

Review comment:
       Nothing from Canaan here this can be dropped.

##########
File path: arch/risc-v/src/c906/hardware/c906_clint.h
##########
@@ -0,0 +1,36 @@
+/****************************************************************************
+ * arch/risc-v/src/c906/hardware/c906_clint.h
+ *
+ * Derives from software originally provided by Canaan Inc
+ *
+ *   Copyright 2018 Canaan Inc

Review comment:
       There is nothing in this file that needs to be attributed to Canaan

##########
File path: arch/risc-v/src/c906/Make.defs
##########
@@ -0,0 +1,61 @@
+############################################################################
+# arch/risc-v/src/c906/Make.defs
+#
+#   Copyright (C) 2019 Masayuki Ishikawa. All rights reserved.

Review comment:
       @masayuki2009 has already filed a ICLA so we can drop this from the license header.




----------------------------------------------------------------
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 #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793137800


   > How to add this dedicated toolchain for the C906 build, or we have to use the standard toolchain but lose any vendor ISA extensions?
   
   You can check arch/risc-v/src/rv32im&rv64gc/Toolchain.defs for how to add a toolchain.   For our CI to be able to build the toolchain needs to be installed in the Docker image as well (https://github.com/apache/incubator-nuttx-testing/blob/master/docker/linux/Dockerfile)
   That said a quick (and maybe dirty) solution right now is to just disable C906 builds (see https://github.com/apache/incubator-nuttx-testing/blob/master/testlist/other.dat#L4-L6)


----------------------------------------------------------------
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] btashton commented on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793143150


   I'm a little hesitant to bring in the overhead of the ISA vector extensions that are pre 1.0 (Linux does not support them for this reason). Are there other ISA extensions that are causing issues.  To be clear I am not saying no, I just want to make sure we are not signing up for what will be legacy baggage from the start.


----------------------------------------------------------------
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] btashton edited a comment on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
btashton edited a comment on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793143150


   I'm a little hesitant to bring in the overhead of the ISA vector extensions that are pre 1.0 (Linux does not support them for this reason). Are there other ISA extensions that are causing issues.  To be clear I am not saying no, I just want to make sure we are not signing up for what will be legacy baggage from the start.
   
   When I designed that docker image I did it with the idea in mind that we might want to have multiple targets to keep the size from getting crazy. macOS we probably do not want to burden with a lot of special  toolchains, but we already have some fairly reasonable coverage 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793141972


   > or we have to use the standard toolchain but lose any vendor ISA extensions?
   
   BTW, you can make the default configuration use the SiFive toolchain that's used by the rest of the RISC-V chips and add a config that uses the vendor's toolchain.  In this case simple examples can be built without the need of another 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] hotislandn commented on a change in pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
hotislandn commented on a change in pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#discussion_r591178307



##########
File path: arch/risc-v/src/c906/hardware/c906_clint.h
##########
@@ -0,0 +1,36 @@
+/****************************************************************************
+ * arch/risc-v/src/c906/hardware/c906_clint.h
+ *
+ * Derives from software originally provided by Canaan Inc
+ *
+ *   Copyright 2018 Canaan Inc

Review comment:
       Those files are derived/copied from K210, let me fix these. 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] Ouss4 edited a comment on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
Ouss4 edited a comment on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793137800


   > How to add this dedicated toolchain for the C906 build, or we have to use the standard toolchain but lose any vendor ISA extensions?
   
   You can check arch/risc-v/src/rv32im&rv64gc/Toolchain.defs for how to add a toolchain.   For our CI to be able to build the toolchain needs to be installed in the Docker image as well (https://github.com/apache/incubator-nuttx-testing/blob/master/docker/linux/Dockerfile)
   That said a quick (and maybe dirty) solution right now is to just disable C906 builds (see https://github.com/apache/incubator-nuttx-testing/blob/master/testlist/other.dat#L4-L6).  Configs beginning with a `-` are ignored.


----------------------------------------------------------------
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 edited a comment on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
Ouss4 edited a comment on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793137800


   > How to add this dedicated toolchain for the C906 build, or we have to use the standard toolchain but lose any vendor ISA extensions?
   
   You can check arch/risc-v/src/rv32im&rv64gc/Toolchain.defs for how to add a toolchain.   For our CI to be able to build, the toolchain needs to be installed in the Docker image as well (https://github.com/apache/incubator-nuttx-testing/blob/master/docker/linux/Dockerfile)
   That said a quick (and maybe dirty) solution right now is to just disable C906 builds (see https://github.com/apache/incubator-nuttx-testing/blob/master/testlist/other.dat#L4-L6).  Configs beginning with a `-` are ignored.


----------------------------------------------------------------
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] hotislandn commented on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
hotislandn commented on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793221775


   > > or we have to use the standard toolchain but lose any vendor ISA extensions?
   > 
   > BTW, you can make the default configuration use the SiFive toolchain that's used by the rest of the RISC-V chips and add a config that uses the vendor's toolchain. In this case simple examples can be built without the need of another toolchain.
   
   Yes, I would like to try this option first, to align with the standard toolchain from SiFive, though we may suffer a bit of performance loss at the beginning. However, this gives better compatibility. 
   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] xiaoxiang781216 merged pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001


   


----------------------------------------------------------------
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 pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-792892046


   Hi @hotislandn thank you very much for adding support on NuttX for Allwinner C906 !
   
   I think you missed to run nxstyle/checkpatch on some files:
   
   ```
   Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:24:1: warning: #include outside of 'Included Files' section
   Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:25:1: warning: #include outside of 'Included Files' section
   Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:26:1: warning: #include outside of 'Included Files' section
   Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:28:1: warning: #include outside of 'Included Files' section
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_idle.c:2:1: error: Too many whitespaces before relative file path
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_lowputc.c:85:63: error: Left bracket not on separate line
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_lowputc.c:85:4: error: Bad alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_lowputc.h:33:3: error: Invalid section for this file type
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_serial.c:263:83: error: Long line found
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_serial.c:310:79: error: Long line found
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_start.c:81:6: error: Missing whitespace after keyword
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/include/board.h:2:1: error: Relative file path does not match actual file
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:31:1: error: Too many blank lines
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:38:1: error: Blank line follows left brace
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:39:1: error: Blank line precedes right brace at line
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:43:1: error: Blank line follows left brace
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:44:1: error: Blank line precedes right brace at line
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:48:1: error: Blank line follows left brace
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:49:1: error: Blank line precedes right brace at line
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/smartl-c906.h:2:1: error: Relative file path does not match actual file
   Error: Process completed with exit code 1.
   ```


----------------------------------------------------------------
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] hotislandn commented on pull request #3001: arch:riscv64:basic porting for C906.

Posted by GitBox <gi...@apache.org>.
hotislandn commented on pull request #3001:
URL: https://github.com/apache/incubator-nuttx/pull/3001#issuecomment-793130539


   > Hi @hotislandn thank you very much for adding support on NuttX for Allwinner C906 !
   > 
   > I think you missed to run nxstyle/checkpatch on some files:
   > 
   > ```
   > Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:24:1: warning: #include outside of 'Included Files' section
   > Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:25:1: warning: #include outside of 'Included Files' section
   > Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:26:1: warning: #include outside of 'Included Files' section
   > Warning: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/include/c906/chip.h:28:1: warning: #include outside of 'Included Files' section
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_idle.c:2:1: error: Too many whitespaces before relative file path
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_lowputc.c:85:63: error: Left bracket not on separate line
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_lowputc.c:85:4: error: Bad alignment
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_lowputc.h:33:3: error: Invalid section for this file type
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_serial.c:263:83: error: Long line found
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_serial.c:310:79: error: Long line found
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/arch/risc-v/src/c906/c906_start.c:81:6: error: Missing whitespace after keyword
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/include/board.h:2:1: error: Relative file path does not match actual file
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:31:1: error: Too many blank lines
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:38:1: error: Blank line follows left brace
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:39:1: error: Blank line precedes right brace at line
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:43:1: error: Blank line follows left brace
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:44:1: error: Blank line precedes right brace at line
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:48:1: error: Blank line follows left brace
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/c906_leds.c:49:1: error: Blank line precedes right brace at line
   > Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/risc-v/c906/smartl-c906/src/smartl-c906.h:2:1: error: Relative file path does not match actual file
   > Error: Process completed with exit code 1.
   > ```
   
   Hi Acassis,
   
   Yes, I failed to do this which lead to those style check failures. I will update them soon.
   BTW, for another 2 build errors, which caused by the wrong toolchain used for C906, it needs a specific toolchain from T-HEAD.
   How to add this dedicated toolchain for the C906 build, or we have to use the standard toolchain but lose any vendor ISA extensions?
   Thanks a lot!


----------------------------------------------------------------
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