Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frank-zago
Copy link
Owner

untested so far

@LouisLambda
Copy link

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
Copy link
Owner Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants