Skip to content
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.

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

drivers/mtd_spi_nor: fix init when only ztimer_msec is used #19908

Merged
merged 2 commits into from
Sep 2, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Aug 31, 2023

Contribution description

If only ztimer_msec (and not ztimer_usec) is used, the mtd_spi_nor_power() function will not execute the wait_chip_wake_up delay.

This causes mtd_init() to fail (and in a very annoying way: adding debug prints can cause enough of a delay that the init then works again).

Testing procedure

tests/drivers/mtd_raw should work with only ztimer_msec instead of ztimer_usec.

Issues/PRs references

wouldn't have happened if we had #19719

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Aug 31, 2023
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) and removed Area: drivers Area: Device drivers labels Aug 31, 2023
@benpicco benpicco force-pushed the mtd_spi_nor-ztimer_msec branch from f68cde4 to 6e290fb Compare August 31, 2023 12:38
@benpicco benpicco added Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 31, 2023
@benpicco benpicco requested a review from fabian18 August 31, 2023 12:40
@viktorbatista
Copy link
Contributor

Should we not add an #else so in case neither xtimer nor ztimer m/usec are used it outputs an error (or at least a warning of some sorts)?

@riot-ci
Copy link

riot-ci commented Aug 31, 2023

Murdock results

✔️ PASSED

4f2f412 drivers/mtd_spi_nor: improve fallback when no timer is used

Success Failures Total Runtime
7938 0 7939 13m:35s

Artifacts

@fabian18
Copy link
Contributor

If the driver needs either ztimer [ms/us] or xtimer, why is there no such logic in the Makefile.dep?

@benpicco
Copy link
Contributor Author

The idea is to have a polling fallback for e.g. use in riotboot.
But then the number of retries needs to be increased in that case.

Copy link
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

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

ACK

@aabadie
Copy link
Contributor

aabadie commented Sep 2, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 2, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 99f0204 into RIOT-OS:master Sep 2, 2023
@benpicco benpicco deleted the mtd_spi_nor-ztimer_msec branch September 4, 2023 09:57
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers 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.

6 participants