hal: GD32A503: add support for gd32a503 #30

feilongfl · 2022-12-04T05:08:52Z

add gd32a503 support.

cameled · 2022-12-11T14:19:37Z

For the reason to quickly deliver new gd32a503 mcu to user, I accept to add a pack in here.

But there have a high potential that gd32a503 pack will be add to keil in next few month. After that, I suggest to remove the pack from gd32 hal, and update the board document.

nandojve · 2022-12-25T12:29:59Z

Hi @feilongfl ,

I agreed with @cameled about pack. You can point that in the board documentation.
Don't forget to run find * type f -print0 | xargs -0 dos2unix -k for hal: gd32a503: add gd32a503 firmware library 1.0.1 commit since we use unix new lines style.

Once you made the changes move to ready so we can move forward. It will be nice have this new platform for next release but we don't have much time.

add firmware library support for gd32a503. Origin: https://www.gd32mcu.com/data/documents/toolSoftware/GD32A50x_Firmware_Library_V1.0.1.rar Signed-off-by: YuLong Yao <[email protected]>

apply fllowing patches for hal: - drop I2CCLK_MAX/I2CCLK_MIN - add `gd32_` prefix for timer_init - add ifdef to BIT macro - remove nvic_vector_table_set function call - rename CAN_xxx_MODE to GD32_CAN_xxx_MODE Signed-off-by: YuLong Yao <[email protected]>

Add pin configurations for the GD32A503XX series. Information has been taken from datasheet. Signed-off-by: YuLong Yao <[email protected]>

Add autogenerated pinctrl files for GD32A503 series. Signed-off-by: YuLong Yao <[email protected]>

Update common_include headers (using gd32headers.py) Signed-off-by: YuLong Yao <[email protected]>

add device package for pyocd origin: https://www.gd32mcu.com/download/down/document_id/405/path_type/1 Signed-off-by: YuLong Yao <[email protected]>

Fix bug in hal. Signed-off-by: YuLong Yao <[email protected]>

nandojve · 2023-01-07T12:23:55Z

Hi @feilongfl ,

This PR still as Draft. When you think it is ready change status to review.

feilongfl · 2023-01-08T03:40:37Z

About commit 4040618

In past few weeks I've try to communicate with FAE of gigadevice to confirm if it correct.
But they insist that the code in their firmware library is correct.

I think the gigadevice code has the following problem, so I think it should be modified.
But FAE's reply confused me.

gigadevice's code will cause compiler warning:

[64/155] Building C object modules/hal_gigadevice/CMakeFiles/hal_gigadevice.dir/home...ephyr/modules/hal/gigadevice/gd32a50x/cmsis/gd/gd32a50x/source/system_gd32a50x.c.obj
/home/feilong/Program-ext/zephyr/modules/hal/gigadevice/gd32a50x/cmsis/gd/gd32a50x/source/system_gd32a50x.c: In function 'SystemCoreClockUpdate':
/home/feilong/Program-ext/zephyr/modules/hal/gigadevice/gd32a50x/cmsis/gd/gd32a50x/source/system_gd32a50x.c:812:25: warning: bitwise comparison always evaluates to false [-Wtautological-compare]
  812 |         pllmf += ((0xFU == (RCU_CFG0 & RCU_CFG0_PLLMF)) ? 1U : 2U);
      |                         ^~
[110/155] Building C object modules/hal_gigadevice/CMakeFiles/hal_gigadevice.dir/hom...zephyr/modules/hal/gigadevice/gd32a50x/standard_peripheral/source/gd32a50x_rcu.c.obj
/home/feilong/Program-ext/zephyr/modules/hal/gigadevice/gd32a50x/standard_peripheral/source/gd32a50x_rcu.c: In function 'rcu_clock_freq_get':
/home/feilong/Program-ext/zephyr/modules/hal/gigadevice/gd32a50x/standard_peripheral/source/gd32a50x_rcu.c:941:25: warning: bitwise comparison always evaluates to false [-Wtautological-compare]
  941 |         pllmf += ((0xFU == (RCU_CFG0 & RCU_CFG0_PLLMF)) ? 1U : 2U);
      |                         ^~

I've checked the chapter 5.3.2. Configuration register 0 (RCU_CFG0) of GD32A50x User Manual
I think the code here is to handle the case of PLL source clock = 16 or 31
But they forgot to shift 0xF left 18bit

...
   01110: (PLL source clock x 16) 
>  01111: (PLL source clock x 16) 
   10000: (PLL source clock x 17)
...
  11101: (PLL source clock x 30) 
  11110: (PLL source clock x 31) 
> 11111: (PLL source clock x 31)

nandojve · 2023-01-08T10:29:08Z

Hi @feilongfl ,

You are correct. My feeling is that they would like to have something like:

pllmf += ((0xFU == GET_BITS(RCU_CFG0, 18, 21)) ? 1U : 2U);

But I think below will be easy to comprehend:

        pllmf = GET_BITS(RCU_CFG0, 18, 21);
        pllmf += ((0xFU == pllmf) ? 1U : 2U);
        pllmf += ((RCU_CFG0 & RCU_CFG0_PLLMF_4) ? 15U : 0U);

nandojve

About support: gd32a50x: add device pack for pyocd. I may contradict my self a little here but I start to think it will be better just point pack in the board documentation. It is far easy to update board docs than hal.

Sorry about above noise. I was reconsidering and it seems best move this way currently.

cameled · 2023-01-08T15:06:45Z

I write a simple test,

#include <stdint.h>
#include <stdio.h>

#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))

#define BIT(x)                       ((uint32_t)((uint32_t)0x01U<<(x)))
#define BITS(start, end)             ((0xFFFFFFFFUL << (start)) & (0xFFFFFFFFUL >> (31U - (uint32_t)(end)))) 
#define GET_BITS(regval, start, end) (((regval) & BITS((start),(end))) >> (start))

/* RCU_CFG0 */
#define RCU_CFG0_PLLMF                  BITS(18,21)
#define RCU_CFG0_PLLMF_4                BIT(27)

static const uint32_t PLLMF[] = {
        2U,  3U,  4U,  5U,  6U,  7U,  8U,  9U,
        10U, 11U, 12U, 13U, 14U, 15U, 16U, 16U,
        17U, 18U, 19U, 20U, 21U, 22U, 23U, 24U,
        25U, 26U, 27U, 28U, 29U, 30U, 31U, 31U
};

static const uint32_t PLLMF_3_0[] = {
        0b0000, 0b0001, 0b0010, 0b0011, 0b0100, 0b0101, 0b0110, 0b0111,
        0b1000, 0b1001, 0b1010, 0b1011, 0b1100, 0b1101, 0b1110, 0b1111,
        0b0000, 0b0001, 0b0010, 0b0011, 0b0100, 0b0101, 0b0110, 0b0111,
        0b1000, 0b1001, 0b1010, 0b1011, 0b1100, 0b1101, 0b1110, 0b1111
};

static const uint32_t PLLMF_4[] = {
        0b0, 0b0, 0b0, 0b0, 0b0, 0b0, 0b0, 0b0,
        0b0, 0b0, 0b0, 0b0, 0b0, 0b0, 0b0, 0b0,
        0b1, 0b1, 0b1, 0b1, 0b1, 0b1, 0b1, 0b1,
        0b1, 0b1, 0b1, 0b1, 0b1, 0b1, 0b1, 0b1
};

int main(void) {
        for (size_t i = 0; i < ARRAY_SIZE(PLLMF); i++) {
                uint32_t RCU_CFG0 = (PLLMF_3_0[i] << 18) | (PLLMF_4[i] << 27);

                /* PLL multiplication factor */
                uint32_t pllmf = GET_BITS(RCU_CFG0, 18, 21);
                pllmf += ((RCU_CFG0 & RCU_CFG0_PLLMF_4) ? 15U : 0U);
                pllmf += ((0xFU == (RCU_CFG0 & RCU_CFG0_PLLMF)) ? 1U : 2U);

                if (pllmf != PLLMF[i]) {
                        printf("Invalid pllmf: PLLMF[%lu] = 0x%x, pllmf = 0x%x\n", i, PLLMF[i], pllmf);
                }
        }

	return 0;
}

OUTPUT:

