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

cpu/esp_common: FreeRTOS adaptation layer changes/extension required for ESP-IDF HAL/LL #18224

Merged

Conversation

gschorcht
Copy link
Contributor

Contribution description

As discussed in PR #17841, this PR is a split out of the changes and extensions that are required to use the ESP-IDF HAL or LL. It includes:

  • fixes for platform independent compilation (figured out in PR cpu/esp32: add support for ESP32-C3 SoC family #17844)
  • implementation of additionally required function xQueueReset
  • implementation of additionally required function xSemaphoreGetMutexHolder
  • implementation of additionally required functions pcTaskGetTaskName and uxTaskGetStackHighWaterMark
  • implementation of additionally required functions pvTaskGetThreadLocalStoragePointer and vTaskSetThreadLocalStoragePointerAndDelCallback
  • implementation of additionally required task notification functions xTaskNotify and xTaskNotifyWait
  • implementation of timeout handling in xQueue* functions
  • implementation of ringbuffer functions xRingbbuffer*

Testing procedure

Without using the ESP-IDF as provided by PR #17841, the only test is green CI.

Issues/PRs references

Split out of PR #17841

@gschorcht gschorcht requested a review from benpicco June 17, 2022 05:30
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Jun 17, 2022
@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 17, 2022
@benpicco benpicco requested a review from maribu June 17, 2022 08:43
@gschorcht gschorcht changed the title cpu/esp_common: FreeRtos adaptation layer changes/extension required for ESP-IDF HAL/LL cpu/esp_common: FreeRTOS adaptation layer changes/extension required for ESP-IDF HAL/LL Jun 17, 2022
@benpicco benpicco requested a review from kaspar030 June 19, 2022 14:30
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Thank you, I think this is pretty much alright.
I've been pretty busy the last couple of weeks, so had little time to review this. I hope this will get better soon.

Comment on lines 97 to 101
ifneq (,$(filter ztimer_msec,$(USEMODULE)))
USEMODULE += core_thread_flags
endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ifneq (,$(filter ztimer_msec,$(USEMODULE)))
USEMODULE += core_thread_flags
endif
ifneq (,$(filter esp_freertos_common,$(USEMODULE)))
ifneq (,$(filter ztimer_msec,$(USEMODULE)))
USEMODULE += core_thread_flags
endif
endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you suggest it for better documentation? esp_freertos_common is always used.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still hope we can make it a standalone module one day, so then this would not be forgotten but just copied over.

@benpicco
Copy link
Contributor

Just squash directly - if there are no regressions this should be good to merge.

@gschorcht gschorcht force-pushed the cpu/esp/freertos_changes_for_esp_idf_hal_ll branch from 98a3196 to a15e01e Compare June 20, 2022 17:36
@gschorcht gschorcht force-pushed the cpu/esp/freertos_changes_for_esp_idf_hal_ll branch from a15e01e to c8d8e5d Compare June 20, 2022 19:24
@benpicco benpicco merged commit 2895688 into RIOT-OS:master Jun 22, 2022
@gschorcht
Copy link
Contributor Author

@benpicco Thanks for reviewing and merging.

@gschorcht gschorcht deleted the cpu/esp/freertos_changes_for_esp_idf_hal_ll branch July 18, 2022 07:56
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants