Skip to content

Fix analogbufio.BufferedIn pin validation on RP2350B #10202

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

Merged
merged 2 commits into from
Mar 31, 2025
Merged

Fix analogbufio.BufferedIn pin validation on RP2350B #10202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8b55b4
Fix analogbufio.BufferedIn pin validation on RP2350B
dhalbert Mar 30, 2025
aa580ea
use CYW43_DEFAULT_PIN_WL_CLOCK when checking for CYW43 shared GPIO29 use
dhalbert Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions ports/raspberrypi/common-hal/analogbufio/BufferedIn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,21 @@
#include "hardware/dma.h"
#include "pico/stdlib.h"

#define ADC_FIRST_PIN_NUMBER 26
#define ADC_PIN_COUNT 4

#define ADC_CLOCK_INPUT 48000000
#define ADC_MAX_CLOCK_DIV (1 << (ADC_DIV_INT_MSB - ADC_DIV_INT_LSB + 1))

void common_hal_analogbufio_bufferedin_construct(analogbufio_bufferedin_obj_t *self, const mcu_pin_obj_t *pin, uint32_t sample_rate) {
// Make sure pin number is in range for ADC
if (pin->number < ADC_FIRST_PIN_NUMBER || pin->number >= (ADC_FIRST_PIN_NUMBER + ADC_PIN_COUNT)) {
raise_ValueError_invalid_pins();
if ((pin->number < ADC_BASE_PIN)
|| (pin->number > ADC_BASE_PIN + NUM_ADC_CHANNELS - 1)
// On many boards with a CYW43 radio co-processor, CYW43_DEFAULT_PIN_WL_CLOCK (usually GPIO29),
// is both a voltage monitor and also SPI SCK to the CYW43.
// Disallow its use for BufferedIn.
#if defined(CIRCUITPY_CYW43) && defined(CYW43_DEFAULT_PIN_WL_CLOCK)
|| (pin->number == CYW43_DEFAULT_PIN_WL_CLOCK)
#endif
) {
raise_ValueError_invalid_pin();
}

// Validate sample rate here
Expand All @@ -35,7 +40,7 @@ void common_hal_analogbufio_bufferedin_construct(analogbufio_bufferedin_obj_t *s
claim_pin(pin);

// TODO: find a way to accept ADC4 for temperature
self->chan = pin->number - ADC_FIRST_PIN_NUMBER;
self->chan = pin->number - ADC_BASE_PIN;

// Init GPIO for analogue use: hi-Z, no pulls, disable digital input buffer.
// TODO: Make sure we share the ADC well. Right now we just assume it is
Expand Down
22 changes: 7 additions & 15 deletions ports/raspberrypi/common-hal/analogio/AnalogIn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,12 @@

#include "hardware/adc.h"

#define ADC_PIN_COUNT (NUM_ADC_CHANNELS - 1)

#if ADC_PIN_COUNT == 4
#define ADC_FIRST_PIN_NUMBER 26
#else
#define ADC_FIRST_PIN_NUMBER 40
#endif

// Voltage monitor is special on Pico W, because this pin is shared between the
// voltage monitor function and the wifi function. Special handling is required
// to read the analog voltage.
// On many boards with a CYW43 radio co-processor, CYW43_DEFAULT_PIN_WL_CLOCK (usually GPIO29),
// is both a voltage monitor and also SPI SCK to the CYW43.
// Special handling is required to read the analog voltage.
#if CIRCUITPY_CYW43
#include "bindings/cyw43/__init__.h"
#define SPECIAL_PIN(pin) (pin->number == 29)
#define SPECIAL_PIN(pin) (pin->number == CYW43_DEFAULT_PIN_WL_CLOCK)

const mcu_pin_obj_t *common_hal_analogio_analogin_validate_pin(mp_obj_t obj) {
return validate_obj_is_free_pin_or_gpio29(obj, MP_QSTR_pin);
Expand All @@ -35,7 +27,7 @@ const mcu_pin_obj_t *common_hal_analogio_analogin_validate_pin(mp_obj_t obj) {
#endif

void common_hal_analogio_analogin_construct(analogio_analogin_obj_t *self, const mcu_pin_obj_t *pin) {
if (pin->number < ADC_FIRST_PIN_NUMBER || pin->number > ADC_FIRST_PIN_NUMBER + ADC_PIN_COUNT) {
if (pin->number < ADC_BASE_PIN || pin->number > ADC_BASE_PIN + NUM_ADC_CHANNELS - 1) {
raise_ValueError_invalid_pin();
}

Expand Down Expand Up @@ -70,15 +62,15 @@ uint16_t common_hal_analogio_analogin_get_value(analogio_analogin_obj_t *self) {
uint32_t old_pad = pads_bank0_hw->io[self->pin->number];
uint32_t old_ctrl = io_bank0_hw->io[self->pin->number].ctrl;
adc_gpio_init(self->pin->number);
adc_select_input(self->pin->number - ADC_FIRST_PIN_NUMBER);
adc_select_input(self->pin->number - ADC_BASE_PIN);
common_hal_mcu_delay_us(100);
value = adc_read();
gpio_init(self->pin->number);
pads_bank0_hw->io[self->pin->number] = old_pad;
io_bank0_hw->io[self->pin->number].ctrl = old_ctrl;
common_hal_mcu_enable_interrupts();
} else {
adc_select_input(self->pin->number - ADC_FIRST_PIN_NUMBER);
adc_select_input(self->pin->number - ADC_BASE_PIN);
value = adc_read();
}
// Stretch 12-bit ADC reading to 16-bit range
Expand Down
7 changes: 6 additions & 1 deletion shared-bindings/analogbufio/BufferedIn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,12 @@
//| """Create a `BufferedIn` on the given pin and given sample rate.
//|
//| :param ~microcontroller.Pin pin: the pin to read from
//| :param ~int sample_rate: rate: sampling frequency, in samples per second"""
//| :param ~int sample_rate: rate: sampling frequency, in samples per second
//|
//| **Limitations**: On many boards with a CYW43 radio module, such as Pico W,
//| GPIO29 (often ``board.A3``) is also used to control the CYW43,
//| and is therefore not available to use as the `BufferedIn` pin.
//| """
//| ...
//|
static mp_obj_t analogbufio_bufferedin_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
Expand Down