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.

Sign up for GitHub

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

Jump to bottom

spi: add settings to embedded-hal-bus SpiDevice impls for CS-to-clock delays. #539

Open
Dirbaio opened this issue Nov 28, 2023 · 3 comments · May be fixed by #605
Open

spi: add settings to embedded-hal-bus SpiDevice impls for CS-to-clock delays. #539

Dirbaio opened this issue Nov 28, 2023 · 3 comments · May be fixed by #605

Comments

@Dirbaio
Copy link
Member

Dirbaio commented Nov 28, 2023

As discussed in today's meeting:

It is better if the CS-assert-to-clock and clock-to-CS-deassert delays are configured in the SpiDevice itself, so that the driver doesn't have to add them to every single transaction, and can use the read/write/write_read convenience methods.

  • add configuration settings to the SpiDevice impls in embedded-hal-bus to do the delays
  • document that drivers are discouraged from using Operation::DelayNs directly for CS-to-clocks delays.
@Dirbaio Dirbaio added this to the v1.0.0 milestone Nov 28, 2023
@eldruin eldruin mentioned this issue Dec 27, 2023
Closed
@Dirbaio Dirbaio removed this from the v1.0.0 milestone Dec 27, 2023
@Dirbaio
Copy link
Member Author

Dirbaio commented Dec 27, 2023

i'm going to remove this from the 1.0 milestone,

It concerns only embedded-hal-bus, we can do it in 0.1.1 in a backwards-compatible way (like adding .set_blah_delays() methods), or in 0.2 if we don't want to be compatible.

@Dirbaio Dirbaio mentioned this issue Dec 27, 2023
Merged
@vpochapuis
Copy link

Hello!
Thank you very much for this amazing set of crates that makes Rust embedded system usage so convenient.

I would like to know if there is any plan to support this feature as its necessary to drive chips like the LTC6813
image

If there is but no one is currently working on it, I would be pleased to be given direction in order to help implement this!
I have previously worked on enabling this in the esp-idf-hal at a higher level to access existing C API, but didn't touch yet to this set of crates in more details.

At first sight I would say, adding a parameter in the Device creation function, as well as a field in the Device struct to store a delay duration and using the delay_ns() function after this line

embedded-hal/embedded-hal-bus/src/spi/exclusive.rs

Line 119 in a0ccb65

self.cs.set_low().map_err(DeviceError::Cs)?;
could do the trick? But I am not sure how/where the clock is controlled.

If that's alright i would be happy to provide a PR for this

@vpochapuis vpochapuis linked a pull request May 31, 2024 that will close this issue
Open
@ProfFan
Copy link

ProfFan commented Jun 7, 2024

Could the SPI SCLK frequency be also added? This is very useful when multiple devices share the same SPI bus, and is very common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(embedded-hal-bus): Spi - Add support for cs to clock delay vpochapuis/embedded-hal
3 participants
@Dirbaio @ProfFan @vpochapuis