RP2350A-A2 USB BOOT activity LED workaround (#2971)
* Add docs and optional work around to rom_reset_usb_boot* for RP2350A2 QFN60 Arm to reboot into RISC-V when acitivity LED is requested
Also renamed rp2040_rom_version rp2350_rom_version for consistency (it doesn't appear to be used)
* remove changes from previous version of the fix
* reorder if
* oops
* Apply suggestions from code review
Co-authored-by: Andrew Scheller <lurch@durge.org>
Co-authored-by: will-v-pi <108662275+will-v-pi@users.noreply.github.com>
* reference correct errata
* Switch bootrom.h back to LF
* Apply suggestions from code review
Co-authored-by: will-v-pi <108662275+will-v-pi@users.noreply.github.com>
Co-authored-by: Andrew Scheller <lurch@durge.org>
* Rename PICO_BOOTROM_WORKAROUND_A2_ACTIVITY_LED_BUG->PICO_BOOTROM_WORKAROUND_RP2350_A2_ACTIVITY_LED_BUG and default it to 1 on RP2350
* Apply suggestions from code review
Co-authored-by: will-v-pi <108662275+will-v-pi@users.noreply.github.com>
---------
Co-authored-by: Andrew Scheller <lurch@durge.org>
Co-authored-by: will-v-pi <108662275+will-v-pi@users.noreply.github.com>
Co-authored-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
diff --git a/src/rp2350/pico_platform/include/pico/platform.h b/src/rp2350/pico_platform/include/pico/platform.h
index 67cb80e..5ade280 100644
--- a/src/rp2350/pico_platform/include/pico/platform.h
+++ b/src/rp2350/pico_platform/include/pico/platform.h
@@ -189,7 +189,7 @@
/*! \brief Returns the RP2350 chip revision number
* \ingroup pico_platform
- * @return the RP2350 chip revision number (1 for B0/B1, 2 for B2)
+ * @return the RP2350 chip revision number (2 for A2, 3 for A3/A4)
*/
uint8_t rp2350_chip_version(void);
@@ -201,11 +201,11 @@
return 2;
}
-/*! \brief Returns the RP2040 rom version number
+/*! \brief Returns the RP2350 rom version number
* \ingroup pico_platform
- * @return the RP2040 rom version number (1 for RP2040-B0, 2 for RP2040-B1, 3 for RP2040-B2)
+ * @return the RP2350 rom version number (2 for RP2350-A2, 3 for RP2350-A3, 4 for RP2350-A4)
*/
-static inline uint8_t rp2040_rom_version(void) {
+static inline uint8_t rp2350_rom_version(void) {
GCC_Pragma("GCC diagnostic push")
GCC_Pragma("GCC diagnostic ignored \"-Warray-bounds\"")
return *(uint8_t*)0x13;
diff --git a/src/rp2_common/pico_bootrom/bootrom.c b/src/rp2_common/pico_bootrom/bootrom.c
index 7665748..c9e59bf 100644
--- a/src/rp2_common/pico_bootrom/bootrom.c
+++ b/src/rp2_common/pico_bootrom/bootrom.c
@@ -31,6 +31,19 @@
return ok;
}
+// The activity LED on RP2350 A2 QFN60 chips doesn't work in Arm mode, so boot into RISC-V if the user
+// really, really wants the activity LED
+#if PICO_RP2350_A2_SUPPORTED && PICO_RP2350A && !PICO_RISCV && PICO_BOOTROM_WORKAROUND_RP2350_A2_ACTIVITY_LED_BUG
+#define rom_reboot_workaround(flags, delay_ms, p0, p1) ({ \
+ if (((p0) & BOOTSEL_FLAG_GPIO_PIN_SPECIFIED) && rp2350_rom_version() == 2) \
+ rom_reboot((flags) | REBOOT2_FLAG_REBOOT_TO_RISCV, delay_ms, p0, p1); \
+ else \
+ rom_reboot(flags, delay_ms, p0, p1); \
+})
+#else
+#define rom_reboot_workaround(flags, delay_ms, p0, p1) rom_reboot(flags, delay_ms, p0, p1)
+#endif
+
void __attribute__((noreturn)) rom_reset_usb_boot(uint32_t usb_activity_gpio_pin_mask, uint32_t disable_interface_mask) {
#ifdef ROM_FUNC_RESET_USB_BOOT
@@ -43,7 +56,7 @@
// the parameter is actually the gpio number, but we only care if BOOTSEL_FLAG_GPIO_PIN_SPECIFIED
usb_activity_gpio_pin_mask = (uint32_t)__builtin_ctz(usb_activity_gpio_pin_mask);
}
- rom_reboot(REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL | REBOOT2_FLAG_NO_RETURN_ON_SUCCESS, 10, flags, usb_activity_gpio_pin_mask);
+ rom_reboot_workaround(REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL | REBOOT2_FLAG_NO_RETURN_ON_SUCCESS, 10, flags, usb_activity_gpio_pin_mask);
__builtin_unreachable();
#else
panic_unsupported();
@@ -63,7 +76,7 @@
flags |= BOOTSEL_FLAG_GPIO_PIN_ACTIVE_LOW;
}
}
- rom_reboot(REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL | REBOOT2_FLAG_NO_RETURN_ON_SUCCESS, 10, flags, (uint)usb_activity_gpio_pin);
+ rom_reboot_workaround(REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL | REBOOT2_FLAG_NO_RETURN_ON_SUCCESS, 10, flags, (uint)usb_activity_gpio_pin);
__builtin_unreachable();
#else
panic_unsupported();
diff --git a/src/rp2_common/pico_bootrom/include/pico/bootrom.h b/src/rp2_common/pico_bootrom/include/pico/bootrom.h
index 667cece..6223d92 100644
--- a/src/rp2_common/pico_bootrom/include/pico/bootrom.h
+++ b/src/rp2_common/pico_bootrom/include/pico/bootrom.h
@@ -241,6 +241,13 @@
}
#pragma GCC diagnostic pop
+#if PICO_RP2350
+// PICO_CONFIG: PICO_BOOTROM_WORKAROUND_RP2350_A2_ACTIVITY_LED_BUG, Workaround RP2350A-A2 (QFN60) bug not displaying USB boot activity LED under Arm by making rom_reset_usb_boot functions reboot to RISC-V when the activity LED is specified, type=bool, default=1, group=pico_bootrom
+#ifndef PICO_BOOTROM_WORKAROUND_RP2350_A2_ACTIVITY_LED_BUG
+#define PICO_BOOTROM_WORKAROUND_RP2350_A2_ACTIVITY_LED_BUG 1
+#endif
+#endif
+
/*!
* \brief Reboot the device into BOOTSEL mode
* \ingroup pico_bootrom
@@ -250,7 +257,13 @@
* Facilities are provided to enable an "activity light" via GPIO attached LED for the USB Mass Storage Device,
* and to limit the USB interfaces exposed.
*
- * \param usb_activity_gpio_pin_mask 0 No pins are used as per a cold boot. Otherwise a single bit set indicating which
+ * \if rp2350_specific
+ * \note On RP2350A-A2 chips, errata RP2350-E3 prevents the activity LED working under Arm.
+ * PICO_BOOTROM_WORKAROUND_RP2350_A2_ACTIVITY_LED_BUG=1 is defined by default to have this method reboot to
+ * RISC-V USB boot to display the activity LED correctly.
+ * \endif
+ *
+ * \param usb_activity_gpio_pin_mask 0 No pins are used as per a cold boot. Otherwise, a single bit set indicating which
* GPIO pin should be set to output and raised whenever there is mass storage activity
* from the host.
* \param disable_interface_mask value to control exposed interfaces
@@ -272,13 +285,19 @@
* Facilities are provided to enable an "activity light" via GPIO attached LED for the USB Mass Storage Device,
* and to limit the USB interfaces exposed.
*
+ * \if rp2350_specific
+ * \note On RP2350A-A2 chips, errata RP2350-E3 prevents the activity LED working under Arm.
+ * PICO_BOOTROM_WORKAROUND_RP2350_A2_ACTIVITY_LED_BUG=1 is defined by default to have this method reboot to
+ * RISC-V USB boot to display the activity LED correctly.
+ * \endif
+ *
* \param usb_activity_gpio_pin GPIO pin to be used as an activitiy pin, or -1 for none
- * from the host.
* \param disable_interface_mask value to control exposed interfaces
* - 0 To enable both interfaces (as per a cold boot)
* - 1 To disable the USB Mass Storage Interface
* - 2 To disable the USB PICOBOOT Interface
- * \param usb_activity_gpio_pin_active_low Activity GPIO is active low (ignored on RP2040)
+ * \param usb_activity_gpio_pin_active_low Activity GPIO is active low (ignored on RP2040). A bug in the bootrom of RP2350
+ * A4 chips means this parameter has no effect on that version of the RP2350.
*/
void __attribute__((noreturn)) rom_reset_usb_boot_extra(int usb_activity_gpio_pin, uint32_t disable_interface_mask, bool usb_activity_gpio_pin_active_low);
