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.

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

drivers/mtd: fix mtd_write_sector #20234

Merged
merged 1 commit into from
Jan 7, 2024
Merged

drivers/mtd: fix mtd_write_sector #20234

merged 1 commit into from
Jan 7, 2024

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR fixes mtd_write_sector function introduced with PR #20180.

mtd_write_page_raw requires the number of bytes to be written as parameter, which is the number of sectors to be written (parameter count) multiplied by the pages per sector mtd->pages_per_sector and the page size mtd->page_size.

Testing procedure

Use any board with SPI SD Card interface and set ENABLE_DEBUG in drivers/mtd_sdcard.c. Without the PR, writing to the SD card fails because of the wrong size. Formatting the SD card doesn't hangs.

> vfs w /sd0/test1.txt ascii o hello
mtd_sdcard_read_page: page:1991 offset:0 size:512
mtd_sdcard_write_page: page:1991 offset:0 size:1019392
mtd_sdcard_write_page: error 4
Write error: -EIO
mtd_sdcard_write_page: page:1991 offset:0 size:1019392
mtd_sdcard_write_page: error 3

With the PR writing to the SD card and formatting it works as expected.

> vfs w /sd0/test1.txt ascii o hello
mtd_sdcard_read_page: page:1991 offset:0 size:512
mtd_sdcard_read_page: page:95 offset:0 size:512
mtd_sdcard_write_page: page:95 offset:0 size:512
mtd_sdcard_write_page: page:2055 offset:0 size:512
mtd_sdcard_read_page: page:1991 offset:0 size:512
mtd_sdcard_write_page: page:1991 offset:0 size:512
mtd_sdcard_write_page: page:64 offset:0 size:512

Issues/PRs references

Fixes PR #20180

@gschorcht
drivers/mtd: fix mtd_write_sector
caa31ec 
Parameter `num` of `mtd_write_page_raw` is the number of bytes to be written, which is the number of sectors to be written`count` multiplied by the pages per sector `mtd->pages_per_sector` and the page size `mtd->page_size`.
@gschorcht gschorcht requested a review from benpicco January 7, 2024 16:14
@github-actions github-actions bot added the Area: drivers Area: Device drivers label Jan 7, 2024
@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 Jan 7, 2024
@gschorcht gschorcht mentioned this pull request Jan 7, 2024
Merged
@riot-ci
Copy link

riot-ci commented Jan 7, 2024

Murdock results

✔️ PASSED

caa31ec drivers/mtd: fix mtd_write_sector

Success Failures Total Runtime
8085 0 8085 11m:37s

Artifacts

benpicco
benpicco approved these changes Jan 7, 2024
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.

Oops, good catch - thank you!

@benpicco benpicco enabled auto-merge January 7, 2024 16:50
@benpicco benpicco added this pull request to the merge queue Jan 7, 2024
Merged via the queue into RIOT-OS:master with commit dca974d Jan 7, 2024
28 checks passed
@benpicco benpicco added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Jan 8, 2024
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024
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

Assignees
No one assigned
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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2024.01
Development

Successfully merging this pull request may close these issues.
4 participants
@gschorcht @riot-ci @benpicco @MrKevinWeiss