Extend sdcard_spi decoder to support more commands, more responses, and decode response fields.

[dvosully]

This PR adds support to the sdcard_spi decoder for new commands, and adds further field decoding to existing commands.
I have been using it successfully in PulseView for diagnosing sdcard initialisation failures and filesystem read/write transactions.

sdcard_spi: Add support for CMD6, CMD10, CMD13, CMD25, ACMD51, CMD58. Add support for R2, R3. Add field decoding to CMD6, CMD9, CMD10, CMD25, ACMD51. Improve robustness of decoder.

[dvosully]

There is some overlap between this and #87

[kamocat]

Hi Dvosully,
Do you have a sigrok dump you've been testing this on?

[dvosully]

Hi kamocat,
Attached is a trace I was performing much of my testing on.
It isn't particularly succinct as it contains spi traffic from other devices in the project i was working on, but the decoder ignores the irrelevant content due to the CS line.
The trace contains: A full SD card probe and initialisation, followed by a filesystem mount (reading the fat filesystem table with numerous block reads), followed by writing a file (many CMD17 block reads, some CMD24 write single block, and CMD25 write multiple block, and plenty of busy-waiting)

This doesn't capture the full spectrum, the robustness improvements were mostly identified when the clock rate exceeded the capability of my logic analyser causing parsing to badly fail. The decoder previously did not use the CS line to re-sync, so once it got lost it would stay lost. Part of the changes were to use the CS to re-sync, so when the device under test reset and re-negotiated at a lower clock rate the decoder would once again give useful information.
Additionally some of the internal state machines were very brittle, if certain events did not happen (ie if not every command completed with success) it would not close parsing one command and start another. This I identified and fixed as it happened with many ephemeral captures, I would struggle to find captures demonstrating this cleanly I'm afraid.

sdcard_spi_trace.zip

[kamocat]

Overall the changes are good. You added a lot of command handlers.
However, it's very verbose and takes several hours to read. I might not notice a mistake in all the bit shifting and masking. Considering how repetitive the annotations are, I think it could be condensed a lot with lists and loops.

[kamocat]

Line 353 can be simplified to if res & 0b111100:

[kamocat]

The end sample appears to be earlier than the start sample - do earlier bits have a later index? Also, it makes me uncomfortable to assign class variables only to use them hidden in the next function call. Why not pass them in?
Instead, make a new method that will read the offsets for you. The function could go something like

def putbits(startbit, length, annotation):
    ss, es = self.read_buf_bits[startbit][1], self.read_buf_bits[startbit-length][2]
    if not isinstance(annotation,list):
        annotation = [annotation]
    self.put(ss, es, self.out_ann, [Ann.BIT, annotation])
    return startbit-length

Even better, you could put it in a loop and calculate the next startbit using the return value.

[kamocat]

This identical putbit function is defined five times in this file. Why not define it once in the class scope?

[kamocat]

Please index a list instead of 15 elif statements.

[dvosully]

Good suggestions, will do that soon.
As for why it's like it is, it's largely because I was just copying the patterns that were already there, being almost entirely unfamiliar with Python.