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 simple UART bootloader example #571

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -44,6 +44,7 @@ App|Description
App|Description
---|---
[enc_bootloader](bootloaders/encrypted) | A bootloader which decrypts binaries from flash into SRAM. See the separate [README](bootloaders/encrypted/README.md) for more information
[uart_boot](bootloaders/uart) | A bootloader which boots a separate RP2350 using the UART boot interface. See section 5.8 in the datasheet for more details, including the wiring requirements

### Clocks

2 changes: 2 additions & 0 deletions bootloaders/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
add_subdirectory_exclude_platforms(uart host rp2040)

if (TARGET pico_mbedtls)
add_subdirectory_exclude_platforms(encrypted host rp2040 rp2350-riscv)
else()
32 changes: 32 additions & 0 deletions bootloaders/uart/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
add_executable(uart_boot
uart_boot.c
)

# pull in common dependencies
target_link_libraries(uart_boot pico_stdlib hardware_flash)

pico_embed_pt_in_binary(uart_boot ${CMAKE_CURRENT_LIST_DIR}/uart-pt.json)
pico_set_uf2_family(uart_boot "absolute")

# create map/bin/hex file etc.
pico_add_extra_outputs(uart_boot)

# add url via pico_set_program_url
example_auto_set_url(uart_boot)


add_executable(uart_binary
uart_binary.c
)

# pull in common dependencies
target_link_libraries(uart_binary pico_stdlib)

pico_set_binary_type(uart_binary no_flash)
pico_package_uf2_output(uart_binary 0x10000000)

# create map/bin/hex/uf2 file etc.
pico_add_extra_outputs(uart_binary)

# call pico_set_program_url to set path to example on github, so users can find the source for an example via picotool
example_auto_set_url(uart_binary)
23 changes: 23 additions & 0 deletions bootloaders/uart/uart-pt.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"version": [1, 0],
"unpartitioned": {
"families": ["absolute"],
"permissions": {
"secure": "rw",
"nonsecure": "rw",
"bootloader": "rw"
}
},
"partitions": [
{
"start": "128K",
"size": "32K",
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is intended as a "generic UART bootloader", should we increase the size of this partition to allow for bigger UF2s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that this simple bootloader just loads the whole partition onto the other device, so the larger the partition the longer it takes. From a quick play it takes 0.35s for this 32kiB partition, vs 5.6s for a full 512kiB partition (which would be the maximum size, as that fills the RP2350 SRAM)

Copy link
Contributor

@lurch lurch Nov 20, 2024

Choose a reason for hiding this comment

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

Ah, that makes sense. I guess in theory a more complicated UART bootloader could look at the IMAGE_DEF and only copy the part of the partition that it needed to?

I've not played with partitions on the RP2350 yet, but what happens if you try drag'n'dropping a 128kiB image into a partition that's only 32kiB big?

EDIT: And in hindsight I've just realised that this can't be a "generic UART bootloader" anyway, because it forcefully resets the other RP2350 if it doesn't receive the string "Hello" over the UART 😀

"families": ["rp2350-arm-s"],
"permissions": {
"secure": "rw",
"nonsecure": "rw",
"bootloader": "rw"
}
}
]
}
65 changes: 65 additions & 0 deletions bootloaders/uart/uart_binary.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/**
* Copyright (c) 2020 Raspberry Pi (Trading) Ltd.
*
* SPDX-License-Identifier: BSD-3-Clause
*/

#include "pico/stdlib.h"
#include "hardware/uart.h"
#include "hardware/structs/pads_qspi.h"
#include "hardware/structs/io_qspi.h"

#ifndef LED_DELAY_MS
#define LED_DELAY_MS 500
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I understand, I think the "parent" RP2350 will reset the "child" RP2350 if it doesn't receive the string "Hello" once per second; so I guess that means that LED_DELAY_MS can't be larger than 500?

#endif

// Initialize the GPIO for the LED
void pico_led_init(void) {
#ifdef PICO_DEFAULT_LED_PIN
// A device like Pico that uses a GPIO for the LED will define PICO_DEFAULT_LED_PIN
// so we can use normal GPIO functionality to turn the led on and off
gpio_init(PICO_DEFAULT_LED_PIN);
gpio_set_dir(PICO_DEFAULT_LED_PIN, GPIO_OUT);
#endif
}

// Turn the LED on or off
void pico_set_led(bool led_on) {
#if defined(PICO_DEFAULT_LED_PIN)
// Just set the GPIO on or off
gpio_put(PICO_DEFAULT_LED_PIN, led_on);
#endif
}

// Set function for QSPI GPIO pin
void qspi_gpio_set_function(uint gpio, gpio_function_t fn) {
// Set input enable on, output disable off
hw_write_masked(&pads_qspi_hw->io[gpio],
PADS_QSPI_GPIO_QSPI_SD2_IE_BITS,
PADS_QSPI_GPIO_QSPI_SD2_IE_BITS | PADS_QSPI_GPIO_QSPI_SD2_OD_BITS
);
// Zero all fields apart from fsel; we want this IO to do what the peripheral tells it.
// This doesn't affect e.g. pullup/pulldown, as these are in pad controls.
io_qspi_hw->io[gpio].ctrl = fn << IO_QSPI_GPIO_QSPI_SD2_CTRL_FUNCSEL_LSB;

// Remove pad isolation now that the correct peripheral is in control of the pad
hw_clear_bits(&pads_qspi_hw->io[gpio], PADS_QSPI_GPIO_QSPI_SD2_ISO_BITS);
}

int main() {
pico_led_init();

// SD2 is QSPI GPIO 3, SD3 is QSPI GPIO 4
qspi_gpio_set_function(3, GPIO_FUNC_UART_AUX);
qspi_gpio_set_function(4, GPIO_FUNC_UART_AUX);

uart_init(uart0, 1000000);

while (true) {
uart_puts(uart0, "Hello, world\n");
pico_set_led(true);
sleep_ms(LED_DELAY_MS);
pico_set_led(false);
sleep_ms(LED_DELAY_MS);
}
}
152 changes: 152 additions & 0 deletions bootloaders/uart/uart_boot.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
#include <stdio.h>
#include <stdlib.h>
#include "pico/stdlib.h"
#include "hardware/uart.h"
#include "pico/bootrom.h"
#include "boot/picobin.h"
#include "hardware/flash.h"

// UART defines for uart boot
#define UART_ID uart1
#define BAUD_RATE 1000000
Copy link
Contributor

Choose a reason for hiding this comment

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

The baud-rate is hardcoded in uart_binary.c (because I assume that it can't be changed, as it's the baud-rate required by the bootrom) so should it also be hardcoded (rather than being a #define) in uart_boot.c too?

Although I guess that the uart-bootloader baud rate could theoretically be different to the post-boot application baud rate (IYSWIM); so I wonder if having them deliberately different might make this a more interesting example? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should be hardcoded in both - I've fixed that

If the post-boot application used a different baud rate, then it wouldn't be able to boot the board again if it's power cycled - this way you can power cycle the separate device and watch the bootloader run again without rebooting the main device

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I was assuming that in order to accommodate my suggestion, the code (in the main device) would need to dynamically reconfigure the baud rate at the start and end of the uart-boot sequence 🙂


// Use pins 4 and 5 for uart boot
#define UART_TX_PIN 4
#define UART_RX_PIN 5

// Use pin 3 for the RUN pin on the other chip
#define RUN_PIN 3


void reset_chip() {
// Toggle run pin
gpio_put(RUN_PIN, false);
sleep_ms(1);
gpio_put(RUN_PIN, true);
}


void uart_boot() {
uint knocks = 0;
while (true) {
// Send the knock sequence
uart_putc_raw(UART_ID, 0x56);
uart_putc_raw(UART_ID, 0xff);
uart_putc_raw(UART_ID, 0x8b);
uart_putc_raw(UART_ID, 0xe4);
uart_putc_raw(UART_ID, 'n');

if (uart_is_readable_within_us(UART_ID, 1000)) {
char in = uart_getc(UART_ID);
printf("%c\n", in);
break;
} else {
if (knocks > 10) {
printf("No response - resetting\n");
reset_chip();
return;
}
printf("No response - knocking again\n");
knocks++;
}
}

printf("Boot starting\n");

// Get partition location in flash
const int buf_words = (16 * 4) + 1; // maximum of 16 partitions, each with maximum of 4 words returned, plus 1
uint32_t* buffer = malloc(buf_words * 4);

int ret = rom_get_partition_table_info(buffer, buf_words, PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_SINGLE_PARTITION | (0 << 24));
Copy link
Contributor

@lurch lurch Nov 14, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what's gone wrong, but it looks like none of the RP2350 functions are included in that documentation - the doxygen is all there, but maybe the #if !PICO_RP2040 halfway down the src/rp2_common/pico_bootrom/include/pico/bootrom.h file has something to do with it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll have a fiddle later... rom_get_partition_table_info isn't appearing in https://datasheets.raspberrypi.com/pico/raspberry-pi-pico-c-sdk.pdf either! 😱

assert(buffer[0] == (PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_SINGLE_PARTITION));
assert(ret == 3);
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the magic value 3 indicate? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one isn't actually a magic value - it's just the number of words written in the buffer (which should be 3 here - 1 word for the returned flags, and 2 words for the partition_location_and_flags)

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I missed that 😆 My eyes just jumped straight to the bottom of the doxygen description looking for the \return ... line 😜


uint32_t location_and_permissions = buffer[1];
uint32_t saddr = XIP_BASE + ((location_and_permissions >> PICOBIN_PARTITION_LOCATION_FIRST_SECTOR_LSB) & 0x1fffu) * FLASH_SECTOR_SIZE;
uint32_t eaddr = XIP_BASE + (((location_and_permissions >> PICOBIN_PARTITION_LOCATION_LAST_SECTOR_LSB) & 0x1fffu) + 1) * FLASH_SECTOR_SIZE;
printf("Start %08x, end %08x\n", saddr, eaddr);

free(buffer);

printf("Writing binary\n");
uint32_t tstart = time_us_32();
uint32_t caddr = saddr;
while (caddr < eaddr) {
uart_putc_raw(UART_ID, 'w');
char *buf = (char*)caddr;
for (int i=0; i < 32; i++) {
uart_putc_raw(UART_ID, buf[i]);
}
if (!uart_is_readable_within_us(UART_ID, 500)) {
// Detect hangs and reset the chip
printf("Write has hung - resetting\n");
reset_chip();
return;
}
char in = uart_getc(UART_ID);
printf("%c\n", in);
caddr += 32;
}

uint32_t tend = time_us_32();
printf("Write took %dus\n", tend - tstart);
printf("Write complete - executing\n");
uart_putc_raw(UART_ID, 'x');
if (!uart_is_readable_within_us(UART_ID, 500)) {
// Detect hangs and reset the chip
printf("Execute has hung - resetting\n");
reset_chip();
return;
}
char in = uart_getc(UART_ID);
printf("%c\n", in);
}


int main()
{
stdio_init_all();

// Set up our UART for booting the other device
uart_init(UART_ID, BAUD_RATE);
gpio_set_function(UART_TX_PIN, GPIO_FUNC_UART);
gpio_set_function(UART_RX_PIN, GPIO_FUNC_UART);

// Set up run pin
gpio_init(RUN_PIN);
gpio_set_dir(RUN_PIN, GPIO_OUT);

// Reset chip
reset_chip();


while (true) {
char splash[] = "RP2350";
char hello[] = "Hello";
Comment on lines +190 to +191
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nitpick: I guess these two variables could be moved outside of the while (true) loop?

char buf[500] = {0};
int i = 0;
while (uart_is_readable(UART_ID) && i < sizeof(buf)) {
char in = uart_getc(UART_ID);
printf("%c", in);
buf[i] = in;
i++;
}
if (i > 0) {
printf(" ...Read done\n");
}
char *ptr = memchr(buf, 'R', sizeof(buf));
if (ptr && strncmp(ptr, splash, sizeof(splash) - 1) == 0) {
printf("Splash found\n");
uart_boot();
} else {
ptr = memchr(buf, 'H', sizeof(buf));
if (ptr && strncmp(ptr, hello, sizeof(hello) - 1) == 0) {
printf("Device is running\n");
} else {
printf("Device not running - attempting reset\n");
reset_chip();
}
}
sleep_ms(1000);
}
}
Loading