drivers/mtd_mci: fix sub-page writes & sector erase

[benpicco]

Contribution description

Always return the original size in _write_page.
Otherwise when doing sub-page writes, we would return a value that is larger than the amount of bytes written, causing an underflow in the MTD layer.

Testing procedure

Use the write command in tests/mtd_raw on a lpc23xx device with an SD card.

Issues/PRs references

[fabian18]

If size is for example 513 (is this allowed?), then pages is 1, then we write one page (512), but would return 513.
Maybe it must be:

return min(size, pages * SD_HC_BLOCK_SIZE)

[benpicco]

Good catch, thank you!

[fabian18]

I don´t have the hardware to verify but I went through some test cases in my head and I think it works:

offset | size | pages | result
-------------------------------
0       500     0 -> 1   500
1       500     0 -> 1   500
50      500     0 -> 1   462
50      1500    2 -> 1   462
50      1024    2 -> 1   462
0       1024    2        1024
0       1023    1        512
0       1025    2        1024
0       511     0        511
0       513     1        512

[fabian18]

Since I don´t have the hardware, I´ll ACK if you could post the results of above test cases (or similar if not appropriate).
Maybe with another DEBUG() what the function returns. You know, just to make sure ...

[benpicco]

Unfortunately I don't have access to the hardware right now, I only noticed this when implementing the SD host controller for sam0 and using this as a template.

I guess back then I only tested with FAT which does not require such 'advanced' features.
But now we have tests/mtd_raw - maybe @maribu can give it a try if he is in the mood for some retro-computing 😉

[maribu]

Testing using tests/mtd_raw on the msba2 board:

master

main(): This is RIOT! (Version: 2022.07-devel-449-g2e1cd)
Manual MTD test
init MTD_0… OK (31166976 kiB)
> info

> 
> info
mtd devices: 1
test 0
 -=[ MTD_0 ]=-
sectors: 62333952
pages per sector: 1
page size: 512
total: 31166976 kiB
> test 0
[START]
tests/mtd_raw/main.c:410 => failed condition
*** RIOT kernel panic:
CONDITION FAILED.

*** halted.

This PR, rebased on top of master:

main(): This is RIOT! (Version: 2022.07-devel-451-g91fac59-ben)
Manual MTD test
init MTD_0… OK (31166976 kiB)
> info

> 
> info
mtd devices: 1
test 0
 -=[ MTD_0 ]=-
sectors: 62333952
pages per sector: 1
page size: 512
total: 31166976 kiB
> test 0
[START]
tests/mtd_raw/main.c:410 => failed condition
*** RIOT kernel panic:
CONDITION FAILED.

*** halted.

This PR, not rebased on master

main(): This is RIOT! (Version: 2022.04-devel-589-g4694c-ben)
Manual MTD test
init MTD_0… OK (31166976 kiB)
> info

> 
> info
mtd devices: 1
test 0
 -=[ MTD_0 ]=-
sectors: 62333952
pages per sector: 1
page size: 512
total: 31166976 kiB
> test 0
[START]
tests/mtd_raw/main.c:410 => failed condition
*** RIOT kernel panic:
CONDITION FAILED.

*** halted.

[benpicco]

welp, looks like 4694c6c does not work as I had hoped.
Does it work with this commit reverted?

[maribu]

I reverted the top-most commit and now have this commit history:

commit 2f7cf351737e985691ab81b290908ae0dca2d07e (HEAD -> ben)
Author: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Date:   Mon Mar 21 11:52:17 2022 +0100

    drivers/mtd_mci: fix sub-page writes
    
    When doing sub-page writes, we would return a value that is larger
    than the amount of bytes written, causing an underflow in the MTD layer.

commit ce1ec91adc478a46ef0617e44ced5ec469240008
Merge: ae0f617b28 a151a8e66a
Author: Kaspar Schleiser <kaspar@schleiser.de>
Date:   Wed Mar 2 10:59:33 2022 +0100

    Merge pull request #17729 from fjmolinas/pr_gomach_no_periph_rtt_ztimer
    
    sys/net/gnrc: ztimer_no_periph_rtt if gomach
...

The test output now is:

main(): This is RIOT! (Version: 2022.04-devel-588-g2f7cf3-ben)
Manual MTD test
init MTD_0… OK (31166976 kiB)
> info

> 
> info
mtd devices: 1
test 0
 -=[ MTD_0 ]=-
sectors: 62333952
pages per sector: 1
page size: 512
total: 31166976 kiB
> test 0
[START]
tests/mtd_raw/main.c:410 => failed condition
*** RIOT kernel panic:
CONDITION FAILED.

*** halted.

[benpicco]

Arg looks like sector erase has always been broken then.
It's not needed for FAT (mtd_sdcard didn't even implement it until very recently)

[benpicco]

I think 2f7cf35 should still be applied.
I can drop 4694c6c