Invalid pllmf: PLLMF[15] = 0x10, pllmf = 0x11
Invalid pllmf: PLLMF[31] = 0x1f, pllmf = 0x20

cameled · 2023-01-09T12:45:07Z

Hi @nandojve, can we push the pack description to exception section?

gd32a50x/cmsis/gd/gd32a50x/source/system_gd32a50x.c

nandojve · 2023-01-09T16:58:36Z

Hi @nandojve, can we push the pack description to exception section?

Yes, good idea. I was thinking about to have a <hal_gigadevice>/support/pack folder and than add any required dependency there. What do you folks think about it?

cameled · 2023-01-10T01:20:12Z

I check the gd32 download site. There only have three gd32 series pack not include in Keil MDK5 Software Packs, GD32A50x, GD32C11x and GD32E11x. These gd32 series both added at the end of 2022.

If we add a directory to store these pack, this directory may change frequently.

note: gd32a50x pack not include in English version download site.

nandojve · 2023-01-10T07:01:12Z

Let`s document pack solution and then I think this is ready.

feilongfl · 2023-01-10T10:47:43Z

I check the gd32 download site. There only have three gd32 series pack not include in Keil MDK5 Software Packs, GD32A50x, GD32C11x and GD32E11x. These gd32 series both added at the end of 2022.

If we add a directory to store these pack, this directory may change frequently.

note: gd32a50x pack not include in English version download site.

espressif HAL have a west espressif install command to prepare debug env,
I think we can add a west gigadevice install command to run pyocd pack install xxx and other command to prepare debug env.For test, I create zephyrproject-rtos/zephyr#53658 and #31
For pack not in Keil MDK5 Software Packs, I recommend storage it in <hal_gigadevice>/<gd32xxxx>/support folder until it be uploaded to MDK website.

nandojve · 2023-01-10T15:06:03Z

Hi @feilongfl ,

Please, document the exception following the current proposal c8faa49. Just add an pack entry mentioning how someone should proceed.

We can discuss west extensions in a way to not block proposals.

- gd32a50x have pllmf calculate error, explain how to fix it. - gigadevice not upload pack to mdk website, explain how to storage pack - update arm library structure Signed-off-by: YuLong Yao <[email protected]>

gmarull · 2023-01-13T10:03:44Z

README.md

+    └── support (optional)
+        └── GigaDevice.GD32xxx_DFP.1.0.0.pack (Only for packs not in [Keil MDK5 Software Packs](https://www.keil.com/dd2/pack/))


Please remove binaries from this module (send a new PR dropping this). This is not acceptable, you can provide details on how to download suck packs (not to mention that GD is responsible to host them, and can be installed automatically via cmsis-pack-manager).

I'm trying to make a west extension commands to solve pack problems.(#31 )

I'd like to know is it possible to save the file link in the repository like this.

hal_gigadevice/zephyr/west/gigadevice.yml

Lines 5 to 9 in 2017359

pyocd-packages:

- pack: gd32e103 # pack in mdk website

- pack: gd32a503 # pack not in mdk website

url: https://www.gd32mcu.com/download/down/document_id/405/path_type/1

hash: 23c8840812ef9d86fbb87bca7d9c452c71bd9291

This way we can use commands like west gigadevice install to install all packages or download them from the gigadevice website

IMO GD should be responsible of updating packages in CMSIS repo, so that the standard tool cmsis-pack-manager can be used. In fact, they seem to have most of them (A503 is new, maybe it's not there yet, time to ping vendor?).

@gmarull ,

Why you are saying that the pack file is a binary file? As far I can see it is just a zip with source code and register definitions. You can just unzip and read all the content.

Below is the full content of the file:

. ├── Device │ ├── Firmware │ │ └── Peripherals │ │ ├── inc │ │ │ ├── gd32a50x_adc.h │ │ │ ├── gd32a50x_bkp.h │ │ │ ├── gd32a50x_can.h │ │ │ ├── gd32a50x_cmp.h │ │ │ ├── gd32a50x_crc.h │ │ │ ├── gd32a50x_dac.h │ │ │ ├── gd32a50x_dbg.h │ │ │ ├── gd32a50x_dma.h │ │ │ ├── gd32a50x_exti.h │ │ │ ├── gd32a50x_fmc.h │ │ │ ├── gd32a50x_fwdgt.h │ │ │ ├── gd32a50x_gpio.h │ │ │ ├── gd32a50x_i2c.h │ │ │ ├── gd32a50x_mfcom.h │ │ │ ├── gd32a50x_misc.h │ │ │ ├── gd32a50x_pmu.h │ │ │ ├── gd32a50x_rcu.h │ │ │ ├── gd32a50x_rtc.h │ │ │ ├── gd32a50x_spi.h │ │ │ ├── gd32a50x_syscfg.h │ │ │ ├── gd32a50x_timer.h │ │ │ ├── gd32a50x_trigsel.h │ │ │ ├── gd32a50x_usart.h │ │ │ └── gd32a50x_wwdgt.h │ │ └── src │ │ ├── gd32a50x_adc.c │ │ ├── gd32a50x_bkp.c │ │ ├── gd32a50x_can.c │ │ ├── gd32a50x_cmp.c │ │ ├── gd32a50x_crc.c │ │ ├── gd32a50x_dac.c │ │ ├── gd32a50x_dbg.c │ │ ├── gd32a50x_dma.c │ │ ├── gd32a50x_exti.c │ │ ├── gd32a50x_fmc.c │ │ ├── gd32a50x_fwdgt.c │ │ ├── gd32a50x_gpio.c │ │ ├── gd32a50x_i2c.c │ │ ├── gd32a50x_mfcom.c │ │ ├── gd32a50x_misc.c │ │ ├── gd32a50x_pmu.c │ │ ├── gd32a50x_rcu.c │ │ ├── gd32a50x_rtc.c │ │ ├── gd32a50x_spi.c │ │ ├── gd32a50x_syscfg.c │ │ ├── gd32a50x_timer.c │ │ ├── gd32a50x_trigsel.c │ │ ├── gd32a50x_usart.c │ │ └── gd32a50x_wwdgt.c │ ├── Include │ │ ├── gd32a50x.h │ │ └── system_gd32a50x.h │ ├── Source │ │ ├── ARM │ │ │ └── startup_gd32a50x.s │ │ └── system_gd32a50x.c │ ├── Template │ │ └── gd32a50x_libopt.h │ └── Utilities │ ├── gd32a503v_eval.c │ └── gd32a503v_eval.h ├── Flash │ ├── GD32A50x_128KB.FLM │ ├── GD32A50x_256KB.FLM │ └── GD32A50x_384KB.FLM ├── GigaDevice.GD32A50x_DFP.pdsc └── SVD ├── GD32A503.SFR └── GD32A503.svd

I think the intention was keep easy to users until pack be available into cmsis-pack-manager and after that content will be dropped from here. The major concern was that pack is only available in the Chinese website version and it is not easy to find it if link is broke.

The content is not binary, but a zip file is a binary from a git perspective. Binaries are well known to blow git history (see zephyr with board pictures), and remember Zephyr modules carry all their Git history.

cameled · 2023-01-14T05:51:44Z

https://open-cmsis-pack.github.io/Open-CMSIS-Pack-Spec/main/html/index.html

zephyrbot mentioned this pull request Dec 4, 2022

Boards: arm: gd32a503v_eval: introduce gd32a503v_eval board zephyrproject-rtos/zephyr#52774

Merged

9 tasks

feilongfl mentioned this pull request Dec 6, 2022

scripts: pyocd: Add pack option to support device custom pyocd packages. zephyrproject-rtos/zephyr#52773

Closed

feilongfl force-pushed the dev-gd32a503 branch 3 times, most recently from 2917edd to 1982915 Compare December 14, 2022 12:51

feilongfl added 7 commits December 26, 2022 06:48

hal: gd32a503: add gd32a503 firmware library 1.0.1

3218040

add firmware library support for gd32a503. Origin: https://www.gd32mcu.com/data/documents/toolSoftware/GD32A50x_Firmware_Library_V1.0.1.rar Signed-off-by: YuLong Yao <[email protected]>

hal: gd32a503: apply patches

1a68a87

apply fllowing patches for hal: - drop I2CCLK_MAX/I2CCLK_MIN - add `gd32_` prefix for timer_init - add ifdef to BIT macro - remove nvic_vector_table_set function call - rename CAN_xxx_MODE to GD32_CAN_xxx_MODE Signed-off-by: YuLong Yao <[email protected]>

pinconfigs: gd32a503: add gd32a503 pin configurations

9027bfa

Add pin configurations for the GD32A503XX series. Information has been taken from datasheet. Signed-off-by: YuLong Yao <[email protected]>

dt-bindings: pinctrl: gd32a503: add autogenerated files for GD32A503

d6eb8af

Add autogenerated pinctrl files for GD32A503 series. Signed-off-by: YuLong Yao <[email protected]>

common_include: add GD32A503 support

3b9a5a8

Update common_include headers (using gd32headers.py) Signed-off-by: YuLong Yao <[email protected]>

support: gd32a50x: add device pack for pyocd

c8faa49

add device package for pyocd origin: https://www.gd32mcu.com/download/down/document_id/405/path_type/1 Signed-off-by: YuLong Yao <[email protected]>

hal: gd32a50x: fix pllmf calc error in hal

4040618

Fix bug in hal. Signed-off-by: YuLong Yao <[email protected]>

feilongfl force-pushed the dev-gd32a503 branch from 1982915 to 4040618 Compare December 25, 2022 22:54

feilongfl marked this pull request as ready for review January 8, 2023 03:42

nandojve suggested changes Jan 8, 2023

View reviewed changes

cameled reviewed Jan 9, 2023

View reviewed changes

gd32a50x/cmsis/gd/gd32a50x/source/system_gd32a50x.c Show resolved Hide resolved

feilongfl force-pushed the dev-gd32a503 branch from d72b902 to 25fa471 Compare January 10, 2023 10:29

cameled approved these changes Jan 10, 2023

View reviewed changes

README.md: add pllmf and pack exception for gd32a50x

afbf04e

- gd32a50x have pllmf calculate error, explain how to fix it. - gigadevice not upload pack to mdk website, explain how to storage pack - update arm library structure Signed-off-by: YuLong Yao <[email protected]>

feilongfl force-pushed the dev-gd32a503 branch from 25fa471 to afbf04e Compare January 10, 2023 18:36

nandojve approved these changes Jan 11, 2023

View reviewed changes

nandojve merged commit 2994b7d into zephyrproject-rtos:main Jan 11, 2023

gmarull reviewed Jan 13, 2023

View reviewed changes

		└── support (optional)
		└── GigaDevice.GD32xxx_DFP.1.0.0.pack (Only for packs not in [Keil MDK5 Software Packs](https://www.keil.com/dd2/pack/))

	pyocd-packages:
	- pack: gd32e103 # pack in mdk website
	- pack: gd32a503 # pack not in mdk website
	url: https://www.gd32mcu.com/download/down/document_id/405/path_type/1
	hash: 23c8840812ef9d86fbb87bca7d9c452c71bd9291

hal: GD32A503: add support for gd32a503 #30

hal: GD32A503: add support for gd32a503 #30

Uh oh!

Conversation

feilongfl commented Dec 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cameled commented Dec 11, 2022

Uh oh!

nandojve commented Dec 25, 2022

Uh oh!

nandojve commented Jan 7, 2023

Uh oh!

feilongfl commented Jan 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nandojve commented Jan 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nandojve left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cameled commented Jan 8, 2023

Uh oh!

cameled commented Jan 9, 2023

Uh oh!

Uh oh!

nandojve commented Jan 9, 2023

Uh oh!

cameled commented Jan 10, 2023

Uh oh!

nandojve commented Jan 10, 2023

Uh oh!

feilongfl commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nandojve commented Jan 10, 2023

Uh oh!

gmarull Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

feilongfl Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

gmarull Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

nandojve Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

gmarull Jan 16, 2023

Choose a reason for hiding this comment

Uh oh!

cameled commented Jan 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feilongfl commented Dec 4, 2022 •

edited

Loading

feilongfl commented Jan 8, 2023 •

edited

Loading

nandojve commented Jan 8, 2023 •

edited

Loading

nandojve left a comment •

edited

Loading

feilongfl commented Jan 10, 2023 •

edited

Loading