Skip to content

drivers/mtd_sdcard: support unaligned reads & writes#17619

Merged
fjmolinas merged 3 commits intoRIOT-OS:masterfrom
benpicco:drivers/mtd_sdcard-unaligned
Feb 17, 2022
Merged

drivers/mtd_sdcard: support unaligned reads & writes#17619
fjmolinas merged 3 commits intoRIOT-OS:masterfrom
benpicco:drivers/mtd_sdcard-unaligned

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Feb 5, 2022 •
edited
Loading

Contribution description

This allows to use SD card as a MTD backed even if users want to do unaligned reads and writes (e.g. SPIFFS, FlashDB).

Testing procedure

The test command from tests/mtd_raw should now work on SD cards.

master 
main(): This is RIOT! (Version: 2022.04-devel-401-g130eb42)
Manual MTD test
init MTD_0… OK (8 MiB)
init MTD_1… OK (256 byte)
init MTD_2… OK (15247 MiB)
> test 2
[START]
tests/mtd_raw/main.c:432 => failed condition
*** RIOT kernel panic:
CONDITION FAILED.

*** halted.
this PR 
 main(): This is RIOT! (Version: 2022.04-devel-221-g91ca7-drivers/mtd_sdcard-unaligned)
Manual MTD test
init MTD_0… OK (0 byte)
init MTD_1… OK (256 byte)
init MTD_2… OK (15247 MiB)
> test 2
[START]
[SUCCESS]

If the SD card is not configured by the board, this requires adding some manual SD card config, e.g. for same54-xpro on EXT2:

diff --git a/tests/mtd_raw/Makefile b/tests/mtd_raw/Makefile
index 37a8d97fe6..2f1a770f45 100644
--- a/tests/mtd_raw/Makefile
+++ b/tests/mtd_raw/Makefile
@@ -7,6 +7,10 @@ USEMODULE += od
 USEMODULE += mtd
 USEMODULE += mtd_write_page
 
+USEMODULE += mtd_sdcard
+CFLAGS += -DSDCARD_SPI_PARAM_SPI="SPI_DEV(1)" -DSDCARD_SPI_PARAM_CS="GPIO_PIN(2,6)"      \
+          -DSDCARD_SPI_PARAM_CLK="GPIO_PIN(2,5)" -DSDCARD_SPI_PARAM_MOSI="GPIO_PIN(2,4)" \
+          -DSDCARD_SPI_PARAM_MISO="GPIO_PIN(2,7)"
 # enable true erase if MTD is an SD card
 CFLAGS += -DCONFIG_MTD_SDCARD_ERASE=1
 
diff --git a/tests/mtd_raw/main.c b/tests/mtd_raw/main.c
index 71681634b4..40979097bf 100644
--- a/tests/mtd_raw/main.c
+++ b/tests/mtd_raw/main.c
@@ -31,6 +31,28 @@
 #include "macros/units.h"
 #include "test_utils/expect.h"
 
+#include "mtd_sdcard.h"
+#include "sdcard_spi_params.h"
+
+/* SD card devices are provided by drivers/sdcard_spi/sdcard_spi.c */
+extern sdcard_spi_t sdcard_spi_devs[ARRAY_SIZE(sdcard_spi_params)];
+
+/* Configure MTD device for the first SD card */
+static mtd_sdcard_t mtd_sdcard_dev = {
+    .base = {
+        .driver = &mtd_sdcard_driver
+    },
+    .sd_card = &sdcard_spi_devs[0],
+    .params = &sdcard_spi_params[0],
+};
+
+static mtd_dev_t *mtd_sd = &mtd_sdcard_dev.base;
+
+#define MTD_2   mtd_sd
+
+#undef MTD_NUMOF
+#define MTD_NUMOF 3
+
 #ifndef MTD_NUMOF
 #ifdef MTD_0
 #define MTD_NUMOF 1

Issues/PRs references

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Feb 5, 2022
@benpicco benpicco force-pushed the drivers/mtd_sdcard-unaligned branch from aa78758 to 91ca757 Compare February 7, 2022 10:40
@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Feb 8, 2022

How should this be tested?

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 8, 2022

