Allow (unpadded) SPI transfers < 8bits

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.

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 bits parameter to the SPI constructor, to specify the data width, although in most cases only bits=8 is supported. So one option would be to allow bits=1 to 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.

@dpgeorge

@dpgeorge

[NB: in all cases below, bits refers to the bits parameter from the constructor, though having a bits parameter 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 bits parameter, and a len parameter. 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 bits and len, and then tell the hardware how many (contiguous) bits to send from the buffer. This code has a bug, however where any value of bits < 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 where bits >= 5:

bits = 13, len = 1: no transfer
bits = 13, len = 2: transfer 13 bits
bits = 13, len = 3: transfer 13 bits
bits = 13, len = 4: transfer 26 bits
bits = 6, len = 1: transfer 6 bits
bits = 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 bits
bits = 2, len = 1: transfer 2 bits
bits = 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 set bits = 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 what len is set to.

But I'm open to other ideas. In any case, whatever behavior we land on needs to be well-documented.

@MrSurly

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 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 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 bits

Proposal:

Keep the bits from the constructor
Do not add bits to write_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 of bits, then do it again, starting from N + 2.

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

@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.

@mirko

@mirko

Your use case sounds compelling. I don't think there's any reason to not allow per-transactions bits parameter; if it's not passed, then just use bits from 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

@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

@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

@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?

@mirko

@mirko
#5542

*/*spi.c

Merge pull request micropython#5225 from weblate/weblate-circuitpytho…

*/*spi.c

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 clockings

bits = 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 clockings

I 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

@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.

Navigation Menu

Repository navigation

Allow (unpadded) SPI transfers < 8bits #5225

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Allow (unpadded) SPI transfers < 8bits #5225

Description

Activity

dpgeorge commented on Oct 18, 2019

MrSurly commented on Oct 18, 2019

dpgeorge commented on Oct 21, 2019

MrSurly commented on Oct 23, 2019

mirko commented on Oct 24, 2019

dpgeorge commented on Oct 24, 2019

MrSurly commented on Oct 26, 2019

dpgeorge commented on Oct 29, 2019

mirko commented on Jan 14, 2020

MrSurly commented on Jan 14, 2020

MrSurly commented on Jan 15, 2020

MrSurly commented on Jan 15, 2020

MrSurly commented on Jan 15, 2020

sebert007 commented on Mar 5, 2023

mirko commented on Mar 5, 2023

sebert007 commented on Mar 6, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions