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

Add memorymap support to RP2 port #8357

Merged
merged 7 commits into from
Sep 8, 2023
Merged

Add memorymap support to RP2 port #8357

merged 7 commits into from
Sep 8, 2023

Conversation

eightycc
Copy link

@eightycc eightycc commented Sep 1, 2023

Resolves #8334 by adding memorymap support to the RP2 port. Also makes these additional changes:

  • Changed memorymap hal function prototypes to use size_t rather than uint32_t arguments, allowing prototypes to work with different port word sizes.
  • Changes memorymap.AddressRange() start argument from signed to an unsigned int to allow ranges to be specified anywhere in the entire port address map. Added support to the runtime for unsigned int arguments.
  • Added wrap-around checking for memorymap.AddressRange() arguments.
  • Tweaked Python commentary embedded in memorymap to reflect reality of single operation accesses.

@eightycc
Copy link
Author

eightycc commented Sep 1, 2023

CI found some breakage. I entirely missed the fact that there are multiple runtimes to consider when adding unsigned integer argument support. Also missed updating the nrf port with the uint32_t -> size_t change. Working...

@eightycc
Copy link
Author

eightycc commented Sep 1, 2023

@tannewt over to you for review. Thanks!

@eightycc
Copy link
Author

eightycc commented Sep 5, 2023

After tuning in to the meeting today my understanding of 8.2.x vs. main branch has improved. @tannewt would you like me to rework this pull request for 8.2.x?

@tannewt
Copy link
Member

tannewt commented Sep 5, 2023

After tuning in to the meeting today my understanding of 8.2.x vs. main branch has improved. @tannewt would you like me to rework this pull request for 8.2.x?

Only if you want to see it in an 8.x release. Otherwise it's fine on main.

Copy link
Member

@tannewt tannewt 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 PR! Just a couple questions/suggestions.

ports/raspberrypi/common-hal/memorymap/AddressRange.c Outdated Show resolved Hide resolved
py/runtime.h Outdated Show resolved Hide resolved
@eightycc
Copy link
Author

eightycc commented Sep 5, 2023

On second thought, let's leave this on main branch.

@eightycc eightycc requested a review from tannewt September 7, 2023 04:06
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Two more minor things. Thanks for the updates!

locale/circuitpython.pot Outdated Show resolved Hide resolved
locale/circuitpython.pot Outdated Show resolved Hide resolved
@eightycc eightycc requested a review from tannewt September 7, 2023 23:47
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you!

@tannewt tannewt merged commit 885dbec into adafruit:main Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RP2040 lacks raw memory map access
2 participants