-
Notifications
You must be signed in to change notification settings - Fork 181
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
feature: expose C api cs_pre_trans_delay
in SpiDriver
#266
Changes from 1 commit
0412e3b
0e2807b
5b1899b
4432eec
3d918c5
3fc68ff
0dd6ba6
164389e
f047c62
c1b1464
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -233,6 +233,10 @@ pub mod config { | |
pub duplex: Duplex, | ||
pub bit_order: BitOrder, | ||
pub cs_active_high: bool, | ||
///< Amount of SPI bit-cycles the cs should be activated before the transmission (0-16). This only works on half-duplex transactions. | ||
pub cs_pre_delay_us: Option<u16>, // u16 as per the C struct has a uint16_t, cf: esp-idf/components/driver/spi/include/driver/spi_master.h spi_device_interface_config_t | ||
///< Amount of SPI bit-cycles the cs should stay active after the transmission (0-16) | ||
pub cs_post_delay_us: Option<u8>, // u8 as per the C struct had a uint8_t, cf: esp-idf/components/driver/spi/include/driver/spi_master.h spi_device_interface_config_t | ||
pub input_delay_ns: i32, | ||
} | ||
|
||
|
@@ -277,6 +281,22 @@ pub mod config { | |
self | ||
} | ||
|
||
/// Add an aditional Amount of SPI bit-cycles the cs should be activated before the transmission (0-16). | ||
/// This only works on half-duplex transactions. | ||
#[must_use] | ||
pub fn cs_pre_delay_us(mut self, delay_us: u16) -> Self { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does it behave when not in half-duplex mode? Do we get a runtime EspError or does it just get eaten silently? If you are not sure could you test out the cases and report how it acts running? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a very funny thing in fact. I only use the FullDuplex mode. When I set it to 1/16/1000, I get the same small time gap between the CS pull-down and the first clock (and MISO answers): This is all using FullDuplex. They say in the C API driver include that its not supported, but somewhere else if we look closely we see:
So, at least from what I see its that, even though some part of the C doc seems to say that in FullDuplex mode it doesn't work. In reality its more like a on-off switch for a 1us pre-delay in FullDuplex mode, but it does make a big difference for some SPI chips. Also, as you can see, setting a value higher than 16 (here 1000), doesn't change the behavior compared to 16. (And sorry for the bad pictures instead of the oscilloscope screen capture, I was lazy to remember the functionality, but I think its still is enough to demonstrate) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Further reading of the doc says:
But on the field itself:
Seems like its not as clear as it could be in the doc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for testing it out! What version of esp-idf are you using? We could annotate your finding's that a delay is happening in Full-Duplex at least in your tested esp-idf version, but with a warning that it might not be a "stable" option to do so. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This week will be busy at work for reports etc... I will see if I have time, if not it will be next week! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its your PR -> your pacing 😄 |
||
self.cs_pre_delay_us = Some(delay_us); | ||
self | ||
} | ||
|
||
/// Add an aditional Amount of SPI bit-cycles the cs should be activated after the transmission (0-16). | ||
/// This only works on half-duplex transactions. | ||
#[must_use] | ||
pub fn cs_post_delay_us(mut self, delay_us: u8) -> Self { | ||
self.cs_post_delay_us = Some(delay_us); | ||
self | ||
} | ||
|
||
#[must_use] | ||
pub fn input_delay_ns(mut self, input_delay_ns: i32) -> Self { | ||
self.input_delay_ns = input_delay_ns; | ||
|
@@ -293,6 +313,8 @@ pub mod config { | |
cs_active_high: false, | ||
duplex: Duplex::Full, | ||
bit_order: BitOrder::MsbFirst, | ||
cs_pre_delay_us: None, | ||
cs_post_delay_us: None, | ||
input_delay_ns: 0, | ||
} | ||
} | ||
|
@@ -324,6 +346,8 @@ impl<T> SpiBusDriver<T> { | |
0_u32 | ||
} | config.duplex.as_flags() | ||
| config.bit_order.as_flags(), | ||
cs_ena_pretrans: config.cs_pre_delay_us.unwrap_or(0), | ||
cs_ena_posttrans: config.cs_post_delay_us.unwrap_or(0), | ||
..Default::default() | ||
}; | ||
|
||
|
@@ -693,6 +717,8 @@ where | |
0_u32 | ||
} | config.duplex.as_flags() | ||
| config.bit_order.as_flags(), | ||
cs_ena_pretrans: config.cs_pre_delay_us.unwrap_or(0), | ||
cs_ena_posttrans: config.cs_post_delay_us.unwrap_or(0), | ||
..Default::default() | ||
}; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you specify more than 16 'bit cycles', how does the api react? does it take 16 than? If not sure can you test it and report back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't change anything given my test (0/1/16/1000), at least in FullDuplex Mode. It behaves like if the value was the highest available.
Further testing on other boards and configs could be interesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you run a short test with half-duplex on your board and just write something and check if the delay is working overall as intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am doing some further testing at home on another oscilloscope and another ESP32 (its still a NodeMcu but instead of having written ESP32S on top its written ESP32-WROOM-32E). It behaves the same as the other one, but for scientific purpose I will redo all the tests.
Just to note that DMA doesn't work on Half-Duplex (I think this is normal and documented) (which I was activating using Fullduplex in my previous tests and no error was shown about it). I will post the results once I finish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for further looking into it. If you want to check out the C driver path. We are using the somewhat stable driver/spi C api, though it itself goes down a bit deeper and is somewhat splitted.