Add a hal for Nuclei RISC-V core and SoCs

[soburi]

This PR adds support for GigaDevice GD32V SoC and SiPeed Longan Nano board that is use the SoC. Currently, the west module dependency points to https://github.com/soburi/hal_nuclei.

Support: GPIO UART (UART0 only, pins are fixed)

WIP: Support pinctrl. Add SPI and other drivers.

Origin

Nuclei Software Development Kit https://github.com/Nuclei-Software/nuclei-sdk

Fork and modifying to work with ZephyrRTOS https://github.com/soburi/hal_nuclei

Purpose

Driver and Board Support Package for Nuclie RISCV core and SoC

Mode of integration

As External Module. Same as other HAL modules are so.

Pull Request

https://github.com/zephyrproject-rtos/zephyr/pull/34970

(As a part of GigaDevice GD32V support PR)

Description

It is a BSP that is needed to support Nuclei's RISC-V core based SoCs. I sent another PR to support GigaDevice GD32V SoCs. This package is used by the PR.

Dependencies

No other module needs to work on this.

Revision

https://github.com/soburi/hal_nuclei ce2159a6320d9b72ed52a609bcce0fb0e2dd5261

License

Apache-2.0

[nandojve]

I noted that hal_nuclei/SoC is, in fact, GD32VF103 Firmware Library. On my view, a hal_gigadevice should be created. That could aggregate all GigaDevice MCUs at same place, including ARM and RISC-V devices.

GigaDevice uses same peripheral on both ARM and RISC-V (some exceptions like interrupt controller) and all drivers developed to GD32 should be shared between both architectures always possible.

@soburi ,

What do you need from hal_nuclei that is not on hal_nuclei/SoC?

[soburi]

I noted that hal_nuclei/SoC is, in fact, GD32VF103 Firmware Library. On my view, a hal_gigadevice should be created. That could aggregate all GigaDevice MCUs at same place, including ARM and RISC-V devices.

Previously, https://github.com/riscv-mcu/GD32VF103_Firmware_Library is distributed by GigaDevice as SDKs. The repository's README references https://github.com/Nuclei-Software/nuclei-sdk that provides more good support for the SoC. nuclei-sdk seems newer, I select it.

GigaDevice uses same peripheral on both ARM and RISC-V (some exceptions like interrupt controller) and all drivers developed to GD32 should be shared between both architectures always possible.

I agree it.

@soburi ,

What do you need from hal_nuclei that is not on hal_nuclei/SoC?

nuclei/NMSIS/Core contains header files for CPU core related defines such as registers etc. Any other files are not included from CMakeLists.txt.

[galak]

The module repo contains various .a files that should not be allowed.

[carlescufi]

@soburi thanks for the submission. This was discussed in today's Technical Steering Committee meeting, and we had a couple of additional questions for you:

• Are you employed by GigaDevices? If not, do you have contact with them? Being employed by the silicon vendor is not a requirement.
• Are you willing to maintain the port for the foreseeable future? This means reacting to GitHub issues being filed, updating the HAL repo and board support when required and other maintainer-related tasks.
• We noticed that the original HAL repo contains files that are not Apache v2 licensed. Could you run a tool like scancode toolkit and post a summary of the licenses found?
• As @galak mentioned, precompiled libraries are not allowed, so please remove those

[soburi]

@carlescufi,

Thanks for checking this PR.

Are you employed by GigaDevices? If not, do you have contact with them? Being employed by the silicon vendor is not a requirement.

I'm not an employee of GigaDevice. and not in contact with them so well. (I'm just a user of their board and chip. and I want to run Zephyr on it. This chip is released a past year ago but has not to do support Zephyr yet. To me, it looks like GigaDevice has not much attention to Zephyr. So I added to support this chip to Zephyr. I want GigaDevice to contribute to Zephyr!)

Are you willing to maintain the port for the foreseeable future? This means reacting to GitHub issues being filed, updating the HAL repo and board support when required and other maintainer-related tasks.

Yes, I can handle problems and maintain this ports.

We noticed that the original HAL repo contains files that are not Apache v2 licensed. Could you run a tool like scancode toolkit and post a summary of the licenses found?
As @galak mentioned, precompiled libraries are not allowed, so please remove those

Correct the problems and pushed. (https://github.com/soburi/hal_nuclei/commit/7b31adc247673bf12fd655361c479a0d4b551787) Remove static link libraries and files that do not explicitly mention licenses.

I attach to this PR the result of scancode (gziped html). (run scancode -clpeu --classify --summary --verbose --html scancode.html [dir])

scancode.html.gz

[carlescufi]

@soburi thank you very much for the clarifications, the attached scan report and the modifications to the PR. Would you mind removing the doc/ folder from the HAL repo so we avoid the GPL licenses? I do not think we need that in Zephyr. Please attach another report after doing that, and hopefully there will only be Apache and BSD after that.

[carlescufi]

@fanghuaqi thank you for your comment in the PR. Can you state, on behalf of GigaDevice, that you intend to help @soburi maintain this port in the future?

[soburi]

@carlescufi

Would you mind removing the doc/ folder from the HAL repo so we avoid the GPL licenses?

I forgot. I correct it and re-pushed. (https://github.com/soburi/hal_nuclei/commit/b22713f28040fd42be0f533eb444678b9933b9d6)

The result of scancode is here. Only Apache2 and BSD3-Clause are remain.

scancode2.html.gz

[fanghuaqi]

@fanghuaqi thank you for your comment in the PR. Can you state, on behalf of GigaDevice, that you intend to help @soburi maintain this port in the future?

Hi @carlescufi ， I am the maintainer from Nuclei System Technology, not on behalf of GigaDevice, by the way, our doc under doc/ folder are also apache 2.0 license.

If you would like to maintain a HAL for nuclei, I would prefer to maintain it in nuclei-sdk directly, we can discuss which files we need to add?

Thanks Huaqi

[olofj]

I am not sure I understand why we are bringing in full SoCs as HALs + external sources like this now. There are native drivers in Zephyr for lots of SoCs, why does this one need to use out-of-tree drivers like this? They don't integrate into Zephyr natively this way, and just makes it awkward.

It would be a lot easier if you did lightweight ports of the peripherals as native Zephyr drivers. I'd expect that at least some of the peripherals are standard ones, they might even already have working drivers in Zephyr.

[soburi]

@olofj

Thank you for your mention.

Please tell me about your concerns. Is it concern about the overall policy about zephyr? Or is it a problem with this PR?

I wrote the code regarding the existing implementation of other SoCs. If my code violates zephyr's manner, I will correct it. If it is problem with zephyr overall, it is a bit difficult for me to answer shortly. I want to discuss.

Regards.

[nandojve]

I would like suggest a structure to accommodate both ARM and RISC-V:

1) Add a hal_gigadevice This will have all SoC related code, something like this proposal 2) Instead add hal_nuclei or even a sdk_nuclei, I believe the relevant part is NMSIS. NMSIS is pretty close to ARM CMSIS. I think this will better scale since NMSIS is supposed to be used by any other new manufacturer than GigaDevices. 3) sdk_nuclei can use hal_gigadevice as submodule, same with NMSIS. 4) Once all gigadevices are at same repository, as consequence it is possible share any gd32 zephyr drivers between ARM and RISC-V.

This ARM+RISC-V build for both ARM and RISC-V. It can be a prove of concept related to the structure proposed above. The gd32f403z_eval was tested on HW and works. RISC-V part implements the structure but don't have a working code yet. It should be reworked using NMSIS to have a functional code. Docs should be Ignored since it is just a structure proposal.

This is just a structure proposal that, I think, can inspire some ideas and help to move forward.

[soburi]

@nandojve

Thanks for the comments.

I was worried about how source code structure is good for this SoC in coding. The suggestion is sound good.

I pushed sources to hal_nuclei which is a single repository. and after, remove files that are licensed unclearly. As a result, now it contains only NMSIS (headers about Nuclei core) and GigaDevice peripheral driver. I think splitting Nuclei's source and GigaDevice's source will make it easy to maintain.

NMSIS is like an ARM's CMSIS, I think it may good for creating that own directory. GigaDevice peripherals are also used by other GigaDevice's ARM SoCs. It was maybe good to split into hal_gigadevice.

