-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
ports/rp2: Added init and reset C hooks for RP2040, and increased external pin count #12283
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
Conversation
2a857c3
to
b88262c
Compare
Could there be a means to include a C header, for hook code that requires it? Or some knack to it? In my case - for PicoSystem - I might want // Apply a modest overvolt, default is 1.10v.
// this is required for a stable 250MHz on some RP2040s
vreg_set_voltage(VREG_VOLTAGE_1_20);
sleep_ms(10);
set_sys_clock_khz(250000, true); But that depends upon: #include "hardware/vreg.h But yeah, +1 to this change, it could benefit PicoSystem and potentially PicoVision. |
I think that may already be covered by this addition, as I do #include "hardware/gpio.h"
#include "hardware/i2c.h" within my Yukon build. See: https://github.com/pimoroni/yukon/blob/main/firmware/PIMORONI_YUKON/board.c The functions in this file are hooked in by: #define MICROPY_BOARD_EARLY_INIT board_init
void board_init(void);
#define MICROPY_BOARD_EARLY_RESET board_reset
void board_reset(void); |
Hi. Just wondering if anyone had a chance to look at this yet? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ZodiusInfuser,
Thanks for being patient while I got around to looking at this.
I think these are useful changes, and thanks for contributing them.
The max EXT_PINs thing is good to go (I was initially cautious until I realised this only raises the maximum to 32, so doesn't change anything on the current boards!)
For the new C hooks, these are great to have on rp2. My only concern is consistency between ports. See the inline comments for specific thoughts.
HI @projectgus, Thank you for taking a look at this.
Glad to hear it!
I hadn't specifically spotted the STM etc hooks, so apologies for their inconsistency. I will have a look and see what can be changed and respond to your specific notes later. I will say, I am only actively using the below hooks in the product this PR was primarily raised for. #define MICROPY_BOARD_EARLY_INIT board_init
#define MICROPY_BOARD_EARLY_RESET board_reset The others were included to account for possible future use-cases, so I agree they should be consistent. |
b88262c
to
ae6c33a
Compare
Code size report:
|
@ZodiusInfuser Are you still working on this PR? Is there anything you're waiting on from the MicroPython side? |
Errr... I'll admit I completely forgot about this open PR after our Pimoroni Yukon was launched 😅. It is still necessary as that product's builds still rely on my Looks like I never addressed your review points before. I'll try and find time this week to look at it, if not it will be after I am back from holiday. Also tangentially, I noticed in #if defined(MICROPY_PY_NETWORK_CYW43) && defined(MICROPY_HW_PIN_EXT_COUNT)
should be changed to: #if CYW43_GPIO == 1 && defined(MICROPY_PY_NETWORK_CYW43) && defined(MICROPY_HW_PIN_EXT_COUNT)
to support any future Pico W-based Yukon I may attempt. Would that be something I could include in this PR, or am I better to raise it as a separate issue / PR? |
ae6c33a
to
7234375
Compare
Signed-off-by: Christopher Parrott <chris@pimoroni.com>
Signed-off-by: Christopher Parrott <chris@pimoroni.com>
7234375
to
d131d20
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, thanks for updating and I hope it makes maintenance easier for the Yukon and other boards!
This |
This looks good. Thanks for making it more consistent with the stm32 port. |
Following discussions in https://github.com/orgs/micropython/discussions/12116, this pull request makes changes primarily to support the upcoming Pimoroni Yukon board, but that could also benefit others in the community.
I have tested these successfully with a custom build, that includes the board definitions and pulls in drivers for the IO expanders so the external pins can work. I have left these changes out of this PR, as they are currently quite intrusive.