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

Changes for pico2_w #1816

Merged
merged 10 commits into from
Sep 18, 2024
Merged

Conversation

peterharperuk
Copy link
Contributor

@peterharperuk peterharperuk commented Aug 13, 2024

Changes for Pico 2 W

@lurch
Copy link
Contributor

lurch commented Aug 13, 2024

The changes I made in #1814 are happy with this new header-file 😃

@peterharperuk peterharperuk force-pushed the pico2_w_changes branch 2 times, most recently from ad9d321 to 6eb230a Compare August 14, 2024 18:43
@peterharperuk peterharperuk force-pushed the pico2_w_changes branch 4 times, most recently from f2f34e5 to 5715e24 Compare August 15, 2024 19:00
@dpgeorge
Copy link

I have a question about board naming, in particular whether there's a space in the "Pico" board names or not.

There are currently the following board definition files in this repo:

include/boards/pico.h:   #define RASPBERRYPI_PICO
include/boards/pico_w.h: #define RASPBERRYPI_PICO_W
include/boards/pico2.h:  #define RASPBERRYPI_PICO2

And then this PR adds:

include/boards/pico2_w.h: #define RASPBERRYPI_PICO2_W

That seems nice and consistent. From MicroPython's side we have the corresponding:

  • RPI_PICO board with name "Raspberry Pi Pico"
  • RPI_PICO_W board with name "Raspberry Pi Pico W"
  • RPI_PICO2 board with name "Raspberry Pi Pico2"

And then I guess we would add:

  • RPI_PICO2_W board with name "Raspberry Pi Pico2 W"

The board name matters because that's the name of the source directory, eg ports/rp2/boards/RPI_PICO2. And also the name of the firmware, eg RPI_PICO2-20240816-v1.23.x.uf2.


My main question is whether there should be a space between "Pico" and "2", because:

  • it's referred to as "Raspberry Pi Pico 2" in almost all locations (with a space between "Pico" and "2"), for example in the RP2350 and Pico 2 datasheets
  • the datasheet for Pico 2 is called pico-2-datasheet.pdf, ie there's a hyphen between the "pico" and "2"
  • the URL is https://www.raspberrypi.com/products/raspberry-pi-pico-2/, ie has a hyphen

So that seems a little inconsistent, whether there's a space or not between the "Pico" and the "2", and then that would follow for the "Pico 2 W" (vs "Pico2 W").

I know this is a minor point, but we'd like to get the names correct in MicroPython so the board names, firmware names and download URL are correct and don't need to change in the future.

Should we call our boards:

  • RPI_PICO2 and RPI_PICO2_W
  • or RPI_PICO_2 and RPI_PICO_2_W?

@peterharperuk
Copy link
Contributor Author

peterharperuk commented Aug 16, 2024

Our board header files have...

#ifndef _BOARDS_PICO2_H
#define _BOARDS_PICO2_H

// For board detection
#define RASPBERRYPI_PICO2

So I think PICO2_W is consistent. I seriously considered PICO2W but that's inconsistent with PICO_W which doesn't quite work as PICOW. I'll ask around if there's a policy

Later: Officially, it's "Pico 2" and "Pico 2 W" with spaces. But I think we should leave things as they are.

This is a copy of pico2 with the definitions from pico_w added.
Set PICO_BOARD=pico2_w
cyw43_spi_init contains code to find a free PIO and state machine. This
can all be replaced with pio_claim_free_sm_and_add_program_for_gpio_range
The CYW43 gpio pins are currently hardcoded. Give the defines better
names and make them overrideable at build time.

Note: CYW43_PIN_WL_REG_ON and CYW43_PIN_WL_HOST_WAKE are already used by
the driver via cyw43_hal_* functions
Need to make sure the pio can work with all the gpios
Add CYW43_PIN_WL_DYNAMIC that means cyw43 gpio pins can be changed at
runtime.

Then CYW43_PIN_WL_* calls cyw43_get_pin_wl to get the gpio out of
the array.

cyw43_set_pins_wl can be used to change the cyw43 gpio pins although
care is needed when calling this?
@peterharperuk peterharperuk marked this pull request as ready for review August 28, 2024 09:49
@kilograham kilograham modified the milestone: 2.0.1 Aug 30, 2024
Add some PICO_CMAKE_CONFIG
Stop using gpio_*_mask64 functions
@peterharperuk
Copy link
Contributor Author

@kilograham I think I resolved all the review comments in this.

# PICO_CMAKE_CONFIG: CYW43_PIO_CLOCK_DIV_INT, integer component of pio clock divider used for cyw43 comms, type=int, default=2, group=pico_cyw43_driver
target_compile_definitions(cyw43_driver_picow INTERFACE CYW43_PIO_CLOCK_DIV_INT=${CYW43_PIO_CLOCK_DIV_INT})
endif()
if (CYW43_PIO_CLOCK_DIV_FRAC)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be _FRAC8 now, or probably we should add _FRAC8 too

Copy link
Contributor

Choose a reason for hiding this comment

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

actually let's leave this for now; i'll fix with #1926

@kilograham kilograham merged commit af2f426 into raspberrypi:develop Sep 18, 2024
1 check passed
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.

4 participants