Skip to content

gpio: gd32: initial support #40676

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Jan 5, 2022
Merged

gpio: gd32: initial support #40676

merged 14 commits into from
Jan 5, 2022

Conversation

@gmarull
Copy link
Member

@gmarull gmarull commented Nov 25, 2021 •
edited
Loading

This PR introduces a GPIO driver for GigaDevice SoCs, including AF (e.g. F4XX) and AFIO (e.g. F403) variants.

Tested on:

@gmarull gmarull requested a review from nandojve November 25, 2021 23:00
@gmarull
Copy link
Member Author

gmarull commented Nov 25, 2021

@cameled FYI

@gmarull gmarull mentioned this pull request Nov 25, 2021
Open
55 tasks
@nandojve nandojve added this to the v3.0.0 milestone Nov 25, 2021
nandojve
nandojve requested changes Nov 26, 2021
View reviewed changes
@cameled
Copy link
Contributor

cameled commented Dec 7, 2021

Hi @gmarull, can we add pin default value setting for output mode? like:

zephyr/drivers/gpio/gpio_nrfx.c

Line 214 in 38c42f6

if ((flags & GPIO_OUTPUT) != 0) {

spi cs_control need this feature to disable the slave device by default.

@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2021

Codecov Report

Merging #40676 (38c42f6) into main (f8f2936) will increase coverage by 0.08%.
The diff coverage is n/a.

❗ Current head 38c42f6 differs from pull request most recent head 4fd78c3. Consider uploading reports for the commit 4fd78c3 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main   #40676      +/-   ##
==========================================
+ Coverage   51.12%   51.20%   +0.08%     
==========================================
  Files         604      604              
  Lines       70771    70815      +44     
  Branches    16298    16313      +15     
==========================================
+ Hits        36179    36262      +83     
+ Misses      28642    28577      -65     
- Partials     5950     5976      +26
Impacted Files Coverage Δ
subsys/logging/log_backend_uart.c 61.70% <0.00%> (-34.97%) ⬇️
drivers/timer/native_posix_timer.c 76.47% <0.00%> (-10.20%) ⬇️
subsys/testsuite/ztest/src/ztest_error_hook.c 74.54% <0.00%> (-9.10%) ⬇️
include/spinlock.h 75.00% <0.00%> (-8.34%) ⬇️
subsys/pm/device.c 40.33% <0.00%> (-6.73%) ⬇️
subsys/pm/pm.c 68.26% <0.00%> (-2.21%) ⬇️
kernel/idle.c 91.66% <0.00%> (-2.09%) ⬇️
drivers/timer/hpet.c 85.56% <0.00%> (-1.04%) ⬇️
kernel/thread.c 82.14% <0.00%> (-0.60%) ⬇️
subsys/fs/littlefs_fs.c 70.12% <0.00%> (-0.56%) ⬇️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8f2936...4fd78c3. Read the comment docs.

cameled
cameled reviewed Dec 8, 2021
View reviewed changes
@github-actions github-actions bot added area: API Changes to public APIs area: Devicetree Binding PR modifies or adds a Device Tree binding area: Tests Issues related to a particular existing or missing test labels Dec 8, 2021
@gmarull
Copy link
Member Author

gmarull commented Dec 8, 2021

Hi @gmarull, can we add pin default value setting for output mode? like:

zephyr/drivers/gpio/gpio_nrfx.c

Line 214 in 38c42f6

if ((flags & GPIO_OUTPUT) != 0) {

spi cs_control need this feature to disable the slave device by default.

done

@gmarull gmarull marked this pull request as ready for review December 8, 2021 18:09
@gmarull gmarull requested a review from nandojve December 8, 2021 18:09
@gmarull
Copy link
Member Author

gmarull commented Dec 8, 2021

@nandojve would appreciate testing on F403, since it uses the AFIO model (implementation not validated on my side)

cameled
cameled reviewed Dec 9, 2021
View reviewed changes
@gmarull gmarull force-pushed the gpio-gd32 branch 2 times, most recently from 9b66572 to c95586c Compare December 9, 2021 09:22
gmarull
gmarull commented Dec 9, 2021
View reviewed changes
@SebastianBoe SebastianBoe removed their request for review December 27, 2021 08:27
@gmarull gmarull force-pushed the gpio-gd32 branch 2 times, most recently from e91e035 to 95362d3 Compare December 29, 2021 22:11
@gmarull
Copy link
Member Author

gmarull commented Dec 29, 2021

Hi @gmarull ,

Added overlay for F403Z-EVAL. You should be able to test it by bridging the 2 DAC pins with a jumper (see overlay details).

Not sure why, GPIO at DAC port fail. My config is: JP12 1-2 and JP7 1-2

I switch to port D 0-1 P2 + P3 which result is:

Running test suite gpio_basic_test
===================================================================
START - test_gpio_port
Validate device GPIOD
Check GPIOD output 0 connected to input 1

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/gpio/gpio_basic_api/src/test_gpio_port.c:99: setup: (v1 & BIT(PIN_IN) not equal to 0)
out low does not read low
 FAIL - test_gpio_port in 0.21 seconds
===================================================================
START - test_gpio_callback_add_remove
callback_2 triggered: 1
callback_1 triggered: 1
callback_2 triggered: 1
callback_2 triggered: 2
FAIL - test_callback_add_remove@113. not trigger callback correctly

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/gpio/gpio_basic_api/src/test_callback_manage.c:235: test_gpio_callback_add_remove: test_callback_add_remove() not equal to TC_PASS

 FAIL - test_gpio_callback_add_remove in 2.432 seconds
===================================================================
START - test_gpio_callback_self_remove
callback_remove_self triggered: 1
callback_1 triggered: 1
callback_1 triggered: 1
callback_1 triggered: 2
FAIL - test_callback_self_remove@174. not trigger remaining callback correctly

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/gpio/gpio_basic_api/src/test_callback_manage.c:241: test_gpio_callback_self_remove: test_callback_self_remove() not equal to TC_PASS

 FAIL - test_gpio_callback_self_remove in 2.534 seconds
===================================================================
START - test_gpio_callback_enable_disable
callback_2 triggered: 1
callback_1 triggered: 1
callback_2 triggered: 1
callback_1 triggered: 1
 PASS - test_gpio_callback_enable_disable in 3.610 seconds
===================================================================
START - test_gpio_callback_variants
callback triggered: 1
OUT init a01, IN cfg 34000, cnt 1
callback triggered: 1
OUT init 600, IN cfg 54000, cnt 1
callback triggered: 1
OUT init 600, IN cfg 5c000, cnt 1
callback triggered: 1
OUT init a01, IN cfg 3c000, cnt 1
Mode 44000 not supported
Mode 24000 not supported
Mode 4c000 not supported
Mode 2c000 not supported
callback triggered: 1
callback triggered: 2
OUT init a01, IN cfg 74000, cnt 2
 PASS - test_gpio_callback_variants in 5.537 seconds
===================================================================
Test suite gpio_basic_test failed.
===================================================================
PROJECT EXECUTION FAILED

Should be fixed now (tested on GD32VF103V-EVAL which has the same GPIO IP).

mnkp
mnkp approved these changes Dec 30, 2021
View reviewed changes
Copy link
Member

@mnkp mnkp left a comment

Choose a reason for hiding this comment

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

Comment to "drivers: gpio: gd32: initial version" mentions "TODO: Add support for interrupts.". However support for interrupts is already implemented. Could be corrected.

@gmarull
Copy link
Member Author

gmarull commented Jan 1, 2022

Comment to "drivers: gpio: gd32: initial version" mentions "TODO: Add support for interrupts.". However support for interrupts is already implemented. Could be corrected.

removed TODO, thanks for spotting

nandojve
nandojve approved these changes Jan 3, 2022
View reviewed changes
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.

GD32F403Z-EVAL all green

  • samples/basic/blinky
  • samples/basic/button
  • tests/drivers/gpio/gpio_basic_api
  • tests/drivers/gpio/gpio_api_1pin

@cfriedt
Copy link
Member

cfriedt commented Jan 4, 2022

@gmarull - can you please rebase and fix merge conflicts?

@gmarull
Copy link
Member Author

gmarull commented Jan 4, 2022

@gmarull - can you please rebase and fix merge conflicts?

done

gmarull added 14 commits January 4, 2022 20:48
@gmarull
dts: bindings: interrupt-controller: add gd32 exti
27533cc 
Add binding for the GigaDevice EXTI interrupt controller.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
dts: arm: gigadevice: gd32f4xx: add support for exti
ceafcac 
Add DT node for EXTI (External Interrupt Controller). Only line
interrupts added for now (used for GPIO interrupts).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
dts: arm: gigadevice: gd32f403: add support for exti
dc81f41 
Add DT node for EXTI (External Interrupt Controller). Only line
interrupts added for now (used for GPIO interrupts).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
drivers: interrupt_controller: initial support for GD32 EXTI
5466c3b 
Add initial support for the GigaDevice External Interrupt Controller.
This driver is required to manage GPIO interrupts. Only EXTI lines 0 to
15 are supported for now (no LVD, RTC, etc.). Driver can be extended in
the future to add support for extra EXTI lines.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
dts: arm: gigadevice: gd32f4xx: fix gpio support
31f0d7d 
- The gpio dt-bindings were not included
- gpio-controller property was not present in gpio nodes
- number of gpio cells was not present in gpio nodes

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
dts: arm: gigadevice: gd32f403: fix gpio support
0b5234c 
- The gpio dt-bindings were not included
- gpio-controller property was not present in gpio nodes
- number of gpio cells was not present in gpio nodes

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
dts: bindings: misc: add gd32 syscfg
7e3dbaf 
Add support for GigaDevice SYSCFG.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
dts: arm: gigadevice: gd32f4xx: add syscfg node
49f27fa 
Add node for the SYSCFG registers (required by the GPIO driver).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
drivers: gpio: gd32: initial version
13c1db9 
Add GPIO driver for Gigadevice SoCs. The driver supports devices with
the AF and AFIO models.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
boards: arm: gd32f450i_eval: add leds and keys
a8cb7e2 
Add definitions for board LEDs and keys now that a GPIO driver exists.
The GPIO driver has also been enabled in the board defconfig, since it
is required to run some basic samples (e.g. blinky).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
boards: arm: gd32f403z_eval: add leds and keys
54a962e 
Add definitions for board LEDs and keys now that a GPIO driver exists.
The GPIO driver has also been enabled in the board defconfig, since it
is required to run some basic samples (e.g. blinky).

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
tests: drivers: gpio: gpio_basic_api: add support for gd32f450i_eval
664996b 
Add DT overlay to run gpio_basic_api test on GD32F450I-EVAL board. In
order to make testing easy a couple of accessible pins have been
selected: PC6 and PC7, the last 2 pins on the DCI camera 2.54" header.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
tests: drivers: gpio: gpio_basic_api: add support for gd32f403z_eval
c270183 
Add DT overlay to run gpio_basic_api test on GD32F403Z-EVAL board. In
order to make testing easy a couple of accessible pins have been
selected: PD0 and PD1 pins exposed via P3 and P2 respectively.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
drivers: pinctrl: gd32: do not disable clock
5145f02 
Disabling clock may conflict with concurrent usage of GPIOs and pins in
AF mode. This can be improved once a proper clock control API is in
place.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@nashif nashif merged commit 6adda40 into zephyrproject-rtos:main Jan 5, 2022
@gmarull gmarull deleted the gpio-gd32 branch January 18, 2022 10:09
@str4t0m str4t0m added the platform: GD32 GigaDevice label Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cameled cameled cameled left review comments

@nandojve nandojve nandojve approved these changes

@mnkp mnkp mnkp approved these changes

@dcpleung dcpleung Awaiting requested review from dcpleung

@nashif nashif Awaiting requested review from nashif

@carlescufi carlescufi Awaiting requested review from carlescufi

@galak galak Awaiting requested review from galak

@mbolivar-nordic mbolivar-nordic Awaiting requested review from mbolivar-nordic

@tejlmand tejlmand Awaiting requested review from tejlmand

@utzig utzig Awaiting requested review from utzig

Assignees

@nandojve nandojve

Labels

area: API Changes to public APIs area: Boards area: Build System area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Documentation area: GPIO area: Interrupt Controller area: Tests Issues related to a particular existing or missing test platform: GD32 GigaDevice

Projects

None yet

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

9 participants

@gmarull @cameled @codecov-commenter @nandojve @cfriedt @mnkp @nashif @zephyrbot @str4t0m