As so result of the above, I think the directory structure will be like this.

modules
 |- hal
    |- nuclei
         |- NMSIS
    |- gigadevice
         |- GD32VF103_Firmware_Library
         |- (other GigaDevice SoC's driver add in future...)

I'll try to it. (https://github.com/soburi/hal_gigadevice)

[fanghuaqi]

Hi， I think a hal for NMSIS is a good idea, we can follow https://github.com/zephyrproject-rtos/cmsis to have a https://github.com/zephyrproject-rtos/nmsis to track https://github.com/Nuclei-Software/NMSIS/

[fanghuaqi]

For https://github.com/soburi/hal_gigadevice, I think you can split the https://github.com/Nuclei-Software/nuclei-sdk/tree/master/SoC/gd32vf103 as part of hal_gigadevice HAL since it used APIs defined in NMSIS, and follow similar projects like https://github.com/zephyrproject-rtos/hal_nxp.

Thanks Huaqi

[soburi]

@fanghuaqi

I think this bit complex situation caused by GD32VF103_Firmware_Library not using NMSIS. So, I modify that library to use NMSIS, and sent PR to GD's repos.

https://github.com/GigaDevice-Semiconductor/GD32VF103_Firmware_Library/pull/2

Once this patch is merged, "hal_gigadevice" will make it easy to separate gd32vf103 drivers, I think. Until so, We need to modify gd32vf103 driver to fit with NMSIS. (Their account seem to be a sleeping. I wonder they can notice this PR...)

[soburi]

I splitting modules by referred to advice from @nandojve, @carlescufi and @fanghuaqi.

https://github.com/soburi/hal_nuclei/tree/nmsis https://github.com/soburi/hal_gigadevice/tree/nmsis (This is in another branch, not push to the PR branch yet.)

It well-separating CPU core-related header and SoC-specific drivers. It is better than my first commit, I think.

Also, I updating the zephyr code along with module changes.

https://github.com/soburi/zephyr/tree/add_basic_support_gd32v_update (It also in another branch, not push to the PR branch yet.)

ScanCode results of the two modules are attached.

scancode.zip

[carlescufi]

@katsuster @cfriedt would you please weigh in on the NMSIS question here?

[cfriedt]

@katsuster @cfriedt would you please weigh in on the NMSIS question here?

I think @nandojve has the right idea. Components seem to be re-usable and are already used with a few SoCs / SoC families, then separate hal_gigadevices and nmsis out and have the nuclei_sdk use those.

However, I do agree with @olofj 's thoughts as well. Will there be a fair amount of code reuse with these modules? Will more SoC's / boards be coming down the pipes that will make it worth the effort? Otherwise, lightweight drivers may be more desirable.

[katsuster]

@katsuster @cfriedt would you please weigh in on the NMSIS question here?

I agree with adding NMSIS and HAL for gigadevice separately.

[carlescufi]

However, I do agree with @olofj 's thoughts as well. Will there be a fair amount of code reuse with these modules? Will more SoC's / boards be coming down the pipes that will make it worth the effort? Otherwise, lightweight drivers may be more desirable.

HALs as a separate repository are fully supported in our current policy. Those are for drivers and potentially some SoC-specific files that are to be shared with bare-metal projects. Major silicon vendors are currently using this model, which has additional benefits for both the vendor and the user. Note that this only includes drivers and SoC-specific code. In no case this includes common architecture code, which belongs in the main zephyr tree under arch/. In the particular case of Arm, we also reuse the CMSIS layer as a module because that is provided by the architecture vendor (Arm itself). This was not the case in earlier versions of Zephyr, but we adopted this model because many of the HALs from vendors were themselves bundling or requiring CMSIS, so we decided to settle on a common version for all those HALs.

In the case of NMSIS, I would make the following statements:

• If it contains generic RISC-V architecture code that applies to all SoCs from all vendors that use RISC-V as an ISA, we shouldn't pull it in, and instead the code should be added to the main tree, under arch/.
• If it contains architecture code that overlaps with what is today in arch/riscv, then we should not pull this in and instead extend arch/riscv
• If it contains non-arch, SoC-specific code that applies only to Nuclei, then I would suggest we pull this together with the HAL, because hal_gigadevices is a repository for everything GD-specific. We do the same in other HALs.

[fanghuaqi]

Hi @carlescufi , about the statements you have made, I need to explain as follows:

1. Nuclei is a company which provide RISC-V Processor IP to customer, Gigadevice use Nuclei's Bumblebee Core(aka N205) RISC-V IP as gd32vf103 SoC's processor core
2. NMSIS was developed by Nuclei, currently only support Nuclei RISC-V processor, it was used to provide Nuclei RISC-V Core HAL such as CSR Registers, TIMER, ECLIC, PMP Registers, DSP Registers(RISC-V P Extension), FPU registers, and Core Access Functions.
3. Previous GD32VF103_Firmware_Library was released before NMSIS, so we have modified the firmware library directly in our Nuclei SDK project https://github.com/Nuclei-Software/nuclei-sdk/tree/master/SoC/gd32vf103, which seperate the RISC-V Core HAL and SoC firmware library
4. Our customer using Nuclei RISC-V Processors now are following the NMSIS standard now, if in future they need to support Zephyr, they may also need to include NMSIS-Core headers, and we are also planning to adding support for our demosoc(fpga based) to Zephyr project, and we are also depending on NMSIS.

Thanks Huaqi

[soburi]

@carlescufi , @fanghuaqi

3. Previous GD32VF103_Firmware_Library was released before NMSIS,

GD32VF103_FirmwareLibrary contains files with unclear licenses. (n200... files in GD32VF103_Firmware_Library) NMSIS has no files with unclear licenses. These files can be replaced by NMSIS's one. I think need to use NMSIS's files from this viewpoint.

This means combining sources from different providers, Nuclei and GigaDevice. Of cource, it can do put into the same directory, but I think a separate directory by each provider easier to maintain. (e.g. hal/nuclei and hal/gigadevice) Also I think, Nuclei provides real hardware to abstract such as interrupt controller, and more. That's a good reason to create a hal / nuclei directory. (This may disagree from a perspective. Difficult ....)

@fanghuaqi, I think NMSIS files no need (or must not) customize by the SoC vendor. Is my understanding is correct? If not so, we need to use these files provided by the SoC vendor.

[kgugala]

Hi @soburi,

I took a deeper look at the NMSIS and hal_gigadevices and I have some comments and questions:

It looks as though what NMSIS provides mostly either duplicates what's already directly included in Zephyr e.g. CSR access convenience macros - for example https://github.com/soburi/hal_nuclei/blob/nmsis/NMSIS/Core/Include/core_feature_base.h#L303-L311 vs https://github.com/zephyrproject-rtos/zephyr/blob/d109805cb2f31d17f4d9a9f8062d746afb038074/include/arch/riscv/csr.h#L183-L189 or convenience macros with various wrappers for many custom CPU instructions - those in turn seem not to be required by the new board port you are adding. Is that correct? Is there anything specific in NMSIS that requires it? Wouldn't it be easier to use what's already included in Zephyr and maybe even extend it if there's indeed something missing?

hal_gigadevice in turn seems to include multiple drivers which are indeed not present in Zephyr at the moment. However they will require proper Zephyr wrappers so maybe it makes sense to port the code to write actual drivers instead (as already suggested in the previous posts)?

[soburi]

Hi @kgugala

Thank you for inspecting the PR.

those in turn seem not to be required by the new board port you are adding. Is that correct? Is there anything specific in NMSIS that requires it? Wouldn't it be easier to use what's already included in Zephyr and maybe even extend it if there's indeed something missing?

NMSIS contains a Nuclei-specific register defines such as an interrupt controller. These are referenced from hal_gigadevice. In this PR, we tried not to change the vendor-supplied code as much as possible.

I think sources under the zephyr directory should use already exists zephyr's definition. If not is so I must correct it. Make to use zephyr's define in hal_gigadevice need work to modify vendor-supplied code. (It maybe undesirable.)

However they will require proper Zephyr wrappers so maybe it makes sense to port the code to write actual drivers instead (as already suggested in the previous posts)?

The short answer is that using vendor-supplied code was the easiest way to get this SoC to work.

Many devices import external open-sourced codes provided by the vendor. It has seemed to me that is a usual method in zephyr.

I think it is follows zephyr's external project policy. https://docs.zephyrproject.org/latest/guides/modules.html#modules-external-projects

But I have no strong reason to use vendor-provided sources. Zephyr seems prefer to be independent of external sources, so I rewrite the PR.

[fanghuaqi]

@carlescufi , @fanghuaqi

1. Previous GD32VF103_Firmware_Library was released before NMSIS,

GD32VF103_FirmwareLibrary contains files with unclear licenses. (n200... files in GD32VF103_Firmware_Library) NMSIS has no files with unclear licenses. These files can be replaced by NMSIS's one. I think need to use NMSIS's files from this viewpoint.

This means combining sources from different providers, Nuclei and GigaDevice. Of cource, it can do put into the same directory, but I think a separate directory by each provider easier to maintain. (e.g. hal/nuclei and hal/gigadevice) Also I think, Nuclei provides real hardware to abstract such as interrupt controller, and more. That's a good reason to create a hal / nuclei directory. (This may disagree from a perspective. Difficult ....)

@fanghuaqi, I think NMSIS files no need (or must not) customize by the SoC vendor. Is my understanding is correct? If not so, we need to use these files provided by the SoC vendor.

No need to be customized by SoC vendor, especially the header files, the only thing need to be customized are the startup/interrupt/exception handing code and link script, which we also provide template files.

If our customer find bugs or need new features, they can open issue or PR to us, and let us improve it, so they can also use the latest version if we provided.

Thanks Huaqi

[fanghuaqi]

Hi @soburi,

I took a deeper look at the NMSIS and hal_gigadevices and I have some comments and questions:

It looks as though what NMSIS provides mostly either duplicates what's already directly included in Zephyr e.g. CSR access convenience macros - for example https://github.com/soburi/hal_nuclei/blob/nmsis/NMSIS/Core/Include/core_feature_base.h#L303-L311 vs

https://github.com/zephyrproject-rtos/zephyr/blob/d109805cb2f31d17f4d9a9f8062d746afb038074/include/arch/riscv/csr.h#L183-L189

or convenience macros with various wrappers for many custom CPU instructions - those in turn seem not to be required by the new board port you are adding. Is that correct? Is there anything specific in NMSIS that requires it? Wouldn't it be easier to use what's already included in Zephyr and maybe even extend it if there's indeed something missing? hal_gigadevice in turn seems to include multiple drivers which are indeed not present in Zephyr at the moment. However they will require proper Zephyr wrappers so maybe it makes sense to port the code to write actual drivers instead (as already suggested in the previous posts)?

Hi @kgugala , NMSIS-Core is a Nuclei RISC-V Core header files defined for our SoC customer to easily to access the cpu features supported by our RISC-V Core, so we need to define the CSR access APIs, of course you can find csr access in Zephyr existing RISC-V supporting code, it might cause some confusions, but maybe customer can include the files they need maybe zephyr one or NMSIS one.

Thanks Huaqi

[nandojve]

I think it is clear that we have two things here:

1) hal_gigadevice 2) NMSIS

