Skip to content

Allow (unpadded) SPI transfers < 8bits #5225

Description

@mirko

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.

Activity

  1. dpgeorge commented on Oct 18, 2019

    @dpgeorge
    Member

    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.

  2. added
    extmodRelates to extmod/ directory in source
    on Oct 18, 2019
  3. MrSurly commented on Oct 18, 2019

    @MrSurly
    Contributor

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

  4. dpgeorge commented on Oct 21, 2019

    @dpgeorge
    Member

    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
  5. MrSurly commented on Oct 23, 2019

    @MrSurly
    Contributor

    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.

  6. mirko commented on Oct 24, 2019

    @mirko
    Author

    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?

  7. dpgeorge commented on Oct 24, 2019

    @dpgeorge
    Member

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

  8. MrSurly commented on Oct 26, 2019

    @MrSurly
    Contributor

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

  9. dpgeorge commented on Oct 29, 2019

    @dpgeorge
    Member

    Is it ok if bit-bang and ESP32 support this?

    Yes.

  10. mirko commented on Jan 14, 2020

    @mirko
    Author

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

  11. MrSurly commented on Jan 14, 2020

    @MrSurly
    Contributor

    I have not; thanks for the prompt. Let me work up a potential PR.

  12. MrSurly commented on Jan 15, 2020

    @MrSurly
    Contributor

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

  13. MrSurly commented on Jan 15, 2020

    @MrSurly
    Contributor

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

  14. MrSurly commented on Jan 15, 2020

    @MrSurly
    Contributor
  15. added a commit that references this issue on Feb 21, 2020
  16. added a commit that references this issue on Aug 26, 2021
  17. added 3 commits that reference this issue on Oct 30, 2021
  18. sebert007 commented on Mar 5, 2023

    @sebert007

    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
  19. mirko commented on Mar 5, 2023

    @mirko
    Author

    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.

  20. sebert007 commented on Mar 6, 2023

    @sebert007

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    extmodRelates to extmod/ directory in source

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions