SPI on ESP32-C3 is not working as expected

[flxzt]

I have been trying to write a display driver for GC9A01 and ESP32-C3, but have issues using the SPI interface. What I have been observing is that for every byte of data I am sending over the bus, there is another concaternated byte with 0xff written on the bus. E.g. writing spi.write(&[0x12]) results in 0x12 0xff observed on the bus with a logic analyzer. Writing multiple bytes, e.g. 0x23 0x45 0x56 results in 0x23 0x45 0x56 0xff 0xff 0xff.

It sounds the same as issue #79 . It seems that for every transaction polling_transmit gets called twice, first for transmitting the data, and a second time with transaction_length = 0, but somehow results in the additional bytes on the bus.

How I initially discovered this:

    fn write_cmd(&mut self, cmd: u8) -> Result<(), error::DriverError> {
        let dc = &mut self.dc;

        self.spi
            .transaction(|b| {
                dc.set_low().unwrap();
                b.write(&[cmd]).unwrap();
                Ok(())
            })
            .map_err(|_| DriverError::IO)?;
        Ok(())
    }

and a view on the bus when calling write_cmd(0xeb); write_cmd(0x14):

First row is CLK, second CS, third MOSI, forth is DC (for the display).

[Dominaezzz]

This code simply doesn't work on non-ESP32 it seems. https://github.com/esp-rs/esp-idf-hal/blob/617a7b3dcbf2f020d0bbf41ad8f8b2513c3527e8/src/spi.rs#L336-L339

As a workaround you can use the e-hal 0.2 SPI apis which doesn't have this issue. (It's not as pleasant to use but oh well)

This might be a case of just managing the CS in software, which sucks but I don't see a way around this. Fixing this upstream in esp-idf will probably take way too long.

[ryanolf-tb]

I came to report this same issue. (I'll add that I think the bit content of the extra bytes sent by polling_transmit(self.handle, ptr::null_mut(), ptr::null(), 0, 0, false) may vary -- I don't have my analyzer in front of me but I don't recall seeing 0xFF for every byte. But the same story -- it's some sort of echo with the same length as the last intentional transmission.)

@Dominaezzz For now I've modified SpiBusMasterDriver::finish() to be a no-op, and for SpiBusMasterDriver::polling_transmit to leave the SPI_TRANS_CS_KEEP_ACTIVE flag false, and I'm just careful to always transmit using equal length read and write buffers, or just read xor write buffers (so that SpiBusMasterDriver::polling_transmit is only ever called once per transaction).

To get unequal read/write transfers working, you could modify SpiBusMasterDriver::polling_transmit to take the SPI_TRANS_CS_KEEP_ACTIVE flag as an argument (like the main polling_transmit function) and just make sure the last call to SpiBusMasterDriver::polling_transmit in a transaction sets it to false. I can post a working example modification in a bit Obviously this isn't ideal, as it kinda breaks the SpiBusMaster API, but for folks who use SpiMasterDriver to access the bus, it gives the expected behavior.

It does seem likely to be an issue upstream, though I haven't gone deep enough into the C code to see the particular issue. Does the team have any progress on this?

[ryanolf-tb]

Has anyone submitted an upstream issue?

[ryanolf-tb]

BTW we are using the ESP32-S3, so this isn’t just specific to the -C3.

[lynaghk]

To clarify, the potential upstream issue is that the rust hal calls (via polling_transmit free function) the esp-idf function spi_device_polling_transmit with spi_transaction_t fields length and rxlength both equal to zero and expects the esp-idf function to:

• transmit nothing
• de-assert the CS line

but instead something is transmitted over the bus?

I took a dive into the esp-idf implementation and found this suspicious code:

static inline void spi_ll_set_mosi_bitlen(spi_dev_t *hw, size_t bitlen)
{
    if (bitlen > 0) {
        hw->ms_dlen.ms_data_bitlen = bitlen - 1;
    }
}

which gets called via the chain

polling_transmit (rust free function)
spi_device_polling_transmit
spi_device_polling_start
spi_new_trans
spi_hal_setup_trans
spi_ll_set_mosi_bitlen

I suspect what's happening (though I haven't tested on hardware) is that the 0 bitlength of the transmission means the underlying ms_data_bitlen register never gets set, and so retains the value of the last transmission. That would explain the three 0xff bytes @flxzt saw after their three byte transmission. Interestingly, the esp32 version of this register-setting function lacks the guard:

static inline void spi_ll_set_mosi_bitlen(spi_dev_t *hw, size_t bitlen)
{
    hw->mosi_dlen.usr_mosi_dbitlen = bitlen - 1;
}

(though in that case I would expect to see a lot of extra bits from the rollover, so something else must be going on on that platform)

In any case, I can think of two ways to resolve:

1. Open an issue on esp-idf to support zero-length spi transmissions
2. Adjust the Rust HAL to de-assert the CS pin more explicitly in the finish method.

My gut feeling leans towards the latter, though I haven't yet looked at the esp-idf methods to see how that might be implemented.

[Dominaezzz]

To get unequal read/write transfers working, you could modify SpiBusMasterDriver::polling_transmit to take the SPI_TRANS_CS_KEEP_ACTIVE flag as an argument (like the main polling_transmit function) and just make sure the last call to SpiBusMasterDriver::polling_transmit in a transaction sets it to false. I can post a working example modification in a bit Obviously this isn't ideal, as it kinda breaks the SpiBusMaster API, but for folks who use SpiMasterDriver to access the bus, it gives the expected behavior.

Yeah, all the SpiDevice methods can be fixed this way, see we know in advance the total length of the transaction. I was planning to do this but lacked motivation since I'm using a plain ESP32 which works. The SpiBusMaster API is fine break, it's not part of the public API of the library. As long as the SpiBus trait is still implemented then it's fine.

It does seem likely to be an issue upstream, though I haven't gone deep enough into the C code to see the particular issue. Does the team have any progress on this?

I haven't created an issue upstream.

[Dominaezzz]

In any case, I can think of two ways to resolve:

1. Open an issue on esp-idf to support zero-length spi transmissions
2. Adjust the Rust HAL to de-assert the CS pin more explicitly in the finish method.

I tried number 2 and for some reason I'm not able to de-assert the pin. It's like the spi driver is preventing it somehow but that would be an excellent solution if it worked!

There's also the option of simply exposing an SpiBus instead of (or in addition to) an SpiDevice, so users can manage CS themselves if they need/want to.

There's also the option of asking esp-idf to allow manual deassertion of the CS pin.

If spi_device_transmit was used instead of spi_device_polling_transmit, would that workaround the issue? Maybe the code path is different enough to make it work. I don't have a device to check this with.

[mr-sven]

I don't know if the issue overlaps, but I've also a strange issue. On receving data the SPI buffer is read to early. I've connected a device via SPI3 and I read the following data, the 8th bit is always in the next byte

Read:   21 7f 81 01 0f 350f 4000 05 18 88
Should: 21 ff 01 01 0f 350f 4000 05 98 08

It looks for me the buffer is read before the 8th bit is shifted into. If I run a C app on the same device with I think same SPI setup, everything is fine.

[Vollbrecht]

I don't know if the issue overlaps, but I've also a strange issue. On receving data the SPI buffer is read to early. I've connected a device via SPI3 and I read the following data, the 8th bit is always in the next byte

Read:   21 7f 81 01 0f 350f 4000 05 18 88
Should: 21 ff 01 01 0f 350f 4000 05 98 08

It looks for me the buffer is read before the 8th bit is shifted into. If I run a C app on the same device with I think same SPI setup, everything is fine.

What version of esp-idf-hal are you using ? You are not using an esp32c3 rather an esp32 or an s2 / s3 ? there could be a couple of problems here at play so please clarify before.

[mr-sven]

Ah, sorry maybe we can move this to a new issue, esp-idf-hal 0.40.1 on an esp32.

[Vollbrecht]

Ah, sorry maybe we can move this to a new issue, esp-idf-hal 0.40.1 on an esp32.

the simplest way to determine if you are affected by this you could replace the normal SpiDeviceDriver with an SpiSoftCsDeviceDriver. usage wise it works the same but manages the chip select internally manual for you, but critically does not reflush the driver after an transaction. so no unnecessary clkout is happening.

[mr-sven]

I checked with a logic analyzer, CS is correct. So I create a new issue for that.

[Vollbrecht]

@ivmarkov is fixed so can be closed now. we don't use an empty flush on the bus anymore that caused the issue