8000 bootloaders: fix bootloader button logic by dylad · Pull Request #19365 · RIOT-OS/RIOT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

bootloaders: fix bootloader button logic #19365

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 1 commit into from
Mar 8, 2023
Merged

bootloaders: fix bootloader button logic #19365

merged 1 commit into from
Mar 8, 2023

Conversation

dylad
Copy link
Member
@dylad dylad commented Mar 7, 2023

Contribution description

In lastest master, the BTN_BOOTLOADER_INVERTED logic doesn't work as expected.
This PR fixes the underlying logic by replacing the BTN_BOOTLOADER_INVERTED macro definition by a runtime function.
In fact the current code:

#ifndef BTN_BOOTLOADER_INVERTED
#if (BTN0_MODE == GPIO_IN_PD)
#define BTN_BOOTLOADER_INVERTED false
#else
#define BTN_BOOTLOADER_INVERTED true
#endif
#endif

cannot work because both BTN0_MODE and GPIO_IN_PD are not known by the precompiler as they are enum values defined at cpu level.
Thus, replaces it by a runtime function in our bootloader applications.
I've also add GPIO_OD_PU along side GPIO_IN_PU and add a new define (which can be override at board level or app level) in case an external pullup is used.

Testing procedure

Flash the riotboot_dfu bootloader:
make BOARD=saml21-xpro -C bootloaders/riotboot_dfu flash
Then, flash any test app:
PROGRAMMER=dfu-util USEMODULE=usbus_dfu make BOARD=saml21-xpro -C tests/shell riotboot/flash-slot0

With master, the application will not start.
With this PR, the application will start after flashing.

Issues/PRs references

Fixes #19364

@dylad dylad requested review from gschorcht and chrysn March 7, 2023 20:06
@github-actions github-actions bot added the Area: sys Area: System label Mar 7, 2023
@dylad dylad force-pushed the pr/dfu/fix_bootloader_button branch from f43d431 to 51406e6 Compare March 8, 2023 08:28
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 8, 2023
@riot-ci
Copy link
riot-ci commented Mar 8, 2023 •
edited
Loading

Murdock results

✔️ PASSED

48b07eb bootloaders: fix bootloader button logic

Success Failures Total Runtime
6882 0 6882 11m:22s

Artifacts

@benpicco
Copy link
Contributor
benpicco commented Mar 8, 2023

Please squash!

@dylad
bootloaders: fix bootloader button logic
48b07eb 
Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
@dylad dylad force-pushed the pr/dfu/fix_bootloader_button branch from 5ebee92 to 48b07eb Compare March 8, 2023 12:08
@benpicco
Copy link
Contributor
benpicco commented Mar 8, 2023

Reminds me that we should eventually combine all those riotboot 'applications' into a single one so we can also easily combine e.g. riotboot_slot and riotboot_dfu for both OTA and direct updates if desired.

bors merge

@bors
Copy link
Contributor
bors bot commented Mar 8, 2023

👎 Rejected by too few approved reviews

benpicco
benpicco approved these changes Mar 8, 2023
View reviewed changes
Copy link
Contributor
@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

bors merge

@RIOT-OS RIOT-OS deleted a comment from bors bot Mar 8, 2023
bors bot added a commit that referenced this pull request Mar 8, 2023
@bors @dylad
Merge #19365
5a05ad6 
19365: bootloaders: fix bootloader button logic r=benpicco a=dylad

### Contribution description

In lastest master, the `BTN_BOOTLOADER_INVERTED` logic doesn't work as expected.
This PR fixes the underlying logic by replacing the `BTN_BOOTLOADER_INVERTED` macro definition by a runtime function.
In fact the current code:
```
#ifndef BTN_BOOTLOADER_INVERTED
#if (BTN0_MODE == GPIO_IN_PD)
#define BTN_BOOTLOADER_INVERTED false
#else
#define BTN_BOOTLOADER_INVERTED true
#endif
#endif
```
cannot work because both `BTN0_MODE` and `GPIO_IN_PD` are not known by the precompiler as they are enum values defined at cpu level.
Thus, replaces it by a runtime function in our bootloader applications.
I've also add `GPIO_OD_PU` along side `GPIO_IN_PU` and add a new define (which can be override at board level or app level) in case an external pullup is used.

### Testing procedure
Flash the riotboot_dfu bootloader:
`make BOARD=saml21-xpro -C bootloaders/riotboot_dfu flash`
Then, flash any test app:
`PROGRAMMER=dfu-util USEMODULE=usbus_dfu make BOARD=saml21-xpro -C tests/shell riotboot/flash-slot0`

With master, the application will not start.
With this PR, the application will start after flashing.

### Issues/PRs references
Fixes #19364 


Co-authored-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
@benpicco
Copy link
Contributor
benpicco commented Mar 8, 2023

bors cancel
bors merge

@bors
Copy link
Contributor
bors bot commented Mar 8, 2023

Canceled.

@benpicco
Copy link
Contributor
benpicco commented Mar 8, 2023

bors merge

@bors
Copy link
Contributor
bors bot commented Mar 8, 2023

Already running a review

@dylad
Copy link
Member Author
dylad commented Mar 8, 2023

Reminds me that we should eventually combine all those riotboot 'applications' into a single one so we can also easily combine e.g. riotboot_slot and riotboot_dfu for both OTA and direct updates if desired.

Sounds good to me ;)

@bors
Copy link
Contributor
bors bot commented Mar 8, 2023

Build succeeded:

@bors bors bot merged commit 1a5cc2a into RIOT-OS:master Mar 8, 2023
@dylad dylad deleted the pr/dfu/fix_bootloader_button branch March 8, 2023 14:56
@dylad
Copy link
Member Author
dylad commented Mar 8, 2023

Thanks for the review.

@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@gschorcht gschorcht Awaiting requested review from gschorcht

@chrysn chrysn Awaiting requested review from chrysn

Assignees
No one assigned
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Milestone
Release 2023.04
Development

Successfully merging this pull request may close these issues.

bootloader: BTN_BOOTLOADER_INVERTED doesn't work as expected.
4 participants
@dylad @riot-ci @benpicco @MrKevinWeiss
0