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

boards: hal_rpi_pico hardware_xip_cache not included #84554

Merged
merged 1 commit into from
Jan 31, 2025
Merged

boards: hal_rpi_pico hardware_xip_cache not included #84554

merged 1 commit into from
Jan 31, 2025

Conversation

DxPru
Copy link
Contributor

@DxPru DxPru commented Jan 25, 2025 •
edited by kartben
Loading

When building with "CONFIG_PICOSDK_USE_FLASH" hardware_xip_cache was not included.

@zephyrbot zephyrbot added the platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) label Jan 25, 2025
@zephyrbot zephyrbot requested review from soburi and yonsch January 25, 2025 18:46
@github-actions GitHub Actions
Copy link

Hello @DxPru, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

soburi
soburi reviewed Jan 26, 2025
View reviewed changes
modules/hal_rpi_pico/CMakeLists.txt Outdated
@@ -42,8 +42,8 @@ if(CONFIG_HAS_RPI_PICO)
BUILD_ALWAYS TRUE
)

zephyr_library_add_dependencies(second_stage_bootloader)
zephyr_library_sources(${rp2_bootloader_asm})
zephyr_library_add_dependencies(second_stage_bootloader)
Copy link
Member

Choose a reason for hiding this comment

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

Do not change anything that is not relevant to the fix.

@soburi soburi added the DNM This PR should not be merged (Do Not Merge) label Jan 26, 2025
@soburi
Copy link
Member

soburi commented Jan 26, 2025

This change also need to modify hal_rpi_pico.
Please update west.yml after zephyrproject-rtos/hal_rpi_pico#9 solved.

@kartben
Copy link
Collaborator

kartben commented Jan 31, 2025 •
edited
Loading

This change also need to modify hal_rpi_pico. Please update west.yml after zephyrproject-rtos/hal_rpi_pico#9 solved.

does it?
this seems to be fixing a huge amount of failing tests for rp2040 boards (basically anything that has flash or hwinfo enabled), and doesn't seem to depend on anything in the HAL from what i can see?

Example of a broken test:

west build -p -b w5500_evb_pico/rp2040 tests/drivers/hwinfo/api  -T drivers.hwinfo.api

@DxPru @kartben
boards: hal_rpi_pico hardware_xip_cache not included
ded8d84 
include hardware_xip_cache when CONFIG_PICOSDK_USE_FLASH is set

Signed-off-by: Dominik Prütting <[email protected]>
@kartben
Copy link
Collaborator

kartben commented Jan 31, 2025

fixed the compliance issues

@zephyrbot zephyrbot requested a review from ThreeEights January 31, 2025 07:43
@kartben kartben requested a review from soburi January 31, 2025 07:43
@soburi
Copy link
Member

soburi commented Jan 31, 2025

This change also need to modify hal_rpi_pico. Please update west.yml after zephyrproject-rtos/hal_rpi_pico#9 solved.

does it? this seems to be fixing a huge amount of failing tests for rp2040 boards (basically anything that has flash or hwinfo enabled), and doesn't seem to depend on anything in the HAL from what i can see?

Example of a broken test:

west build -p -b w5500_evb_pico/rp2040 tests/drivers/hwinfo/api  -T drivers.hwinfo.api

I checked. Indeed, there is no need to change HAL at now.
In the RP2350 case does require HAL modification, but since it does not yet have flash functionality, it is not an issue at the moment.
Thank you confirmation.

@soburi soburi removed the DNM This PR should not be merged (Do Not Merge) label Jan 31, 2025
@soburi soburi mentioned this pull request Jan 31, 2025
Closed
@kartben kartben merged commit 4e7ed9d into zephyrproject-rtos:main Jan 31, 2025
25 of 26 checks passed
@github-actions GitHub Actions
Copy link

Hi @DxPru!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soburi soburi soburi approved these changes

@kartben kartben kartben approved these changes

@yonsch yonsch Awaiting requested review from yonsch

@ThreeEights ThreeEights Awaiting requested review from ThreeEights

Assignees

@soburi soburi

Labels
platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DxPru @soburi @kartben @zephyrbot