In case of [1]

hal_gigadevice in turn seems to include multiple drivers which are indeed not present in Zephyr at the moment. However they will require proper Zephyr wrappers so maybe it makes sense to port the code to write actual drivers instead (as already suggested in the previous posts)?

I would said that we should create a hal_gigadevices. The majority of manufacturers that are supported on Zephyr have their own hal to help. Native drivers are ideal and requires a deep knowledge about both side ( manufacturer IPs and Zephyr driver model ) and HW to test. A driver that manufacturer provide is already well tested and it is easy to create a wrap for it. Because of that I prefer a hal_gigadevice.

About [2]

On my view, it is necessary more talking to understand Zephyr RISC-V shared code vs NMSIS specifics vs NMSIS vendor templates.

@carlescufi Do you think is already possible take a decision about create a hal_gigadevice?

[soburi]

Hi all,

I got some suggestions, so I created and pushed a version that doesn't depend on external modules. (It is prototyping for proceeding with this discussion. I will undo depending on the result of the discussion )

https://github.com/soburi/zephyr/commits/add_basic_support_gd32v

previous push: https://github.com/soburi/zephyr/tree/99c5643d4cb4cf73d3b66644d6489ae6f4173da9

---

GigaDevice Driver can remove by reimplementing register access functions. Pros: No dependence on an external module. Cons: Reimplementing vendor-supplied code may not be good for the quality aspect.

Nuclei's Driver also able to remove, but these definitions are needed. It needs to Incorporate their files or import them as external modules. (It is preferred to import as an external module)

---

In actuality, most of GigaDevice drivers are simple register access functions. It can be replaced by defining the register structure. GigaDevice driver is a thin layer to wrap resigter access, but it is useful to clarify meaning by the function name. The difference between SoCs is the address of the register, but this could be handled well by the DTS mechanism, I think.

As @nandojve says, the vendor-supplied code has been vendor-tested, I think it's preferred to use it from a quality aspect.

The Nuclei's code was an Apache2 license, so I've incorporated the necessary parts. I've omitted unnecessary parts, but I think it's easier to maintain import whole modules or as an external (It has file dependency in modules. It is not preferred to pick only some files. And As @fanghuaqi says, It is no modification for each SoC.)

[carlescufi]

@soburi @fanghuaqi @feilongfl

It seems we have a second proposal for a gigadevices set of ICs that are proposed in this PR and that requires its own HAL. We need to decide if we create a single repo for both, or one for each of those.

[soburi]

@carlescufi @fanghuaqi @feilongfl

https://github.com/feilongfl/hal_gd32 stores to separate directory the source for each SOC. It is the preferable structure. I think it can add gd32vf103's firmware well to it with a bit of modification for coexistence with other SoC's firmware.

