Add support for decode spi dual/quad#103
Add support for decode spi dual/quad#103mfont-bz17 wants to merge 2 commits intosigrokproject:masterfrom
Conversation
|
Nice job! |
| es += self.samplenum - self.sio0bits[0][1] | ||
|
|
||
| self.sio0bits.insert(0, [sio0, self.samplenum, es]) | ||
| self.sio1bits.insert(0, [sio1, self.samplenum, es]) |
There was a problem hiding this comment.
Consider using a named tuple instead of a list for [bit,ss,es]
There was a problem hiding this comment.
It was based on spi code that use list, but I'll change for named tuple, the code will be easier to understand.
There was a problem hiding this comment.
I was looking to change this part but I think that is better to keep using list instead of named tuple:
- Code is more similar to the spi decoder.
- To keep the outputs as are in the spi decoder if we use named tuple we have to reconvert the bits output.
['BITS', [[1, 80, 82], [1, 83, 84], [1, 85, 86], [1, 87, 88], ... - Named tuples are immutable and we have to use _replace function to create the new one.
Maybe instead of using directly the index we can use some constant
BIT_VAL = 0
BIT_SS = 1
BIT_ES = 2
self.sio0bits[0][BIT_VAL]
@kamocat what do you think?
There was a problem hiding this comment.
I had forgotten that tuples are immutable. Yes, you could use constants to annotate. Something like VAL,SS,ES = range(3). Alternatively, you could use a class to hold the data. I will accept either option.
About the output compatibility, I understand that we need to keep the compatibility to allow the same stacked decoders. It does seem redundant, though, to include ES and SS in the data when they're already in the function parameters.
| self.bitcount += 2 | ||
|
|
||
| # Continue to receive if not enough bits were received, yet. | ||
| if self.bitcount != ws: |
There was a problem hiding this comment.
Is there any validation to check that the wordsize is a multiple of 4? It would be safer to use < rather than !=
There was a problem hiding this comment.
Good point. I'll add a validation in decode function to check if wordsize is multiple of 2/4 depending on mode.
f4435ab to
3632775
Compare
…list for constants
|
I've just written a Dual/Quad SPI decoder myself: https://github.com/mbenkmann/quadspi_sigrok It takes a different approach than the one from this PR. The main problem with Dual/Quad SPI is that you don't know which parts of a transfer are Dual or Quad without interpreting the command (which is usually transmitted as basic SPI). So my decoder incorporates a command set selection (currently only "Auto" supported and only the W25Q command set is implemented) and provides a more high level decoding |
|
This is an excellent contribution, but I'm afraid I have to agree with @mbenkmann that a pure Dual/Quad SPI decoder may not be applicable in all the cases. When we talk about the SPI flash, usually the command is transmitted via the Single SPI, and the following data is transferred in Single/Dual/Quad SPI, depending on the instruction. See the W25M02GV datasheet as an example. A decoder for the Dual/Quad SPI flash would require some extensive abstraction leakage, since one would need to decode the command, and based on that command to switch to the appropriate mode on-the-fly. That's exactly what @mbenkmann's implementation does. |

Hi,
This is my first contribution to sigrok project.
The PR adds support to decode SPI Dual/Quad. The decoder is based on the spi decoder.
Maybe a new option can be added in the future to allow decode at double data rate (DDR).
I tested the quad spi using the the capture https://github.com/sigrokproject/sigrok-dumps/tree/master/spi/sqi
With the same example dual spi can be tested also.
Logic analyzer setup
The capture was taken at a samplerate of 100MHz.
Probe SPI
2 SCK (clock)
3 CS (select)
4 IO0 (data, LSB)
5 IO1
6 IO2
7 IO3 (data, MSB)
sqi-four-data-lines-one-transfer.sr
This capture uses four I/O data lines. Data gets sampled at the rising
clock edge (single data rate). Data is sent in MSB first order. The SPI
transaction's data byte sequence is:
80 00 00 10 22 42 4f 4f 54 00 80 00 00 a8 85 77 00 20 4e 00 00
Best regards