SPI Slave won't handle CS Interrupt if routed through GPIO Matrix whereas the data lines are on the IO MUX (IDFGH-6768)

[fm1234]

There is a bug in the SPI Slave driver, which only occurs if the SPI Slave uses a GPIO port for the CS line which needs to use the GPIO matrix (that means CS GPIO != 15 for ESP32) whereas the other lines (MISO/MOSI/SCLK) use the default lines and do not need the GPIO matrix.

In this case host->flags&SPICOMMON_BUSFLAG_IOMUX_PINS does not get set, as bus_uses_iomux_pins in spi_common.c only returns false if either the SCLK, QUADWP, QUADHP, MOSI or MISO line uses the GPIO matrix.
I would highly suggest not to add the CS line to this check (which would be one solution) as the CS line is not a high-speed / data line and using using the GPIO matrix for all data lines just because the CS is using the GPIO matrix would reduce the maximum performance to the half.

The initial handling of the CS signal in spi_common.c (spicommon_cs_initialize()) works as the check here is performed not against the flag SPICOMMON_BUSFLAG_IOMUX_PINS but instead it is checked if the CS GPIO is the default or needs routing through the GPIO matrix.

So my suggestion (and this push-request) is not to use the function bus_is_iomux() in restore_cs() (as this returns false even though the CS signal needs the GPIO matrix) but instead does the same as is done in spicommon_cs_initialize and check the routing of the CS signal separately in restore_cs() in spi_slave.c (which doesn’t take longer and thus does not increase the execution time of the interrupt handler).

[ginkgm]

Hi @fm1234 ,

On ESP32, there is a delay if the input signals go through GPIO matrix. For this reason, all input signals should be using/not using IOMUX together. Otherwise there are synchronization issue. For example, due to the delay on CS line, first clock will not be seen and first bit will not be latched.

In practice, if you can ensure the setup/hold time of CS line is long enough, it's not harmful if user want to route data/clock via IOMUX, while CS via GPIO matrix. But this is dangerous, so only expert should do like this. This is the reason why I don't want to merge this PR.

TBH I understand the restore thing will prevent some flexibility. For your requirement, you can use some tricks to do it:

1. Initialize the driver with GPIO matrix, so that the restore feature will always put CS to the GPIO matrix.
2. Reconfigure the GPIO matrix input signals of MOSI, CLK to bypass_gpio matrix. This way, those signals will be placed to IOMUX to get higher frequency performance.

Hope this will help you.

[fm1234]

Hi Michael,

thank you very much for your comments on this.
I see that there can be a timing issue when mixing the signals between IO MUX and the GPIO Matrix, so my pull request is probably not the best way to do it.

But I think this issue needs to be addressed somehow. Currently when wanting to use the "correct" (IO-Mux) GPIOs for MISO, MOSI and SCLK and only the GPIO for the CS line is a different one, then the SPI slave driver does not work as the restore_cs() function currently tries to route the CS directly over IO MUX bypassing the GPIO Matrix as well.

The problem is that the checks performed in bus_uses_iomux_pins() (spi_common.c line 361 and subsequent) are not performed for the CS line (because the struct spi_slave_t is not passed to bus_uses_iomux_pins as this routine is independent of SPI master/slave). If this would be possible then this function would then (correctly) return false even if only the CS pin is using the GPIO Matrix.
This would lead to the parameter force_gpio_matrix to be set to true when calling spicommon_cs_initialize() (spi_common.c line 570 and subsequent). Then all communication would be done over the GPIO Matrix if one of the involved pins are routed over the GPIO Matrix.

It would involve some architectural (function prototype) changes though....

What do you think?

[ginkgm]

Ah, I misunderstood your problem I guess... And I also didn't notice that the check of CS is not consistent with other pins. Good catch..

So the problem here is that, if the other pins are using IOMUX, then the restore function will input CS from IOMUX pin, even if you configured another pin via GPIO matrix. Right?

Then I think it's really an issue. I think I can solve it in a direct way - just to store the GPIO matrix config when freeze CS, while restore it in the restore function.

[fm1234]

Yes, that is part of the issue and that is why this constellation (MOSI/MISO/SCLK over IOMUX and CS over GPIO Matrix) isn't transferring any data at the moment.

I just think that your solution will not fix the issue 100%.
The part that is missing would involve re-routing the MISO/MOSI/SCLK lines over the GPIO Matrix as well as soon as one device uses a CS line which is routed over the GPIO matrix in order to have the same latency / keep the synchronization.
Just using your fix would result in the same behavior as my pull-request.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[wanckl]

Hi @fm1234
I find now that this issue already fixed in same way, cs pin and signal pins are bound to check route through iomux or not.

another issue you mentioned that as soon as one device ,,, may not the issue in slave driver.

so if no other issues, you can close this issue. 👍

[Alvin1Zhang]

Thanks for contribution again, and sorry for the very slow turnaround, and the fix is available at 6cc9380