Skip to content

native/mtd: implement .write_page()#15378

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
benpicco:native/mtd_pagewise
Nov 17, 2020
Merged

native/mtd: implement .write_page()#15378
benpicco merged 1 commit intoRIOT-OS:masterfrom
benpicco:native/mtd_pagewise

Conversation

@benpicco
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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