micropython/drivers/sdcard.py: Add CRC7 values for SDCard.cmd() function calls

[multicoder9]

Seems to be needed for some SDXC cards (SanDisk Ultra SDXC). FIXES Issue #17039

Edit: I'm including the following from the issue to clarify this pull request.

This is my first post to GitHub, but I've been working with micropython for about 10 years.

Recently, I have been writing some micropython based sdcard dataloggers for a CANbus project, mostly with rp2040 and rp2350 devices. I have been using the sdcard.py source file as a driver in my projects.

Today I noticed that switching from my 32GB SanDisk Ultra SDHC which worked just fine to a 32GB SanDisk Ultra SDXC caused it to fail on initialization. Others have noticed the same thing caused by missing CRC7 such as issue #12687. But none of them resulted in the fix getting merged back upstream. Here is a trivial diff for sdcard.py which adds the CRC7 values needed to get it working with my SDXC card, while still appearing to work on the older cards I've tested it on. Just for completeness, I've added a commented out minimalist Python function to compute these CRC7 values. (A quick internet search didn't pull up any Python functions for this.)

I figured I'd try to get the minimal changes merged upstream while more significant rewrites like the one in the linked thread above work their way through the process.

[Josverl]

Thanks for the PR.

It would be nice if you could add some information about these magic values that make things work better.
Either in comments, or by defining it as a const with an explanatory name.

That would make it possible for future maintainers to understand better wat the code's intent is.

[scruss]

... or why this is better than the long-pending, mostly-reviewed SDcard sdcard.py rewrite for efficiency and function by mendenm · Pull Request #765 · micropython/micropython-lib

[multicoder9]

Sorry, I started this as an issue here. I'm new to GitHub and didn't realize that the pull request would start another conversation. Is there a way to link the conversations together besides just posting a link?

But short version, as I explained in the issue, is that older cards such as my 32GB SanDisk Ultra SDHC work just fine with the current sdcard.py code, but a newer 32GB SanDisk Ultra SDXC fails at initialization. This is due to CRC7 values missing from the original code. Older cards allow ignoring the CRC7 values, but newer ones seem to be more strict. At the top of my patch, I've commented out a short Python function to generate the correct CRC7 values. Since I couldn't find a Python of this while searching around, I'll post it here in case anyone comes along:

def crc7( cmd:bytes ):
    crc = 0
    for value in cmd:
        for i in range(8):
            crc <<= 1
            if (value&0x80)^(crc&0x80):
                crc ^= 0x09
            value <<= 1
    return ((crc<<1)|1)&0xFF
def cmd_args_to_bytes( cmd:int, args:int ):
    return bytes( [cmd|0x40,(args>>24)&0xFF,(args>>16)&0xFF,(args>>8)&0xFF,args&0xFF] )

So for example, on the line self.cmd(58, 0, 0xFD, 4), if you do:

print( f'0x{crc7( cmd_args_to_bytes( 58, 0 ) ):02X}' )

You'll get the 0xFD I've included in the code.

[multicoder9]

... or why this is better than the long-pending, mostly-reviewed SDcard sdcard.py rewrite for efficiency and function by mendenm · Pull Request #765 · micropython/micropython-lib

As for that, my thought is simple. This gets the existing code working with SDXC cards by doing nothing more than filling in the correct CRC7 values according to the spec. It is more of a bug fix than a restructuring. In the entire file, I've done nothing more than set 11 numbers that were unset (10 were 0, and one was 0xFF). I was hoping this would allow the existing sdcard.py to work for the users having problems with it while the "long-pending, mostly-reviewed" pull request continues its process of getting reviewed. But that is a major rewrite for overall improvement. Mine is simply correcting the fact that the CRC7 values in the existing one are wrong, and while many older cards are okay with this, some newer ones aren't.

So I hoped it would get approved in the meantime, until the bigger cleanups are approved. Although one thing that is better about this than the sdcard.py in the PR#765 is that one is computing CRC's dynamically that can't possibly change. All the parameters are hardcoded, so the CRC will always be a constant value. On a desktop, I would have written it that way, but on a microcontroller, I would normally precompute any values that are known at coding time and will never change.

And hardcoding it has the advantage of not using any memory for storing a globally computed value, and keeping the value next to the hard coded parameters that it was computed from. If anyone changes a hard coded argument, it will be good to have the hard coded CRC next to it to update too.

But these are style differences, and most of the CRC computations will execute once, so it doesn't add a great impact, so I'll leave it to others to decide. The same with the 256 byte hardcoded memory usage in the global CRC table. Usually it is probably worth it, but may not be if you can get by without it.

For readblocks and writeblocks, I've read that passing a constant 0x7F will cause the card to accept the command, which I've done, and works on my card. But that PR computes the correct CRC values for these functions. Which I'd normally prefer, once that PR is approved. But the 0x7F value appears to be an allowed one while the value in the current file returns an error on some cards. So again, this is a minimal fix to get the existing code working while long languishing PRs get finalized.

[multicoder9]

FYI I just found two more self.cmd lines in writeblocks() with zero CRC7 values. I changed them to 0x7F and have confirmed that writeblocks now works with my SDXC cards.