sdcard library #12687
Replies: 19 comments 19 replies
-
Yes: You google e.g. "sdcard lib micropython". |
Beta Was this translation helpful? Give feedback.
-
There was no deep knowledge. I just happened to plug in a nice, new SDXC card, and it didn't work. Some print statements showed that it failed as soon as incorrect (zero) CRCs started being sent, so I looked up the CRC7 algorithm.
Speed is still slow, but for a lot of data-logging stuff, maybe OK. At 120 MHz CPU, 40 MHz SPI on my Pi Pico, I get about 600kB/s write speed, and just over 1000 KB/s read speed. Much of this may due to small block sizes and poor buffer handling.
Note that before the changes, I was only getting 65 kB/s read speed, due to some time.sleep() in the read loop.
… On Oct 27, 2023, at 4:46 AM, Raul Kompaß ***@***.***> wrote:
@mendenm: Congratulations, that is good news. I hope you can do a pull request to micropython-lib for that.
Certainly a lot of background knowledge was enabling you to do that.
It would be interesting to learn something more about the new capabilities (what type of cards are read/writable now that weren't before, perhaps benchmarks regarding speed).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
@mendenm: I did a test of your driver. # file write and erase test, includes timing
import machine, os, random, time, sdcard_m
from pyb import SPI, Pin
from time import ticks_ms, ticks_diff
spi = SPI(2, SPI.MASTER, baudrate=400_000, polarity=0, phase=0) # SCK=PB13, MISO=PB14, MOSI=PB15
sd = sdcard_m.SDCard(spi, Pin('PB12'), baudrate=8000000)
os.mount(sd, '/sd')
os.chdir('/sd')
def run(n=10, len=8000):
t_wr = []
t_rd = []
bw = bytearray(len)
br = bytearray(len)
random.seed(0)
for i in range (n):
for j in range(len):
bw[j] = random.getrandbits(8)
name = "file" + str(i)
print("create ", name)
start = ticks_ms()
f = open(name, "wb")
f.write(bw)
f.close()
stop = ticks_ms()
t_wr.append(ticks_diff(stop, start))
for e in os.ilistdir(""):
print(e)
random.seed(0)
for i in range (n):
for j in range(len):
bw[j] = random.getrandbits(8)
name = "file" + str(i)
print("read ", name)
start = ticks_ms()
f = open(name, "rb")
f.readinto(br)
f.close()
stop = ticks_ms()
t_rd.append(ticks_diff(stop, start))
if bw != br:
print("Read mismatch")
input("Next")
for i in range (n):
name = "file" + str(i)
print("delete ", name)
os.remove(name)
for e in os.ilistdir(""):
print(e)
print(t_wr)
print(t_rd) Results:
My observations from this so far are: From a look at your code I see that in initialization there is a slightly better distinction between different card versions. I think the code is worth to be looked after by the experts/maintainers. Seeing the possible improvement by setting a higher SPI baudrate the question occurs: Which maximal baudrate is safe for a certain card type, and how can this type/baudrate be automatically determined? |
Beta Was this translation helpful? Give feedback.
-
good to know it is working out. I have been doing some more general housekeeping on it to reduce code size and increase efficiency. I have been doing all my testing at 24 MHz SPI speed, and even the very old cards I have seem OK with that. I haven't looked into the process of probing cards to see what speed they actually support.
… On Oct 30, 2023, at 5:31 PM, Raul Kompaß ***@***.***> wrote:
@mendenm: I did a test of your driver.
Using the following program (modified for timing, original by @robert-hh), using a Blackpill with SPI2.
# file write and erase test, includes timing
import machine, os, random, time, sdcard_m
from pyb import SPI, Pin
from time import ticks_ms, ticks_diff
spi = SPI(2, SPI.MASTER, baudrate=400_000, polarity=0, phase=0) # SCK=PB13, MISO=PB14, MOSI=PB15
sd = sdcard_m.SDCard(spi, Pin('PB12'), baudrate=8000000)
os.mount(sd, '/sd')
os.chdir('/sd')
def run(n=10, len=8000):
t_wr = []
t_rd = []
bw = bytearray(len)
br = bytearray(len)
random.seed(0)
for i in range (n):
for j in range(len):
bw[j] = random.getrandbits(8)
name = "file" + str(i)
print("create ", name)
start = ticks_ms()
f = open(name, "wb")
f.write(bw)
f.close()
stop = ticks_ms()
t_wr.append(ticks_diff(stop, start))
for e in os.ilistdir(""):
print(e)
random.seed(0)
for i in range (n):
for j in range(len):
bw[j] = random.getrandbits(8)
name = "file" + str(i)
print("read ", name)
start = ticks_ms()
f = open(name, "rb")
f.readinto(br)
f.close()
stop = ticks_ms()
t_rd.append(ticks_diff(stop, start))
if bw != br:
print("Read mismatch")
input("Next")
for i in range (n):
name = "file" + str(i)
print("delete ", name)
os.remove(name)
for e in os.ilistdir(""):
print(e)
print(t_wr)
print(t_rd)
Results:
Filesystem tests with original Micropython-lib sdcard driver
Creating and writing files with length of 4000 bytes (ms):
[160, 157, 157, 159, 157, 157, 157, 159, 157, 182]
Reading of those files (ms):
[62, 63, 63, 63, 63, 63, 63, 63, 63, 63]
Creating and writing files with length of 8000 bytes (ms):
[206, 203, 204, 203, 203, 203, 203, 204, 203, 205]
Reading of those files (ms):
[111, 111, 111, 111, 111, 111, 111, 111, 111, 111]
Filesystem tests with improved sdcard driver
Creating and writing files with length of 8000 bytes (ms):
[188, 186, 189, 186, 186, 189, 186, 186, 189, 186]
Reading of those files (ms):
[100, 100, 100, 100, 100, 100, 100, 100, 100, 100]
Filesystem tests with improved sdcard driver, using optional baudrate=4000000 argument in sdcard init:
Creating and writing files with length of 8000 bytes (ms):
[70, 69, 73, 68, 71, 69, 68, 69, 70, 71]
Reading of those files (ms):
[29, 29, 30, 30, 29, 29, 29, 30, 30, 29]
Filesystem tests with improved sdcard driver, using optional baudrate=8000000 argument in sdcard init:
Creating and writing files with length of 8000 bytes (ms):
[51, 49, 53, 49, 71, 49, 49, 50, 52, 50]
Reading of those files (ms):
[18, 18, 18, 18, 18, 18, 18, 18, 18, 18]
Filesystem tests with original Micropython-lib sdcard driver, after adding an optional baudrate=8000000 argument in sdcard init:
Creating and writing files with length of 8000 bytes (ms):
[68, 390, 59, 59, 61, 60, 60, 60, 62, 59]
Reading of those files (ms):
[26, 26, 26, 26, 26, 26, 26, 26, 26, 26]
My observations from this so far are:
Your driver is faster. The biggest improvement is due to the ability to set a higher SPI baudrate at sdcard initialization.
Such an option is easily added to the original sdcard driver.
With higher SPI baudrate your driver excels at reducing file read times by another ca. 30%.
From a look at your code I see that in initialization there is a slightly better distinction between different card versions.
I did not test the crc option.
I think the code is worth to be looked after by the experts/maintainers.
Seeing the possible improvement by setting a higher SPI baudrate the question occurs: Which maximal baudrate is safe for a certain card type, and how can this type/baudrate be automatically determined?
You commented out the print of e.g. [SDCard] v2 card. My preference would rather be to have a more detailed log/print of the type of sdcard.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
O.k. I tried again with 25MHz, same tests: At higher SPI speed the speed advantage is even more pronounced. How does e.g. Linux determine the SPI speed when accessing an sd card? I should experiment with more different sd cards. You noted that the CRC option may allow to use sdcard types that otherwise are not accessible. Is that correct? Can we make this more specific? On my Blackpill board I soldered Winbond W25Q128 (16 MByte) chip, together with capacitor, of course. See here. Calculating these figures: 8000*8 bits / 3000 us = 21 bits/us => could be achieved with 24 MHz SPI speed (!?), although the SPI may be maximally set to 50 MHz (48 to be precise, as processor freq=96MHz). Theoretically dual mode SPI could be used too. |
Beta Was this translation helpful? Give feedback.
-
It was only the CRC on the commands that seemed to be necessary for one of my cards. The CRC is sent with commands independent of the CRC option; that only affects data transfers. As of now, I have succeeded with three completely different cards (2 GB, 32 GB and 64 GB of different eras).
I may spend a little time looking at the SD card spec to see how to probe the maximum speed. It might be useful to make this module really compliant.
Marcus
… On Oct 31, 2023, at 5:42 AM, Raul Kompaß ***@***.***> wrote:
O.k. I tried again with 25MHz, same tests:
original sdcard.py with baudrate=25_000_000 option:
write: [49, 48, 48, 51, 48, 48, 48, 50, 48, 48]
read: [25, 25, 25, 25, 25, 25, 25, 25, 25, 25]
your modified sdcard_m.py:
write: [35, 33, 34, 33, 32, 32, 36, 33, 32, 36]
read: [12, 12, 12, 12, 12, 12, 11, 12, 12, 12]
At higher SPI speed the speed advantage is even more pronounced.
How does e.g. Linux determine the SPI speed when accessing an sd card?
Can you tell the exact detail that gives this doubling of speed?
I should experiment with more different sd cards. You noted that the CRC option may allow to use sdcard types that otherwise are not accessible. Is that correct? Can we make this more specific?
On my Blackpill board I soldered Winbond W25Q128 (16 MByte) chip, together with capacitor, of course. See here.
It is connected to SPI1. The same test on /flash here gives
write: [280, 274, 273, 283, 271, 276, 272, 274, 271, 269]
read: [3, 4, 4, 4, 3, 3, 3, 4, 4, 4]
Calculating these figures: 8000*8 bits / 3000 us = 21 bits/us => could be achieved with 24 MHz SPI speed (!?), although the SPI may be maximally set to 50 MHz (48 to be precise, as processor freq=96MHz). Theoretically dual mode SPI could be used too.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
I found an error in After reading the specs I found that on line 103 it should be Do you have a v1 card version (< 2GB) perhaps? That should not be working or reporting the wrong size. |
Beta Was this translation helpful? Give feedback.
-
I don't think I can actually find an old V1 card. I will double check your correction against the specs, and put it in.
Thanks.
Marcus
… On Oct 31, 2023, at 11:16 AM, Raul Kompaß ***@***.***> wrote:
I found an error in sdcard.py:
After reading the specs I found that on line 103 it should be
c_size_mult = ((csd[6] & 0b11) << 1) | csd[5] >> 7
instead of
c_size_mult = ((csd[9] & 0b11) << 1) | csd[10] >> 7
in v1 card version.
Do you have a v1 card version (< 2GB) perhaps? That should not be working or reporting the wrong size.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
I was just in the processing of annotating the CSD table, and making sure we were counting bytes in the right order.
I am writing a function to extract bits from it by bit number, and if I use this, which will very slightly increase code size, it will greatly increase readability, and maybe allow future improvements which depend on specific bits from this block.
… On Oct 31, 2023, at 1:24 PM, Raul Kompaß ***@***.***> wrote:
Found an old v1 card (64MB) and it's working.
Correction: There was no error: The CSD obviously is read in as bytes object indexed in reverse order:
So csd[15] translates to csd[0], csd[14] to csd[1] and so on.
In the present code therefore there is no error.
The new SPI baudrate code has to be corrected for this though:
# set to high data rate now that it's initialised
trsfr_unit = 10**((csd[3]&0b111)+4)
trsfr_val = (5,10,12,13,15,20,25,30,35,40,45,50,55,60,70,80)[csd[3]>>3&0x0F]
baudrate = trsfr_unit*trsfr_val
print('SPI baudrate:', baudrate)
self.init_spi(baudrate)
is the correct code: It gives 25 MHz even for my old 64 MB v1 sdcard, as expected and written in the specs.
I noted that with the value mentioned in the above specification I get 25 Mhz, but my card (a SanDisc Ultra 8 GB) actually reports 10 MHz.
And 25MHz for this card now, as it should be. The read times are now at 12 ms too.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
The 25 MHz baud rate seems to be required by the standard for SPI. I'm not sure any other value is actually permitted.
… On Oct 31, 2023, at 1:58 PM, Raul Kompaß ***@***.***> wrote:
More sdcards tested:
Transcend 4 GB: works,
Transcend 16 GB: works after trying to install it a second time, first time:
Traceback (most recent call last):
File "<stdin>", line 7, in <module>
File "sdcard_m.py", line 59, in __init__
File "sdcard_m.py", line 103, in init_card
File "sdcard_m.py", line 165, in init_card_v2
OSError: timeout waiting for v2 card
SanDisk Extreme 256 GB: same timeout error, at every attempt to set up with use_crc = False.
Same error with use_crc = True.
BaseTech64 GB: Does not work the first time after power on:
Traceback (most recent call last):
File "<stdin>", line 7, in <module>
File "sdcard_m.py", line 59, in __init__
File "sdcard_m.py", line 101, in init_card
RuntimeError: SD card rejected CRC on command: 08
Second time it seems to work, regardless of use_crc option .
But the file write / read test reports once it starts to read:
read file0
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<stdin>", line 39, in run
OSError: [Errno 2] ENOENT
This sdcard was full with a lot of different files.
All sdcards reported 25 MHz baudrate.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
I re-thought how to do it, and now it is much smaller, too. I just convert the entire CSD 16-byte array to a big integer, and then directly extract bits. Very clean, compact and easy to read.
… On Oct 31, 2023, at 5:21 PM, Raul Kompaß ***@***.***> wrote:
I was just in the processing of annotating the CSD table, and making sure we were counting bytes in the right order.
nice
I am writing a function to extract bits from it by bit number, and if I use this, which will very slightly increase code size, it will greatly increase readability, and maybe allow future improvements which depend on specific bits from this block.
I'm not in support of this. At least not for a final version. The driver being small will be a criterion for acceptance in the end, probably.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
OK, I did a bunch of cleanup, and have created my own fork of the micropython_lib repo, in eventual preparation for a pull request. here's the updated version, with much cleaner data field extraction. I have verified that my 2 GB card is V1, so I have tested this on V1 and V2. Let me know what you think. I also split up the files differently, so the crc7 code and crc16 code are in different modules. This way, if you aren't using crc16 on the data, you don't have to have the crc16.py file at all. It only gets imported when you turn on data crc. |
Beta Was this translation helpful? Give feedback.
-
Hello @mendenm, I looked at and briefly tested the last version. One argument for your driver is the read/write speed. This I found out depends heavily on the SPI baudrate. In my initial tests I used the default baudrate. Only later I noted that the current MP-lib version has an option for the baudrate. This makes sense as apparently all modern sdcards allow for 25 MHz to be used, but it may be desirable to use a slower rate for other reasons like protection from interference/crosstalk on longer SPI cables. I think you should do a pull request on your sdcard driver version. You might get more feedback and as pull request it might be found by other developers more easily than as appended file in this conversation. Even if it is not accepted in mp-lib it may help/contribute to improve the driver in the future. |
Beta Was this translation helpful? Give feedback.
-
I think the bigint version is much smaller. The calls to the bit extraction routine take almost no space at all. I wii be posting another version sortly, with even more dead code removed; the separate v1 and v2 init routines are now merged, since there was little difference (the recheck for block size was redundant, wince that info also comes from the CSD). So, now I think the overall code size is much smaller.
The exfat formatting is, of course, a problem at a different layer of code. A VFS for micropython to support exfat would have to be written.
I had one card which refused to take commands without the CRC7 correctly computed, so I think it is necessary. Now that I have moved the CRC16 into a separate module, it doesn't cost any code size unless you use it.
I am still cleaning things up, but I think this will be a much more compact module, and it certainly reads and writes faster than the original.
Marcus
… On Nov 3, 2023, at 7:31 PM, Raul Kompaß ***@***.***> wrote:
Hello @mendenm,
I looked at and briefly tested the last version.
From my certainly limited understanding / point of view I welcome your code as an interesting alternative/extended version of the sdcard code. Your changes to represent csd as a bigint may make the code clearer - at the cost of code size. To have the option to check the CRCs is a nice to have. At present from my tests I cannot see that it is a necessary option. My cards work well with the current sdcard.py. Only two of my sdards were not usable, neither by the standard sdcard.py nor by your driver: one of them is hopelessy write protected apparently (any help welcome to change that state) and the other 256GB card has exfat formatting, which is not usable by MP atm.
So a critical question is whether there are sdcards that are usable with your driver that are not usable with the present driver.
Provided that there are none: Following the logic of "if it's not broken, don't fix it" I would hesitate to recommend your new driver.
As soon as there were some sdcards found that would be enabled to work by your code improvements I would recommend the driver.
One argument for your driver is the read/write speed. This I found out depends heavily on the SPI baudrate. In my initial tests I used the default baudrate. Only later I noted that the current MP-lib version has an option for the baudrate. This makes sense as apparently all modern sdcards allow for 25 MHz to be used, but it may be desirable to use a slower rate for other reasons like protection from interference/crosstalk on longer SPI cables.
So big improvements on read/write speed may be achieved with the standard sdcard.py driver by using the baudrate option for setting higher SPI speed.
Given that option is used up to the limit and one wants still higher read/write speed then your driver offers improvement: Somewhat faster write and up to 30-50% faster read speed. I wonder where this speed advantage actually originates from. If just a small part of changed code were responsible for that then I would vote for having a pull request for just this limited code change.
I think you should do a pull request on your sdcard driver version. You might get more feedback and as pull request it might be found by other developers more easily than as appended file in this conversation. Even if it is not accepted in mp-lib it may help/contribute to improve the driver in the future.
You might consider posting the code to awesome micropython perhaps.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
@rkompass I have put an RFC on the micropython issues tracker, at: |
Beta Was this translation helpful? Give feedback.
-
@mendenm: My above mentioned Basetech 64 GB sdcard, that was neither write- nor formattable has the PERM_WRITE_PROTECT bit set. print('sdcard perm_write_protect: {}'.format('on' if gb(csd_int, 13, 13) else 'off'))
print('sdcard tmp_write_protect: {}'.format('on' if gb(csd_int, 12, 12) else 'off')) to the driver I got this message:
Now I learnt that this bit is automatically set by the controller if the sdcard got too many write cycles (or perhaps got too "old" in the old-fashioned sense). Apparently there is no sdcard lib in the Python/Micropython world that allows to solve/diagnose problems like that. At least I did an extensive search. |
Beta Was this translation helpful? Give feedback.
-
I will make a pull request. I was hoping to get some people previewing the zips, before I started the more formal PR process.
I do have my fork, in which I am developing this, at
https://github.com/mendenm/micropython-lib
but just grabbing the zip is easier for a tiny package. Is posting the zips a no-no?
Marcus
… On Nov 5, 2023, at 4:11 PM, Robert Hammelrath ***@***.***> wrote:
You should make a pull request to the lib instead providing the ZIP file.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
In the sdcard.py update I am working on, there is (probably) legacy code which invokes a separate single-block read and single-block write command for those cases. Do either of you think we should not just always use the multi-block command, and only get one block? This removes quite a bit of code. Also, I changed the init call to make it compatible with almost any future upgrades to the crc16 we do. It now takes a function for the crc argument; if the function is None, no crc gets used. If a function is provided, it is used. That way the user can select whatever optimized crc code is available. If we upgrade the rp2 spi code to compute the crc by DMA, the spi.get_crc (whatever we end up calling it) can be passed in. The current work I am doing on this is in: https://github.com/mendenm/micropython-lib/tree/sdcard-fixes/micropython/drivers/storage/sdcard I have slightly more cleanup before I do a PR on this. I am pretty close. I am in the process of converting the asserts into normal exceptions, and doing some other structural cleanup. Those changes will appear in my repo later today. |
Beta Was this translation helpful? Give feedback.
-
@rkompass
Doing a multi-block read or write, and asking it to end after one block, should always be valid. The multi-block commands are just 'at least one block, keep running until told to stop' and stopping after one block is valid.
… On Nov 9, 2023, at 12:06 PM, Raul Kompaß ***@***.***> wrote:
It now takes a function for the crc argument; if the function is None, no crc gets used.
I think that's a good approach. Makes things simpler.
Do either of you think we should not just always use the multi-block command, and only get one block? This removes quite a bit of code.
As non-expert at a first glance I cannot decide whether these are logically equivalent. They use different commands.
For legacy reasons the simple write could be a call to writeblocks, if this would work in a compatible way, what I don't now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
is there a simple way to find where you're hidding the sdcard library ?
Beta Was this translation helpful? Give feedback.
All reactions