From 86c466b6bcba398319a99f49c5bf24fc405949f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niccol=C3=B2=20Maggioni?= <nicco.maggioni@gmail.com>
Date: Thu, 14 Aug 2025 18:31:58 +0200
Subject: [PATCH] tools/rp{2040,23xx}: Ensure that picotool is found or built
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The current logic failed silently on some systems, maybe because of a
different default shell? This resulted in builds that completed
successfully but generated invalid UF2 files, which were refused by
the uC bootloader.

Now the check is properly enforced and picotool is either found in
the $PATH or built from the pico-sdk source.

Signed-off-by: Niccolò Maggioni <nicco.maggioni+nuttx@gmail.com>
---
 tools/rp2040/Config.mk | 35 ++++++++++++++++++++---------------
 tools/rp23xx/Config.mk | 37 ++++++++++++++++++++++---------------
 2 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/tools/rp2040/Config.mk b/tools/rp2040/Config.mk
index 51bafdf6f40..54a00132506 100644
--- a/tools/rp2040/Config.mk
+++ b/tools/rp2040/Config.mk
@@ -26,7 +26,9 @@
 
 # POSTBUILD -- Perform post build operations
 
-PICOTOOL_BIN_PATH?=$(PICO_SDK_PATH)$(DELIM)_deps$(DELIM)picotool$(DELIM)picotool
+PICOTOOL_FOUND := $(shell command -v picotool 2> /dev/null)
+
+PICOTOOL_BIN_PATH ?= $(PICO_SDK_PATH)$(DELIM)_deps$(DELIM)picotool$(DELIM)picotool
 
 define GEN_PICO_UF2
   $(Q)echo "Generating: nuttx.uf2"; \
@@ -36,23 +38,26 @@ define GEN_PICO_UF2
 endef
 
 ifeq ($(CONFIG_RP2040_UF2_BINARY),y)
-  ifneq ($(shell which picotool),)
+  ifdef PICOTOOL_FOUND
     define POSTBUILD
       $(call GEN_PICO_UF2, picotool)
     endef
-  else ifdef PICO_SDK_PATH
-    define POSTBUILD
-      $(Q)(if ! $(PICOTOOL_BIN_PATH) help >&/dev/null; then \
-              echo "Building: picotool"; \
-              cd $(PICO_SDK_PATH); \
-              cmake . >&/dev/null; \
-              make picotoolBuild >&/dev/null; \
-           fi;)
-      $(call GEN_PICO_UF2, $(PICOTOOL_BIN_PATH))
-    endef
   else
-    define POSTBUILD
-      $(Q) echo "PICO_SDK_PATH/picotool must be specified/installed for flash boot"
-    endef
+    ifdef PICO_SDK_PATH
+      define POSTBUILD
+        $(warning "picotool not found in $$PATH, it will be sourced from pico-sdk")
+        $(Q)if [[ ! -x "$(PICOTOOL_BIN_PATH)" ]]; then \
+          echo "Warning: building picotool from pico-sdk will skip USB support! See https://github.com/raspberrypi/pico-sdk/issues/1827" >&2; \
+          cd $(PICO_SDK_PATH); \
+          cmake . >&/dev/null; \
+          make picotoolBuild >/dev/null; \
+        fi
+        $(call GEN_PICO_UF2, $(PICOTOOL_BIN_PATH))
+      endef
+    else
+      define POSTBUILD
+        $(error "Generating UF2 files requires picotool to be available in $$PATH, or $$PICO_SDK_PATH must be specified")
+      endef
+    endif
   endif
 endif
diff --git a/tools/rp23xx/Config.mk b/tools/rp23xx/Config.mk
index b8fbfbef493..4226782b8df 100644
--- a/tools/rp23xx/Config.mk
+++ b/tools/rp23xx/Config.mk
@@ -1,6 +1,8 @@
 ############################################################################
 # tools/rp23xx/Config.mk
 #
+# SPDX-License-Identifier: Apache-2.0
+#
 # Licensed to the Apache Software Foundation (ASF) under one or more
 # contributor license agreements.  See the NOTICE file distributed with
 # this work for additional information regarding copyright ownership.  The
@@ -24,7 +26,9 @@
 
 # POSTBUILD -- Perform post build operations
 
-PICOTOOL_BIN_PATH?=$(PICO_SDK_PATH)$(DELIM)_deps$(DELIM)picotool$(DELIM)picotool
+PICOTOOL_FOUND := $(shell command -v picotool 2> /dev/null)
+
+PICOTOOL_BIN_PATH ?= $(PICO_SDK_PATH)$(DELIM)_deps$(DELIM)picotool$(DELIM)picotool
 
 define GEN_PICO_UF2
   $(Q)echo "Generating: nuttx.uf2"; \
@@ -34,23 +38,26 @@ define GEN_PICO_UF2
 endef
 
 ifeq ($(CONFIG_RP23XX_UF2_BINARY),y)
-  ifneq ($(shell which picotool),)
+  ifdef PICOTOOL_FOUND
     define POSTBUILD
       $(call GEN_PICO_UF2, picotool)
     endef
-  else ifdef PICO_SDK_PATH
-    define POSTBUILD
-      $(Q)(if ! $(PICOTOOL_BIN_PATH) help >&/dev/null; then \
-              echo "Building: picotool"; \
-              cd $(PICO_SDK_PATH); \
-              cmake . >&/dev/null; \
-              make picotoolBuild >&/dev/null; \
-           fi;)
-      $(call GEN_PICO_UF2, $(PICOTOOL_BIN_PATH))
-    endef
   else
-    define POSTBUILD
-      $(Q) echo "PICO_SDK_PATH/picotool must be specified/installed for flash boot"
-    endef
+    ifdef PICO_SDK_PATH
+      define POSTBUILD
+        $(warning "picotool not found in $$PATH, it will be sourced from pico-sdk")
+        $(Q)if [[ ! -x "$(PICOTOOL_BIN_PATH)" ]]; then \
+          echo "Warning: building picotool from pico-sdk will skip USB support! See https://github.com/raspberrypi/pico-sdk/issues/1827" >&2; \
+          cd $(PICO_SDK_PATH); \
+          cmake . >&/dev/null; \
+          make picotoolBuild >/dev/null; \
+        fi
+        $(call GEN_PICO_UF2, $(PICOTOOL_BIN_PATH))
+      endef
+    else
+      define POSTBUILD
+        $(error "Generating UF2 files requires picotool to be available in $$PATH, or $$PICO_SDK_PATH must be specified")
+      endef
+    endif
   endif
 endif