Repository navigation
Allow (unpadded) SPI transfers < 8bits #5225
Description
Activity
Thanks for the suggestion. I guess it makes sense to support this because SPI is very useful to read/write data with an accurate bit rate. (I checked stm32 MCUs and they don't seem to support this feature, the best they can do is 4-bit transfer resolution.)
Note that there is the
bitsparameter to the SPI constructor, to specify the data width, although in most cases onlybits=8is supported. So one option would be to allowbits=1to set the SPI in bit-mode, but that would probably need to correspond to each byte specifying only one bit, eg:spi = machine.SPI(bits=1) spi.write(b'\x00\x01\x00\x01') # write 4 bits: 0-1-0-1
This would work but it's not a very efficient on buffer space.
Really all that's needed is a way to specify the bit-length of a read/write. It could then be simply a new argument like:
# only transfer 12 bits total, using byte-based buffers spi.write_readinto(outbuf, inbuf, bits=12)
Also might be worthwhile checking other frameworks (eg mbed) to see if/how they support specifying the number of bits.
Reacted by Robert Hammelrath- addedextmodRelates to extmod/ directory in sourceRelates to extmod/ directory in source
on Oct 18, 2019 [NB: in all cases below,
bitsrefers to thebitsparameter from the constructor, though having abitsparameter on the transfer might make more sense.]@mirko And I discussed this on the MP Slack. WRT HW transfers, we determined that for xfers that are >= 8 bits, it works as expected, but for xfers < 8 bits, it always transfers 8 bits.
This is a bit quirky in how it needs to be implemented, since in C, we receive the
bitsparameter, and alenparameter. The former refers to the length of the C buffer (in bytes) to be transmitted.I should note that "under the hood" for the ESP32 HW implementation, we simply calculate "number of bits to send" using
bitsandlen, and then tell the hardware how many (contiguous) bits to send from the buffer. This code has a bug, however where any value ofbits< 8, then "number of bits to send" is rounded up to 8 -- that needs to be fixed.My intention is to also ensure the SW implementation does the same as the HW, but I haven't looked yet at the SW code.
"Obvious" scenarios. Transfer as many bits as you can, as long as that number is a multiple of
bits. These all work wherebits>= 5:bits= 13,len= 1: no transferbits= 13,len= 2: transfer 13 bitsbits= 13,len= 3: transfer 13 bitsbits= 13,len= 4: transfer 26 bitsbits= 6,len= 1: transfer 6 bitsbits= 6,len= 2: transfer 12 bits (though it's unclear if it should be 6 bits from each byte, or 8 from byte 0 and 2 from byte 1; the latter is what's supported by the ESP32 HW, per the note above.)
"Not so obvious" scenarios. Not sure exactly how this should work, but here's a stab:
bits= 4,len= 1: transfer 4 bitsbits= 2,len= 1: transfer 2 bitsbits= 2,len= 2: transfer the first 4 bits of the first byte?
For those cases where
bits< 5, it isn't really obvious what the "right" thing to do is, since doing "as many as you can" isn't really the right thing. If you setbits= 2, you really shouldn't be doing four 2-bit transfers, as that's obviously not what is desired.Arguably, for any case where
bits< 5 (or < 8), you simply transfer that number of bits, regardless of whatlenis set to.But I'm open to other ideas. In any case, whatever behavior we land on needs to be well-documented.
Thanks @MrSurly for the details. I think it's a bit confusing because there are two concepts of "bits":
- the word size, ie number of bits transferred in/out per "word" (like character size for UART)
- the total number of bits transferred in the whole SPI transaction
Some MCUs (eg STM32) have the first concept and some (eg ESP32 AFAIK) don't:
- For esp32 you specify the total bit length and give it data in bytes and it just transfers in/out the requested number of bits. So it's like a word size of 8 bits, but you can do fractional words.
- For stm32, you configure either 4, 8 or 16 bit word size, then tell it how many words to transfer. And it takes the required number of bits from each word (eg only 4 bits from each byte given).
As I see it there are two (separate but related) issues here:
- how to handle
bitsin the constructor when it's not 8 or 16 - whether to add a
bitsparameter towrite_readinto()to support sending arbitrary number of bits
As I see it there are two (separate but related) issues here:
how to handle bits in the constructor when it's not 8 or 16 whether to add a bits parameter to write_readinto() to support sending arbitrary number of bitsProposal:
- Keep the
bitsfrom the constructor - Do not add
bitstowrite_readinto() - If bits < 8, send that # of bits from each byte
- If bits > 8, send that group of bits as many times as we can for the bytes given, leaving unsent bits in a byte if necessary
For the last item, send the first N bytes where N is
bits / 8, and the remainder (bits % 8) is sent from byte N + 1. If there are enough bytes to send another full set ofbits, then do it again, starting from N + 2.- Keep the
Might be another topic (happy to open another issue if desired): For my above stated case - talking SWD protocol - I'll have to change word sizes on the fly.
Right now, within micropython, that implies a complete re-init of the SPI device, while - at least the underlying ESP IDF SDK - allows modifications of those attributes without a complete deinit/(re)init.
This sounds like inconvenient at most, however an init of the SPI bus toggles MOSI/SCK which in fact interferes with the protocol.
So I'd highly wish for either a quiet (re)init of an SPI device config or alteration of its attributes without re-init-ing.
Do you think that's reasonable?@MrSurly your proposal sounds good to me!
a quiet (re)init of an SPI device config or alteration of its attributes without re-init-ing.
Do you think that's reasonable?Yes that's very reasonable behaviour to expect (independent of the discussion about bits length). I don't think it'd be too difficult to implement.
Your use case sounds compelling. I don't think there's any reason to not allow per-transactions
bitsparameter; if it's not passed, then just usebitsfrom the constructor. Implementing this is trivial, at least for the ESP32, and I suspect also for the bit-bang implementation.@dpgeorge What about other implementations? Is it ok if bit-bang and ESP32 support this? I suspect the answer is "yes" since different ports have different restrictions on SPI anyway.
Is it ok if bit-bang and ESP32 support this?
Yes.
@MrSurly Did you by any chance made any progress here? If not (really): could you imagine taking a(nother) look at this in the foreseeable feature? I'd still be very happy about this!
I have not; thanks for the prompt. Let me work up a potential PR.
@dpgeorge WRT updating xfer functions (i.e. read, readinto, read_writeinto, write, write_readinto) -- should that be an optional KW arg, or just an optional positional arg?
@dpgeorge There's also this:
https://github.com/micropython/micropython/blob/master/extmod/machine_spi.c#L204
if (args[ARG_bits].u_int != 8) { mp_raise_ValueError("bits must be 8"); }
Presuming that it's okay to change this for bitbang?
I could not find any updated documentation so far for the implemention of #5542
So I played around with different scenarios to find out how it works.
Either I'm not getting it right or we have a bug here.
The following two worked as expected:
bits = 8
length 1 byte: 8 clockings
length 2 byte: 16 clockingsbits = 10
length 1 byte: "buffer too short"
length 2 byte: 10 clockings (8 bit of byte no 1, 2 bit of byte no 2)The following two kind of worked:
bits = 6
length 1 byte: 6 clockings
length 2 byte: 12 clockings (8 bit of byte no 1, 4bit of byte no 2)
I find the second case not really obvious.The following really confused me. Is it a bug? Or am I missing any logic?:
bits = 3
length 1 byte: 6 clockings > would expect 3 clockings
length 2 byte: 15 clockings > would expect either 6 clockings (3 bits each byte) or 11 clockings (8 from byte no 1, 3 from byte no 2)bits = 2
length 1 byte: 8 clockings > would expect 2 clockings
length 2 byte: 16 clockingsI used the following code to test:
spi.init(baudrate=SPI_BAUD_RATE, polarity=0, phase=0, bits=10, firstbit=SPI.MSB, sck=Pin(SPI_SCK_PIN, Pin.OUT), mosi=Pin(SPI_MOSI_PIN, Pin.OUT), miso=Pin(SPI_MISO_PIN, Pin.OUT) )Using MicroPython v1.19.1 on 2022-06-18; ESP32S3 module (spiram) with ESP32S3
Taking the quotation from above:
For esp32 you specify the total bit length and give it data in bytes and it just transfers in/out the requested number of bits. So it's like a word size of 8 bits, but you can do fractional words.
The most logical solution so me would be:
- If the number of bits to be transferred is equal to 8, transfer all bytes that were provided (compatibility to the current implementation)
- If the number of bits is greater than the number of words*8, throw an error
- If the number of bits is smaller than the number of words*8 ant not equal to 8, "cut" the rightmost bits from the rightmost byte
I have a patchset lying around applying on latest master. Unpolished, though. Planning to create a new PR. If you're willing to help, it will definitely speed things up.
@mirko Do you have any nightly build or something like that so I can test it? I'm not a developer so I can't compile anything. But I can try out a new flash .bin file if this helps.
SPI allows word sizes of several bits, not necessarily rounded up to a multiple of 8.
While I don't now about every platform / hardware, at least soft-SPI (GPIO bitbanged SPI) and the ESP IDF driven ESP32 SPI hardware support transferring single bits via SPI.
Looking at the micropython code though, the whole SPI abstraction layer - HW backed or SW - assumes lengths of multiples of bytes, meaning, I can't just adjust the parts related to esp32 / gpio.
To provide a use case - and I indeed see this being an edge case, but keep in mind that it still conforms with the SPI spec: (ab)using SPI to speak SWD.
Suggestion / Feature request: Adjust the SPI code so that we can transmit single bits instead of multiples of bytes.
Happy to do it myself / help / assist / for discussion. However at first glance, the changeset apparently would be larger than anticipated and I'd be glad for opinions / input / help.