frank-zago / ch341-i2c-spi-gpio

WinChipHead CH341 linux driver for I2C, SPI and GPIO mode
GNU General Public License v2.0
70 stars 29 forks source link

try to keep CS up when a next message is coming #16

Open frank-zago opened 1 year ago

frank-zago commented 1 year ago

untested so far

LouisLambda commented 1 year ago

Unfortunately, cs_change has a meaning for every transfer in a message, not just the last. If you're aiming for upstream, you should deal with that.

See https://github.com/torvalds/linux/blob/ba0ad6ed89fd5dada3b7b65ef2b08e95d449d4ab/include/linux/spi/spi.h#L994-L1012

Also, see the kernel default implementation of transfer_one_message for definitive desired behavior.

I realize this model screws up your attempts at packing individual transfers into URBs for better utilization. But this won't actually hurt performance in practice since multi-transfer messages are:

  1. rare
  2. when they occur it's usually in the context of small control I/O, not large data streaming.
  3. CH341 is slow as hell to begin with. perf is not an issue.

Maybe you should just implement set_cs and transfer_one and let the kernel's default transfer_one_message deal with the cs_change logic? It's easier, simplifies the code, will behave correctly and will not cause any noticable perf hit IMO.

frank-zago commented 1 year ago

This change is next on my list. I finished simplifying i2c. I'll do the same for spi.