Skip to content

Conversation

@cameled
Copy link
Contributor

@cameled cameled commented Oct 19, 2021

Add GD32F4xx firmware library V2.1.3.

@cameled
Copy link
Contributor Author

cameled commented Oct 19, 2021

GD32F405/407, GDF450 use the same gd32f4xx firmware library.
GD32F403 have different hardware and software implement.

@cameled cameled force-pushed the gd32f4xx branch 2 times, most recently from 60cbedd to fdc9571 Compare October 19, 2021 11:28
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

  • Please, squash into a single commit
  • Provide the origin download URL for the HAL in the commit message

@gmarull gmarull requested a review from nandojve October 20, 2021 21:22
@nandojve
Copy link
Member

Hi @cameled ,

Thank you for helping us. I few minors to make sure everything is in the right place:

  • It is important make sure you are sending last version. I noted that last one is 2.1.3
  • make sure code uses Linux LF. You can run something like find * type f -print0 | xargs -0 dos2unix -k
  • make sure directory is upper case GD32F4XX and follow current directory structure:
GD32F4XX
  ├── CMSIS
  │   └── GD
  │       └── GD32F4XX
  └── standard_peripheral
      ├── Include
      └── Source
  • You may need adjust timer_init name, see ddde23c
  • You may need adjust some CAN defines, see 223ccbf

@cameled cameled force-pushed the gd32f4xx branch 2 times, most recently from 70c03e0 to 0cf1247 Compare October 21, 2021 03:17
@cameled
Copy link
Contributor Author

cameled commented Oct 21, 2021

Hi @cameled ,

Thank you for helping us. I few minors to make sure everything is in the right place:

* It is important make sure you are sending last version. I noted that last one is  [2.1.3](http://www.gd32mcu.com/download/down/document_id/247/path_type/1)

* make sure code uses Linux LF. You can run something like `find * type f -print0 | xargs -0 dos2unix -k`

* make sure directory is upper case GD32F4XX and follow current directory structure:
GD32F4XX
  ├── CMSIS
  │   └── GD
  │       └── GD32F4XX
  └── standard_peripheral
      ├── Include
      └── Source
* You may need adjust timer_init name, see [ddde23c](https://github.com/zephyrproject-rtos/hal_gigadevice/commit/ddde23cadb1f089263fae2d167e4d83a742d373c)

* You may need adjust some CAN defines, see [223ccbf](https://github.com/zephyrproject-rtos/hal_gigadevice/commit/223ccbfd8ae4bb6b3ada14e4a44995baeefad762)

I have update the GD32F4XX.

  • Upgrade to V2.1.3. (done)
  • Linux LF check. (done)
  • GD32F4XX directory. (done)
  • Rename timer_init function. (done)
  • Fix gd32f4xx_can name conflict. (done)

@cameled
Copy link
Contributor Author

cameled commented Oct 21, 2021

Hi @nandojve
Could you please make some adjust on zephyrproject-rtos/zephyr#38661 SoC firmware library select logic?

@nandojve
Copy link
Member

Hi @nandojve Could you please make some adjust on zephyrproject-rtos/zephyr#38661 SoC firmware library select logic?

If you detected any problem, please, suggest at zephyrproject-rtos/zephyr#38661.

@nandojve
Copy link
Member

Hi @cameled ,

To get this in, we need a PR at Zephyr that build this library with a board. Are you open a PR for F405?
Now that zephyrproject-rtos/zephyr#38661 got merged, it will be easy to send it.

@cameled
Copy link
Contributor Author

cameled commented Oct 29, 2021

Hi @cameled ,

To get this in, we need a PR at Zephyr that build this library with a board. Are you open a PR for F405?

I have a GD32F450I-EVAL board, and gd32f4xx library will work with it.

Now that zephyrproject-rtos/zephyr#38661 got merged, it will be easy to send it.

Thast's great!

@gmarull
Copy link
Member

gmarull commented Oct 29, 2021

FYI I'll open a PR for GD32F450I-EVAL board in the next couple of days, I just need to improve the organization of Kconfig options as current model doesn't fit F4XX (SoC name != HAL name as in F403).

Ref. https://github.com/teslabs/zephyr/tree/gd32f450

@cameled
Copy link
Contributor Author

cameled commented Oct 31, 2021

FYI I'll open a PR for GD32F450I-EVAL board in the next couple of days, I just need to improve the organization of Kconfig options as current model doesn't fit F4XX (SoC name != HAL name as in F403).

Ref. https://github.com/teslabs/zephyr/tree/gd32f450

Hi @gmarull
GD32F450I-EVAL have support on this PR zephyrproject-rtos/zephyr#39909 , review are welcome.

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Download URL is http://www.gd32mcu.com/download/down/document_id/247/path_type/1 (0cf1247 message needs to be adjusted)

GD32F4xx firmware library support GF32F405, GD32F407 and GF32F450.
Origin: http://www.gd32mcu.com/en/download
Filename: GD32F4xx Firmware Library

Signed-off-by: HaiLong Yang <[email protected]>
@cameled
Copy link
Contributor Author

cameled commented Nov 2, 2021

Download URL is http://www.gd32mcu.com/download/down/document_id/247/path_type/1 (0cf1247 message needs to be adjusted)

I prefer the GD32F4 download site, people can easily to know where the gd32f4xx firmware library come from.

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Please, review my comments on conflicting names. Also, please create a section in the HAL README file/s to document which names have conflict and their new names.

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Thanks for the efforts, LGTM!

@carlescufi
Copy link
Member

@nandojve could you please review?

@nandojve
Copy link
Member

nandojve commented Nov 5, 2021

@nandojve could you please review?

Hi @carlescufi , yes, I can start in the weekend. It has been an unprecedented week and I had not have proper time to check Zephyr PRs since last week.

Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

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

Hi @cameled and @gmarull ,

LGTM in overall. However, first we should define what will be the standard that we will follow. Otherwise we can fall into the trap of opinions and create unnecessary conflicts.

In my view, this can be in following what we already have in tree or only after #8 with proper adjusts.

@gmarull
Copy link
Member

gmarull commented Nov 13, 2021

@cameled can you use gd32_/GD32_ prefixes instead of __? Then we should be good to go.

GD32 timer_init function name conflict with Zephyr.
Rename it to gd32_timer_init to temporarily solve the conflict.

Signed-off-by: HaiLong Yang <[email protected]>
GD32 CAN have some name conflict with Zephyr.
Add prefix 'GD32_' to these GD32 CAN macro name.

Signed-off-by: HaiLong Yang <[email protected]>
Zephyr have BIT macro defined in sys/util_macro.h, add a redefine check
for gd32 BIT MACRO.

Signed-off-by: HaiLong Yang <[email protected]>
@cameled
Copy link
Contributor Author

cameled commented Nov 14, 2021

@cameled can you use gd32_/GD32_ prefixes instead of __? Then we should be good to go.

Done.

@gmarull gmarull requested a review from nandojve November 14, 2021 10:21
Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

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

LGTM

@gmarull gmarull merged commit 7bd8904 into zephyrproject-rtos:main Nov 14, 2021
@cameled cameled deleted the gd32f4xx branch November 15, 2021 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants