Skip to content

Use cyw43_delay_ms() in cyw43_spi_reset() instead of sleep_ms() #2431

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

Merged
merged 1 commit into from
May 8, 2025

Conversation

mbrase
Copy link

@mbrase mbrase commented Apr 18, 2025

Since cyw43_spi_reset() may be executed from an async context, we should use cyw43_delay_ms() instead of sleep_ms(). This is particularly a problem when using the async_context_threadsafe_background backend, because sleep_ms() will assert in an ISR.

Since cyw43_spi_reset() may be executed from an async context, we
should use cyw43_delay_ms() instead of sleep_ms(). This is particularly
a problem when using the async_context_threadsafe_background backend,
because sleep_ms() will assert in an ISR.
@kilograham kilograham added this to the 2.1.2 milestone Apr 19, 2025
@mbrase
Copy link
Author

mbrase commented Apr 21, 2025

For additional context, this is the code that I'm running that crashes without this patch:

#include "pico/cyw43_arch.h"

void StartBlinkingLed() {
  static bool state = false;
  static async_at_time_worker_t worker = {
      .next = nullptr,
      .do_work =
          [](async_context_t* context, async_at_time_worker_t* worker) {
            bool& state = *static_cast<bool*>(worker->user_data);
            state = !state;
            cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, state);
            async_context_add_at_time_worker_in_ms(context, worker, 1000);
          },
      .next_time = 0,
      .user_data = &state,
  };
  cyw43_arch_init();
  async_context_add_at_time_worker_in_ms(
      cyw43_arch_async_context(), &worker, 50);
}

Copy link
Contributor

@peterharperuk peterharperuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be ok. Ran some tests and everything worked.

@kilograham kilograham merged commit 792f556 into raspberrypi:develop May 8, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants