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

Async SPI support is not straightforward #486

Open
Kernald opened this issue Sep 26, 2024 · 2 comments
Open

Async SPI support is not straightforward #486

Kernald opened this issue Sep 26, 2024 · 2 comments

Comments

@Kernald
Copy link

Kernald commented Sep 26, 2024

Currently, async support for SPI (and implementation of the embedded-hal-async traits) is dependent on CONFIG_SPI_MASTER_ISR_IN_IRAM being disabled:

esp-idf-hal/src/spi.rs

Lines 570 to 571 in 4f44787

#[cfg(not(esp_idf_spi_master_isr_in_iram))]
pub async fn read_async(&mut self, words: &mut [u8]) -> Result<(), EspError> {

There are two things:

  • The crate documentation doesn't make this obvious at all. One has to dig into the source to figure out why read_async and co aren't defined and figure out that it's a IDF setting.
  • Async SPI support should still be provided with the interrupt handler in IRAM.

The first point is pretty straightforward to address, but addressing the second would make that irrelevant - not sure how much effort the second point actually is.

@ivmarkov
Copy link
Collaborator

To make the driver's async machinery operable even when the C driver's ISR processing code is in IRAM, we need to make sure, that this function is placed in IRAM as well, because spi_notify would actually be called from within the SPI C driver ISR, so it should comply to the same restrictions - i.e. the closure of all code called from an ISR handler that pretends to be "in IRAM" needs to be in IRAM too.

@ivmarkov
Copy link
Collaborator

The above can be done with a simple rustc #[linker_section = "something-something-iram"]. We can also conditionalize this attribute with cfg_attr to only do that if esp_idf_spi_master_isr_in_iram holds.

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

No branches or pull requests

2 participants