Skip to content

rpi_pico: Add hwinfo support #42558

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 6 commits into from
Mar 16, 2022
Merged

Conversation

yonsch
Copy link
Collaborator

@yonsch yonsch commented Feb 7, 2022

Adds hwinfo support for the rpi_pico series. Additionally, support for calling bootrom functions was added.

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Generally looks good.

Minor request regarding compiler flag.

Comment on lines 94 to 99
# A function in flash.c adds 1 to a function pointer, causing a warning
set_source_files_properties(
${rp2_common_dir}/hardware_flash/flash.c
PROPERTIES
COMPILE_FLAGS "-Wno-pointer-arith"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

good that disabling of such warning is local to the file in question.

However, we generally try to avoid adding compiler specific flags directly on sources / libs as they may not be supported on all compilers.

Could you add an extra entry in those two files, and then fetch the property before calling set_source_files_properties():
https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/compiler/compiler_flags_template.cmake
https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/compiler/gcc/compiler_flags.cmake

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tejlmand done

{
/* The reset register can't be erased, nothing to do. */

return 0;
Copy link
Member

Choose a reason for hiding this comment

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

-ENOSUP would be more precise here.

@alexanderwachter
Copy link
Member

@yonsch I have to apologize. I told you the wrong errno.
-ENOSYS is the correct one. The API doc is not up to date.

@github-actions github-actions bot added the area: API Changes to public APIs label Feb 22, 2022
tejlmand
tejlmand previously approved these changes Mar 3, 2022
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm

@yonsch
Copy link
Collaborator Author

yonsch commented Mar 7, 2022

@alexanderwachter Can you take another look?

Copy link
Member

@alexanderwachter alexanderwachter left a comment

Choose a reason for hiding this comment

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

Thanks for the doc change!

yonsch added 5 commits March 9, 2022 08:53
Add a compiler property for disabling pointer arithmetic warnings,
and implement that property for GCC.

Signed-off-by: Yonatan Schachter <[email protected]>
Support function and data lookup in the RPi's ROM.
The objects are obtained by including <pico/bootrom.h> and calling
rom_func_lookup or rom_data_lookup.

Signed-off-by: Yonatan Schachter <[email protected]>
Added support for hwinfo's hwinfo_get_device_id for the
RPi Pico series.

Signed-off-by: Yonatan Schachter <[email protected]>
Added support for hwinfo's reset cause functions, for the Raspberry Pi
Pico series.

Signed-off-by: Yonatan Schachter <[email protected]>
Adds HWINFO to rpi_pico.yaml and documentation

Signed-off-by: Yonatan Schachter <[email protected]>
Fix hwinfo documentation claiming API returns ENOTSUP for
unimplemented functions, with the correct ENOSYS

Signed-off-by: Yonatan Schachter <[email protected]>
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm - re-approved

@carlescufi carlescufi merged commit ad63b2b into zephyrproject-rtos:main Mar 16, 2022
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.

7 participants