Yea I started this in a bit of a WIP state. But the idea is that the automatic test in tests/mtd_raw (and FlashDB) do now work on SD cards.

@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 Feb 9, 2022
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Feb 16, 2022
@benpicco benpicco requested a review from chrysn February 16, 2022 15:57
@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Feb 16, 2022

Without investigating much, this is my initial result:

main(): This is RIOT! (Version: 2022.04-devel-222-g0b4d5-pr-17619)
Manual MTD test
init MTD_0… error: -
> info

> info
mtd devices: 3
test 0
 -=[ MTD_0 ]=-
sectors: 4069
pages per sector: 3969
page size: 3901
total: 2871065617 byte
 -=[ MTD_1 ]=-
sectors: 4069
pages per sector: 3969
page size: 3901
total: 2871065617 byte
 -=[ MTD_2 ]=-
sectors: 61067264
pages per sector: 1
page size: 512
total: 29818 MiB
> test 0
usage: test <dev>
> Timeout in expect script at "child.expect_exact("[START]")" (tests/mtd_raw/tests/01-run.py:19)

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 16, 2022 •
edited
Loading

What are MTD0 and MTD1 in your test?
Looks like the SD card is MTD2, so test 2 should exercise it.

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Feb 16, 2022

What are MTD0 and MTD1 in your test? Looks like the SD card is MTD2, so test 2 should exercise it.

I don't know... I copied your patch for another sdcard setup, I'll re-try tomorrow

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Feb 17, 2022

Hmm I guess in your hardware you have 2 more MTD devices?

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Feb 17, 2022

Hmm I guess in your hardware you have 2 more MTD devices?

I re-tested with NUMOF:

  • master
  • pr
info
> info
mtd devices: 1
test 0
 -=[ MTD_0 ]=-
sectors: 61067264
pages per sector: 1
page size: 512
total: 29818 MiB
> test 0
[START]
tests/mtd_raw/main.c:430 => failed condition
*** RIOT kernel panic:
CONDITION FAILED.

*** halted.


Context before hardfault:
   r0: 0x0000000d
   r1: 0x00000000
   r2: 0x22400000
   r3: 0x00000000
  r12: 0x0000000a
   lr: 0x00000a81
   pc: 0x00000dec
  psr: 0x21000000

FSR/FAR:
 CFSR: 0x00000000
 HFSR: 0x80000000
 DFSR: 0x00000002
 AFSR: 0x00000000
Misc
EXC_RET: 0xffffffed
Active thread: 1 "main"
Attempting to reconstruct state for debugging...
In GDB:
  set $pc=0xdec
  frame 0
  bt

ISR stack overflowed by at least 16 bytes.
main(): This is RIOT! (Version: 2022.04-devel-388-g726c4-HEAD)
Manual MTD test
init MTD_0… OK (29818 MiB)
  • pr
info
> info
mtd devices: 1
test 0
 -=[ MTD_0 ]=-
sectors: 61067264
pages per sector: 1
page size: 512
total: 29818 MiB
> test 0
[START]
[SUCCESS]
>

@fjmolinas fjmolinas added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Feb 17, 2022
fjmolinas
fjmolinas reviewed Feb 17, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

LGTM, only one question.

fjmolinas
fjmolinas approved these changes Feb 17, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

@fjmolinas fjmolinas enabled auto-merge February 17, 2022 09:08
@fjmolinas fjmolinas merged commit e58af6a into RIOT-OS:master Feb 17, 2022
@benpicco benpicco deleted the drivers/mtd_sdcard-unaligned branch February 17, 2022 10:15
@benpicco benpicco mentioned this pull request Feb 26, 2022
Merged
@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fjmolinas fjmolinas fjmolinas approved these changes

@jnohlgard jnohlgard Awaiting requested review from jnohlgard

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

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

@smlng smlng Awaiting requested review from smlng

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

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

@chrysn chrysn Awaiting requested review from chrysn

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 3-testing The PR was tested according to the maintainer guidelines

Projects

None yet

Milestone

Release 2022.04

Development

Successfully merging this pull request may close these issues.

3 participants

@benpicco @fjmolinas @OlegHahm