Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cpu/gd32v: fix gpio_read in periph_gpio #19418

Merged
merged 1 commit into from
Mar 24, 2023

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR fixes a bug in gpio_read which made gpio_read completely unusable!

A small bug with big consequences. In gpio_read the combined port | pin_num parameter pin was used instead of the pin number pin_num for the call of _pin_is_input. This caused the problem that for example instead of accessing GPIOA->CTL0 with address 0x40010800, address 0x60018c00 was accessed. As a result, a pin was randomly detected as input or output and thus a result was arbitrarily returned. Approx. 50% of all inputs always returned LOW.

I found this error by coincidence when I tried to find out why the BOOT0 button on a Sipeed Longan Nano is not usable as a button in RIOT.

Testing procedure

Flash tests/periph_gpio

BOARD=sipeed-longan-nano make -j8 -C tests/periph_gpio flash

and use commands

init_in 0 8
read 0 8

Without this PR, the pin is always LOW. With the PR, the pin should be HIGH when the BOOT button is pressed.

Issues/PRs references

@github-actions github-actions bot added the Area: cpu Area: CPU/MCU ports label Mar 24, 2023
@gschorcht gschorcht requested review from bergzand and benpicco March 24, 2023 07:01
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 24, 2023
@riot-ci
Copy link

riot-ci commented Mar 24, 2023

Murdock results

✔️ PASSED

f979730 cpu/gd32v: fix gpio_read in periph_gpio

Success Failures Total Runtime
6882 0 6882 10m:39s

Artifacts

@gschorcht
Copy link
Contributor Author

@MrKevinWeiss This PR should be included in the Hard Freeze because it fixes a small but serious bug.

@benpicco
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 24, 2023

Build succeeded:

@bors bors bot merged commit 50cd32f into RIOT-OS:master Mar 24, 2023
@gschorcht gschorcht deleted the cpu/gd32v/fix_periph_gpio branch March 24, 2023 11:42
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants