Skip to content

native/mtd: implement .write_page() #15378

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
Nov 17, 2020
Merged

native/mtd: implement .write_page() #15378

merged 1 commit into from
Nov 17, 2020

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Nov 3, 2020

Contribution description

native didn't implement the new pagewise API yet, so unaligned writes / writes across page boundaries will fail.

Testing procedure

Issues/PRs references

#15362 (comment)

@benpicco benpicco requested review from jue89 and vincent-d November 3, 2020 21:46
@benpicco benpicco added Platform: native Platform: This PR/issue effects the native platform Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Nov 3, 2020
@jue89
Copy link
Contributor

jue89 commented Nov 4, 2020 •
edited
Loading

Does it really make sense to implement .write and .write_page separately?

The mtd driver would translate between both worlds if I understand its implementation correctly. So to me this is more or less redundant code - we don't loose functionality if we'd get rid of .write.

@jue89
Copy link
Contributor

jue89 commented Nov 4, 2020

But all in all this PR fixes #15362 (comment) :)

git checkout benpicco/tests/mtd_raw
git checkout -b mtd_raw_test
git merge benpicco/native/mtd_pagewise
make -C tests/mtd_raw BOARD=native all term
(...)
/home/jue/Projects/RIOT/tests/mtd_raw/bin/native/tests_mtd_raw.elf  
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2021.01-devel-547-g4caa95-mtd_raw_test)
Manual MTD test
init MTD_0… OK (8192 kiB)
> test 0
test 0
[START]
[SUCCESS]

@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 Nov 4, 2020
@benpicco
Copy link
Contributor Author

benpicco commented Nov 4, 2020

Heh, looks like the unit test expects mtd_write() to fail on unaligned writes.
I'll move the 2nd commit to the other PR.

@benpicco benpicco force-pushed the native/mtd_pagewise branch from b438449 to 5ecfd67 Compare November 4, 2020 14:54
@benpicco benpicco mentioned this pull request Nov 4, 2020
Merged
5 tasks
bergzand
bergzand reviewed Nov 17, 2020
View reviewed changes
bergzand
bergzand approved these changes Nov 17, 2020
View reviewed changes
Copy link
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

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

Ack! Tested with tests/pkg_littlefs

@benpicco benpicco merged commit dbba4f9 into RIOT-OS:master Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bergzand bergzand bergzand approved these changes

@jue89 jue89 Awaiting requested review from jue89

@vincent-d vincent-d Awaiting requested review from vincent-d

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees
No one assigned
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benpicco @jue89 @bergzand