Rationalize board header pin defines, and add partner board headers #192
No reviewers
Labels
No labels
bazel
bluetooth
board settings
boot_stage2
bug
build
c++
can't reproduce
clang
clib-interface
cmsis
code cleanliness
code size
compiler version issues
cyw43_driver
debug
discussion
documentation
duplicate
dynamic_clock
elf2uf2
enhancement
errata
filesystem
flash layout
floating point
FreeRTOS
hardware_adc
hardware_claim
hardware_clocks
hardware_dma
hardware_exception
hardware_flash
hardware_gpio
hardware_i2c
hardware_irq
hardware_pio
hardware_pwm
hardware_rcp
hardware_rtc
hardware_spi
hardware_sync
hardware_uart
hardware_vreg
hardware_watchdog
hardware_xosc
help wanted
host
IAR
installation
invalid
linker scripts
lto
lwip
mbedtls
missing API
needs reporter detail
networking
newlib
newlib-multicore
non-c
non-cmake
non-gcc
openocd
orphan PR
OS integration
pico_async_context
pico_base
pico_binary_info
pico_flash
picolibc
pico_lwip
pico_malloc
pico_multicore
pico_printf
pico_runtime
pico_stdio
pico_stdio_usb
pico_sync
pico_time
pico_util
pioasm
port
question
review
review2
rtos interop
runtime init
static library
svd
test
tinyusb
vscode
wifi integration
wontfix
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-raspberrypi/pico-sdk!192
Loading…
Reference in a new issue
No description provided.
Delete branch "board-headers"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -32,0 +35,4 @@#ifndef PICO_DEFAULT_I2C#define PICO_DEFAULT_I2C 0#endif#ifndef PICO_DEFAULT_I2C_SDA_PINDo we want to designate "default I2C" pins for Raspberry Pi Pico, given that there's no such designation of default pins on the pinout diagram at https://www.raspberrypi.org/documentation/pico/getting-started/ ? ping also @aallan
@ -0,0 +1,68 @@/** Copyright (c) 2020 Raspberry Pi (Trading) Ltd.** SPDX-License-Identifier: BSD-3-ClauseI'm obviously not a legal-expert 😉 but does the BSD-3-Clause require a
<YEAR>and<COPYRIGHT HOLDER>? Or is it safe to assume those from the owner-of-the-repo and the date-of-commit?@ -12,22 +12,6 @@#ifndef _BOARDS_VGABOARD_HNice 👍
...although I guess replacing
#define PICO_DEFAULT_UART_TX_PIN 20with#include "boards/pico.h"means that any programs built for vgaboard now have a differentPICO_DEFAULT_UART_TX_PIN? (although perhaps this doesn't matter? 🤷 .... although looking at page 26 of https://datasheets.raspberrypi.org/rp2040/hardware-design-with-rp2040.pdf I see that it does break out GPIO20 (as UART1_TX) to a UART header.)@ -56,6 +56,16 @@ extern "C" {#include "pico/stdio_semihosting.h"#endif// PICO_CONFIG: PICO_DEFAULT_LED_PIN, Optionally defined a pin that drives a regular LED on the board, default=undefined, group=pico_stdlibSee e.g. https://github.com/raspberrypi/pico-sdk/pull/183/commits/9b05c241a86e46ad1e8aa8831e159440e23bc232
I wonder if it's better to simply omit
default, rather than havingdefault=undefined?EDIT: Or I could add special-handling to our workflows for
default=undefinedif you think that'd work better?@ -56,6 +56,16 @@ extern "C" {#include "pico/stdio_semihosting.h"#endif// PICO_CONFIG: PICO_DEFAULT_LED_PIN, Optionally defined a pin that drives a regular LED on the board, default=undefined, group=pico_stdlibThe comments on #191 kinda imply we might also want an optional boolean
PICO_DEFAULT_LED_PIN_ACTIVE_HIGHconfig-setting? (defaulting to 1, can be set to 0 for those boards with an active-low LED)EDIT: Now added as
PICO_DEFAULT_LED_PIN_INVERTEDWhy remove the warning? Are we expecting RP2040 boards to appear with no Flash fitted at all?
LOL :)
This may be a silly question, but why the separate
uart_defaultdefine rather than just continuing to usePICO_DEFAULT_UART_INSTANCE?just seemed friendlier
I removed it in favor of a warning where it is used
@ -55,9 +57,9 @@ void stdout_uart_init() {}AFAICT this is "inside" the outer
#if defined(uart_default) && ...block, which suggests that the|| !defined(uart_default)here is redundant?Something else I was wondering earlier today... would it be possible to have a compile-time
#errorif you give an invalid combination of UART_DEFAULT settings? e.g. setting {PICO_DEFAULT_UART=0, PICO_DEFAULT_UART_TX_PIN=13} (invalid because GPIO13 is UART0 RX) or (PICO_DEFAULT_UART=1, PICO_DEFAULT_UART_TX_PIN=0) (invalid because GPIO0 is UART0 TX)(with similar things being done for I2C and SPI) ... hmmm perhaps I should create this as a separate issue for the next SDK release? 🙂
EDIT: Created #204 for this.
The fact that this is tied specifically to the
uart_default(rather than taking auart_inst_tparameter as other functions do) seems a bit limiting, so I did agit grepand this function doesn't actually seem to be used anywhere, so could probably be removed?...oh, I see it's actually used in https://github.com/raspberrypi/pico-examples/blob/master/clocks/hello_resus/hello_resus.c Does that "make sense" (I know nothing about resus) and does that mean that the hello_resus example won't work if the USB stdio is used instead? 🤷
@ -343,3 +343,3 @@static inline void sm_config_set_mov_status(pio_sm_config *c, enum pio_mov_status_type status_sel, uint status_n) {valid_params_if(PIO, join >= STATUS_TX_LESSTHAN && join <= STATUS_RX_LESSTHAN);valid_params_if(PIO, status_sel >= STATUS_TX_LESSTHAN && status_sel <= STATUS_RX_LESSTHAN);c->execctrl = (c->execctrlOMG, sorry I didn't spot that! ☹️
@ -56,6 +56,16 @@ extern "C" {#include "pico/stdio_semihosting.h"#endifHaha, I suspect
PICO_DEFAULT_LED_PIN_PICO_DEFAULT_LED_PIN_INVERTEDought to be justPICO_DEFAULT_LED_PIN_INVERTED😜@ -48,6 +48,10 @@ extern "C" {IIRC SDA is bidirectional, so I suspect the "I2C TX SDA" here should have been "I2C SDA PIN" ?
@ -32,0 +35,4 @@#ifndef PICO_DEFAULT_I2C#define PICO_DEFAULT_I2C 0#endif#ifndef PICO_DEFAULT_I2C_SDA_PINthis is just to indicate an I2C to use by default
@ -12,22 +12,6 @@#ifndef _BOARDS_VGABOARD_Hoops
@ -56,6 +56,16 @@ extern "C" {#include "pico/stdio_semihosting.h"#endif// PICO_CONFIG: PICO_DEFAULT_LED_PIN, Optionally defined a pin that drives a regular LED on the board, default=undefined, group=pico_stdlibthe intention here is to indicate that the value is omitted rather than having a default value
@ -55,9 +57,9 @@ void stdout_uart_init() {}fixed
@ -56,6 +56,16 @@ extern "C" {#include "pico/stdio_semihosting.h"#endiffixed
@ -32,0 +35,4 @@#ifndef PICO_DEFAULT_I2C#define PICO_DEFAULT_I2C 0#endif#ifndef PICO_DEFAULT_I2C_SDA_PINIn that case I wonder if it might be worth highlighting them with a slightly different colour on https://www.raspberrypi.org/documentation/pico/getting-started/static/15243f1ffd3b8ee646a1708bf4c0e866/Pico-R3-Pinout.svg as we do for the UART function on physical pins 1 and 2? 🤷
fixes #134
Needs rebasing as GH is reporting a conflict in
src/rp2_common/hardware_flash/include/hardware/flash.h.And I'm still not convinced about having
default=undefinedin the PICO_CONFIG lines, but we can address that in a subsequent PR.Otherwise LGTM 👍
@ -0,0 +1,91 @@/*Mistake in the header here -- these GPIOs are both for UART1.
@ -0,0 +1,91 @@/*Ahhh, so perhaps https://github.com/raspberrypi/pico-sdk/pull/192#issuecomment-786318489 wasn't such a crazy idea after all 😉 I'll create an issue for it.
open a separate PR
@ -0,0 +1,68 @@/** Copyright (c) 2020 Raspberry Pi (Trading) Ltd.** SPDX-License-Identifier: BSD-3-Clausefixed
@ -0,0 +1,91 @@/*fixed
Copyright years probably ought to be 2021 rather than 2020, but I'm not gonna quibble that as this PR has already seen enough comments.
Dates could be 2021 but not critical