It will be like this. (It is similar to proposed by @nanojove)

modules
|- hal
   |- gigadevice (hal_gd32)
       |- devices
             |- gd32e1xx
             |    |- GD32E10x_Firmware_Library_V1.0.3
             |
             | - gd32vf103
             |    |- GD32VF103_Firmware_Library
             |
             |- ( add new device future...)

Nuclei's module can separetly discuss from GigaDevice's hal. It is no necessary at this time. But I think it is prefer to create separate hal module. (It is useful but not fully follows zephyr's coding conventions. Therefore, it is more convenient to use an external module than to import it.)

[fanghuaqi]

Hi, for hal_gd32, it currently contains SoC firmware using ARM core, it we put gd32vf103 library into it, it means this hal will contains firmware for different cpu arch? Is it ok for this case, and of course it will depend on different arch hal such as CMSIS or NMSIS.

Thanks Huaqi

[soburi]

@fanghuaqi @carlescufi

Zephyr's guide has no description about the case which is HAL directory contains various architectures. (From what I saw, it looks like there is no such case so far.)

https://docs.zephyrproject.org/latest/guides/modules.html

In the GigaDevice case, splitting directories with CMakeLists.txt should work well. CMake can easily exclude unused SoC code because the firmware is divided into directories for each SoC.

Zephyr seems to prefer to create modules per vendor. STM32 HAL is splitting from ST HAL, but it seems an unusual case. I think a good way is for every GigaDevice SoC's firmware put into a single external module.

@carlescufi Is there are a convention for the case that is HAL module contains code for various architectures? (or is it acceptable?)

[carlescufi]

@soburi

Is there are a convention for the case that is HAL module contains code for various architectures? (or is it acceptable?)

No, I don't see why that is a problem at all. I think a hal_gigadevice would probably make sense, we already have HALs that cover more than one arch. I have mentioned that here as well.

[soburi]

@carlescufi

Thanks for your opinion and advice. I understand. I thinks the proposal #38657 to make single hal_gigadevice from @nandojve is looks good.

[soburi]

HAL module for GigaDevice is aleady done with #38727. The discussion about NMSIS will be taking over by #39491.

This PR will be close.

[carlescufi]

After a lengthy discussion, we have come to the following conclusions:

• We will not integrate the NMSIS codebase with Zephyr as a module
• Instead, support for Nuclei cores will be added natively in the main tree, in soc/riscv/

This request for an external HAL is therefore no longer needed.