Skip to content

Conversation

@feilongfl
Copy link
Contributor

@feilongfl feilongfl commented Dec 4, 2022

introduce gd32a503v_eval board

This is Gigadevice's first automotive chip with similar peripheral IP to STM32 or GD32.

due to the following requirement of the ECC memory:
image

I enabled CONFIG_PLATFORM_SPECIFIC_INIT and implement z_arm_platform_init function in soc.c to clear all memory in da51427 . Without clear memory, it will fall into ecc nmi exception after power off reboot.
I'm not sure if there is a better way to implement it.I split it from the soc implementation for review purposes.


todo:

  • add documents for this board
  • enable and test other gd32 peripheral
    • pinctrl & gpio
    • rcu
    • USART
    • dac
    • WDT
    • adc
    • SPI

About the IIC driver:
This SOC use a different IP, it changes a lot, so I decided not to implement it in this PR


other gigadevice works:
#38657

@feilongfl feilongfl changed the title Boards: arm: gd32a503v_eval: intruduct gd32a503v_eval board Boards: arm: gd32a503v_eval: intruduce gd32a503v_eval board Dec 4, 2022
@feilongfl feilongfl changed the title Boards: arm: gd32a503v_eval: intruduce gd32a503v_eval board Boards: arm: gd32a503v_eval: introduce gd32a503v_eval board Dec 4, 2022
@zephyrbot zephyrbot added manifest manifest-hal_gigadevice DNM This PR should not be merged (Do Not Merge) labels Dec 4, 2022
@zephyrbot
Copy link

zephyrbot commented Dec 4, 2022

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_gigadevice zephyrproject-rtos/hal_gigadevice@024ed9e zephyrproject-rtos/hal_gigadevice@2994b7d (main) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@feilongfl feilongfl force-pushed the dev-gd32a503v_eval branch 5 times, most recently from 7a5f6b1 to da51427 Compare December 13, 2022 22:16
@feilongfl
Copy link
Contributor Author

@cameled

About FMC, I've checked #42809 , and I think gd32a50x series use GD32 FMC v2, can you confirm if it is correct?


gigadevice manual

image

@cameled
Copy link
Contributor

cameled commented Dec 14, 2022

@cameled

About FMC, I've checked #42809 , and I think gd32a50x series use GD32 FMC v2, can you confirm if it is correct?

gigadevice manual

image

Yes, it is.

@feilongfl feilongfl force-pushed the dev-gd32a503v_eval branch 2 times, most recently from 2062bbd to 4fd90e8 Compare December 14, 2022 12:47
Comment on lines 10 to 11
Copy link
Member

Choose a reason for hiding this comment

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

Two things to take in consideration:

  1. Is it valuable check if zephyr,sram would be better option.
  2. Is it possible to use memset ?

Copy link
Contributor Author

@feilongfl feilongfl Dec 14, 2022

Choose a reason for hiding this comment

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

Is it valuable check if zephyr,sram would be better option.

fixed

Is it possible to use memset ?

I’ve tried this, and got NMI exception at startup.

void z_arm_platform_init(void)
{
	memset((void *)DT_REG_ADDR(DT_INST(0, SRAM)), 0, DT_REG_SIZE(DT_INST(0, SRAM)));
}

I think the reason is memset store data by char, but memory want written in 32bit.

void *memset(void *buf, int c, size_t n)
{
	unsigned char *d_byte = (unsigned char *)buf;
	unsigned char c_byte = (unsigned char)c;
              /* ...... */
	while (n > 0) {
		*(d_byte++) = c_byte;
		n--;
	}

	return buf;
}

image

image

@feilongfl feilongfl force-pushed the dev-gd32a503v_eval branch 3 times, most recently from 4189506 to 15a87cc Compare December 15, 2022 22:52
@nandojve nandojve mentioned this pull request Dec 22, 2022
55 tasks
@nandojve
Copy link
Member

Hi @feilongfl ,

In general you are doing a very good work. However, you need reorder you commits sequence as:

  1. update hal
  2. DTS
  3. introduce SoC
  4. drivers
    4.1) driver DTS
    4.2) drive itself
    4.3) driver tests
  5. board
  6. samples

Why? If you look drivers: pinctrl: gd32: add gd32a50x support it requires a CONFIG_SOC_SERIES_GD32A50X which is defined later at soc: gd32a50x: introduce gd32a50x soc series. This is an inconsistent on the PR sequence that we need avoid.

I'll wait you reorder the PR sequence to start review : )

@feilongfl feilongfl force-pushed the dev-gd32a503v_eval branch 2 times, most recently from 40493d7 to f5e4f7d Compare December 25, 2022 23:36
@nandojve
Copy link
Member

Hi @feilongfl ,

Please squash following commits:

  1. Below 3 as soc: gd32a50x: introduce gd32a50x soc series
    modules: hal_gigadevice: add defintions for GD32A50X
    soc: gd32a50x: introduce gd32a50x soc series
    soc: gd32a50x: clear ecc memory when system start
  2. Below 2 as drivers: pinctrl: gd32: add gd32a50x support
    include: pinctrl: gd32: add gd32a50x support
    drivers: pinctrl: gd32: add gd32a50x support

@nandojve
Copy link
Member

Hi @feilongfl ,

Please, rebase and fix conflict. After that update west.yml with new hash from hal.

@nandojve nandojve added this to the v3.3.0 milestone Jan 11, 2023
add gd32a50x hal code

Signed-off-by: YuLong Yao <[email protected]>
add gd32a50x series support

Signed-off-by: YuLong Yao <[email protected]>
add support for gd32a50x

Signed-off-by: YuLong Yao <[email protected]>
introduce gd32a50x series

Signed-off-by: YuLong Yao <[email protected]>
soc: gd32a50x: introduce gd32a50x soc series

Signed-off-by: YuLong Yao <[email protected]>
set port speed for `gd32a50x`

Signed-off-by: YuLong Yao <[email protected]>
add gd32a50x support

Signed-off-by: YuLong Yao <[email protected]>
add macro for gd32a50x

Signed-off-by: YuLong Yao <[email protected]>
drop APB marco because clock_control driver is added.

Signed-off-by: YuLong Yao <[email protected]>
add support for gd32a50x series.

Signed-off-by: YuLong Yao <[email protected]>
introduce gd32a503v_eval board

Signed-off-by: YuLong Yao <[email protected]>
add support for gd32a50x

Signed-off-by: YuLong Yao <[email protected]>
add support for gd32a503v_eval board

Signed-off-by: YuLong Yao <[email protected]>
add support for gd32a503v_eval board

Signed-off-by: YuLong Yao <[email protected]>
add support for gd32a503v_eval board.

Signed-off-by: YuLong Yao <[email protected]>
add unit test for gd32a503v_eval

Signed-off-by: YuLong Yao <[email protected]>
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Jan 11, 2023
@nandojve nandojve requested review from cameled and nandojve January 11, 2023 21:53
Copy link
Contributor

@cameled cameled left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @feilongfl !

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.

Good work @feilongfl !

@carlescufi carlescufi merged commit 2017ed5 into zephyrproject-rtos:main Jan 12, 2023
@gmarull
Copy link
Member

gmarull commented Jan 13, 2023

Note: this was merged adding binaries (cmsis-packs) to the module. I don't think this is acceptable, nor Zephyr's responsability, to host CMSIS packs. Please send a PR removing that from upstream Zephyr.

# SPDX-License-Identifier: Apache-2.0
#

CONFIG_GPIO=y
Copy link
Member

Choose a reason for hiding this comment

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

too late, but why doesn't board enable GPIO, as per-policy? this would not be needed at all.

Copy link
Contributor Author

@feilongfl feilongfl Jan 13, 2023

Choose a reason for hiding this comment

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

Sorry, I didn't know about this policy before.
Are all boards supposed to have the following peripherals enabled by default?

CONFIG_CONSOLE=y
CONFIG_UART_CONSOLE=y
CONFIG_SERIAL=y

CONFIG_GPIO=y

Is there anything else I missed?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, in general GPIO/UART are considered "minimal peripherals" used by lots of samples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants