Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arch:riscv64:basic porting for C906. #3001

Merged
merged 1 commit into from Mar 10, 2021
Merged

arch:riscv64:basic porting for C906. #3001

merged 1 commit into from Mar 10, 2021

Conversation

hotislandn
Copy link
Contributor

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)

@acassis
Copy link
Contributor

acassis commented Mar 8, 2021

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.

@hotislandn
Copy link
Contributor Author

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!

@Ouss4
Copy link
Member

Ouss4 commented Mar 8, 2021

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.

@Ouss4
Copy link
Member

Ouss4 commented Mar 8, 2021

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.

@btashton
Copy link
Contributor

btashton commented Mar 8, 2021

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.

@hotislandn
Copy link
Contributor Author

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!

@hotislandn
Copy link
Contributor Author

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!

Copy link
Contributor

@btashton btashton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple minor comments on some copyright headers but looks good to me otherwise as a great starting point

arch/risc-v/src/c906/hardware/c906_clint.h Outdated Show resolved Hide resolved
arch/risc-v/src/c906/c906_clockconfig.c Outdated Show resolved Hide resolved
arch/risc-v/src/c906/Make.defs Outdated Show resolved Hide resolved
Signed-off-by: hotislandn <hotislandn@hotmail.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 5e50938 into apache:master Mar 10, 2021
@hotislandn hotislandn deleted the rv64gc_c906_base_porting branch March 10, 2021 13:24
@btashton btashton added this to To-Add in Release Notes - 10.1.0 Apr 12, 2021
@jerpelea jerpelea moved this from To-Add to arch in Release Notes - 10.1.0 Apr 13, 2021
@jerpelea jerpelea moved this from arch to Added in Release Notes - 10.1.0 Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants