SD card access and Read Only FAT32 file system support #360
Conversation
| // clang-format off | ||
| #include <ds/xoroshiro.h> | ||
| #include <cheri.hh> | ||
| // clang-format on |
There was a problem hiding this comment.
Why do you turn off clang format for these?
There was a problem hiding this comment.
Copied/mimicked code; it's done in other files. I think we even still have a source file that #includes a single file twice, once inside those comments and once outside!
| static int compare_bytes(const uint8_t *ref, const uint8_t *data, size_t len, Log &log) { | ||
| unsigned mismatches = 0u; | ||
| while (len-- > 0u) { | ||
| mismatches += *ref++ != *data++; |
sw/cheri/tests/sdcard_tests.hh
Outdated
| static uint8_t fileBuffer[BLOCK_LEN]; | ||
|
|
||
| // Dump out a sequence of bytes as hexadecimal and ASCII text. | ||
| static void dump_bytes(const uint8_t *buf, size_t blkBytes, Log &log) { |
There was a problem hiding this comment.
Is this the same as the one in the sdcard check? Can we re-use them somehow?
There was a problem hiding this comment.
There are presently 3 copies, and one has a comment about moving this into console.hh; I'll move it there unless you can think of a better home?
There was a problem hiding this comment.
I've moved it to console.hh as proposed.
| // Check each of the read bytes against what we tried to write. | ||
| failures += compare_bytes(write_data, read_data, kBlockLen, log); | ||
| } | ||
| write_test_result(log, failures); |
There was a problem hiding this comment.
Is this function only used for tests, or can it also be used in a check?
There was a problem hiding this comment.
There is precedent for using it in checks and I'm not aware of any reason not to do so.
|
Thanks for this @alees24. I'm keen we do not spend significant time on this right now so let's not worry about spending time on an SD card DPI model. As we already have the test code it's reasonable for us to include the test in CI to run against the real FPGA. We also need to consider how to to best use this in sonata-software. It's important we have a working example in there, so where shall We could also just say no sd card util at all in sonata-system to simplify things. That would mean no testing of sd card functionality in CI though. I don't think it's that important as it's just a plain SPI device but would mean if we mess up something do with the pinmux/pin mapping it would get flagged. |
|
The DPI model already exists and works; sdcard_tests.hh works in sim and it's even possible to play video files to the simulated LCD; that was done a couple of weeks ago. I guess it wasn't clear that I meant only that it needs a little tidying before inclusion, so it's not in this PR. |
Awesome. May as well leverage it then. re: example in sonata-software I think the thing to do is get this PR merged ignoring that issue. We can then deal with that separately. |
alees24
force-pushed
the
sdcard-access
branch
from
08104a1
to
19c130a
Compare
|
Right, I think this is ready to go; long filename support and directory traversal/matching could use some more testing but it works well enough for our present needs. The DPI model is now present, and although the FPGA system presently has problems, 'test_runner' should pass on this PR when they have been resolved. For simulation, however, we have an issue.....it needs an 'sd.img' file that is ca. 1GiB (see the instructions I've added in 'doc/guide/sdcard-setup.md') which currently won't be present when the simulation runs, presumably....suggestions welcome, please! |
alees24
force-pushed
the
sdcard-access
branch
from
19c130a
to
5ece601
Compare
Note that this manual test will corrupt the contents of the card so it can only be run manually and will intentionally insist that you wilfully insert an SD card before it performs any writes to the card. This is to avoid inadvertent data loss.
SD card test checks for the presence of a microSD card in the slot, reads and reports the card properties and then proceeds to check the contents of a test file `LOREM.IPS` in the root directory of a FAT32-formatted card against its internal reference. See the source for directions on how to set up a microSD card with a suitable file; set 'emitText' to true on the first run and it will emit the sample text with instructions. Support for Long FileNames.
alees24
force-pushed
the
sdcard-access
branch
from
5ece601
to
db37f73
Compare
CI failures are expected; there is no microSD card( image) in
eithersimulationor FPGA testingat present.This PR does the following:
Commit 1:
Commit 2:
Whilst 'sdraw_check' may be run against any modern microSD card of suitable capacity that does not contain data you wish to keep, the test program in the second commit requires that the card be prepared with a suitable file 'LOREM.IPS' (with that exact name and capitalisation at present). Detailed instructions may be found in the source file
sw/cheri/tests/sdcard_tests.hh; in outline, modify that file by setting 'emitText = true' before running it for the first time and it will emit the instructions and the sample text that must be used in setting up the microSD card.Create a single FAT32 partition on the card, using the 'Master Boot Record' scheme and not 'GPT.' Newly-purchased cards should already be suitable although only 32GiB and 64GiB cards have been tried to date.
Still to come, and probably required before we can merge this (to keep the FPGA and simulation consistent):
1. DPI model of a SPI-connected SD card; this allows the above test to pass in simulation too.Still on the 'TODO' list - but perhaps may be left - until a later PR are:
1. Improved filename matching within directories, including Long FileName support.2. A fresh attempt to get multiple block read/write operations working; these are presently implemented using repeated single block reads.