Skip to content

Conversation

@psiddh
Copy link
Contributor

@psiddh psiddh commented Jul 15, 2025

Test Plan:
1\ executorch/examples/rpi/build_firmware_pico.sh

2\ Flash the firmware

  • Hold BOOTSEL
  • Plug in Pico 2
  • Mounts as RPI-RP2
  • Copy executorch_pico.uf2 to that drive

3\ Check that LED light blinks 10 times (with 500 ms interval) , indicating that inference has been successful

Summary

[PLEASE REMOVE] See CONTRIBUTING.md's Pull Requests for ExecuTorch PR guidelines.

[PLEASE REMOVE] If this PR closes an issue, please add a Fixes #<issue-id> line.

[PLEASE REMOVE] If this PR introduces a fix or feature that should be the upcoming release notes, please add a "Release notes: " label. For a list of available release notes labels, check out CONTRIBUTING.md's Pull Requests.

Test plan

[PLEASE REMOVE] How did you test this PR? Please write down any manual commands you used and note down tests that you have written if applicable.

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 15, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/12518

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures

As of commit 34cdc6b with merge base b02db12 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 15, 2025
@psiddh psiddh self-assigned this Jul 15, 2025
@github-actions
Copy link

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@@ -0,0 +1,40 @@
cmake_minimum_required(VERSION 3.13)

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

@psiddh psiddh requested a review from digantdesai July 15, 2025 22:34

if(NOT DEFINED EXECUTORCH_BUILD_ARM_BAREMETAL)
# If not defined, assume we're building standalone
set(EXECUTORCH_BUILD_ARM_BAREMETAL ON)

This comment was marked as resolved.

This comment was marked as resolved.

@@ -0,0 +1,129 @@
#include "pico/stdio_usb.h"

This comment was marked as resolved.

This comment was marked as resolved.

}
}

int main() {

This comment was marked as resolved.

#include <executorch/runtime/platform/runtime.h>

// Declare your model data (from simple_addmodule_pte.c)
extern const uint8_t model_pte[] __attribute__((aligned(8)));

This comment was marked as resolved.

This comment was marked as resolved.

@@ -0,0 +1,38 @@
__attribute__((aligned(8))) const unsigned char model_pte[] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

OK for now, but for the tutorial we should generate --> pte to pte.h --> include.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added explicit comments in README on how to generate this header (also left comments in the file), This way Users can generate their own hex array and replace the contents

* We are adding a custom syscall_stubs.c file to provide dummy implementations for syscalls that are not
* available on the Pico platform. This is necessary because the Pico does not have an operating system,
* and therefore does not support standard C library functions like _exit, _sbrk, _read, etc.
* By adding these stubs, we can resolve linker errors that occur when building our project for the Pico.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

include(${CMAKE_SOURCE_DIR}/tools/cmake/Codegen.cmake)

target_sources(executorch_core PRIVATE ${CMAKE_CURRENT_LIST_DIR}/syscall_stubs.c)
gen_selected_ops(LIB_NAME "portable_ops_lib" ROOT_OPS "aten::add.out")

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

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

Looks great. Esp how standalone this is from rest of the ExecuTorch. Does it make sense for it to live in https://github.com/pytorch-labs/executorch-examples

TBH we can land in executorch/examples, I was thinking long term if we want to build more around it.

@psiddh psiddh force-pushed the pico2_poc branch 3 times, most recently from 94cabc6 to bed8352 Compare July 24, 2025 09:56
CMakeLists.txt Outdated

if(EXECUTORCH_BUILD_ARM_BAREMETAL)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/backends/arm)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/examples/arm/rp_pi)

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

CMakeLists.txt Outdated

if(EXECUTORCH_BUILD_ARM_BAREMETAL)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/backends/arm)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/examples/arm/rp_pi)

This comment was marked as resolved.


### (Pre-requisistes) Prepare the Environment for Arm
1. See <a href="docs/source/backends-arm-ethos-u.md"/> for instructions on setting up the environment for Arm.
2. Make sure you have the toolchain configured correctly.

This comment was marked as resolved.

This comment was marked as resolved.

Comment on lines 10 to 13
if(NOT DEFINED EXECUTORCH_BUILD_ARM_BAREMETAL)
# If not defined, assume we're building standalone
set(EXECUTORCH_BUILD_ARM_BAREMETAL ON)
endif()

This comment was marked as resolved.

This comment was marked as resolved.

)
add_compile_definitions(C10_USING_CUSTOM_GENERATED_MACROS)

# Set correct flags for Pico (Cortex-M0+)

This comment was marked as resolved.

This comment was marked as resolved.

${BAREMETAL_BUILD_DIR}/kernels/portable/libportable_ops_lib.a
-Wl,--no-whole-archive
${BAREMETAL_BUILD_DIR}/kernels/portable/libportable_kernels.a
pico_stdlib

This comment was marked as resolved.

This comment was marked as resolved.

Comment on lines 47 to 48
#set(CMAKE_C_FLAGS "-Os -ffunction-sections -fdata-sections")
set(CMAKE_EXE_LINKER_FLAGS "-Wl,--gc-sections")

This comment was marked as resolved.

This comment was marked as resolved.


// Make sure blink_indicator is defined somewhere, or add a stub if not
void blink_indicator(uint pin, int times, int delay_ms = 100) {
for (int i = 0; i < times; ++i) {

This comment was marked as resolved.

This comment was marked as resolved.

@@ -0,0 +1,47 @@
/* Copyright (c) Meta Platforms, Inc. and affiliates.

This comment was marked as outdated.

This comment was marked as outdated.

@psiddh psiddh force-pushed the pico2_poc branch 2 times, most recently from af70e69 to e5d6a3e Compare August 19, 2025 05:42
@psiddh psiddh marked this pull request as ready for review August 19, 2025 05:43
@psiddh psiddh force-pushed the pico2_poc branch 2 times, most recently from 78a6000 to d4f09ab Compare August 19, 2025 18:23
@psiddh psiddh requested a review from mergennachin August 19, 2025 20:29
@@ -0,0 +1,59 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not remember what we agreed on for the home for this? Given we might also enable this for RISCv (not just arm)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to examples/rpi

@@ -0,0 +1,111 @@
/* Copyright (c) Meta Platforms, Inc. and affiliates.
Copy link
Contributor

Choose a reason for hiding this comment

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

let'e generate the pte and use pte_to_header.py?

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 , forgot to check-in in last iter, this is script: pte_to_array.py. Users can pass any model of their choice , this script converts the pte to hex array and bakes it into final pico2 firmware.

@@ -0,0 +1,86 @@
## Overview
Copy link
Contributor

@digantdesai digantdesai Aug 20, 2025

Choose a reason for hiding this comment

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

Too much code in the readme for my taste. Likey get outdated and broken soon. I would write a script and ask users to use that. If you want we can put that script in CI as well (until PTE generation and runner bin building)

Copy link
Contributor Author

@psiddh psiddh Aug 26, 2025

Choose a reason for hiding this comment

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

Fixed README, all the steps now happens in this one single script : executorch/examples/rpi/build_firmware_pico.sh (forgot to add this script in the last iter)

Comment on lines 98 to 99
float input_data_0[4] = {4.0, 109.0, 13.0, 123.0};
float input_data_1[4] = {9.0, 27.0, 11.0, 8.0};
Copy link
Contributor

Choose a reason for hiding this comment

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

can we take these from bundleio? And also verify the output?
And take input index from cmdline (optionally)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • There is no cmdline interface for pico2 (baremetal ). I
  • have added the validation step in this iter,.
  • bundleio is a great idea. Do you think we can add bundleio support as followup change to this ?

Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

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

Thanks @psiddh. Looking much cleaner now. Left some more comments.

@@ -0,0 +1,59 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
# All rights reserved.
# Copyright 2023-2025 Arm Limited and/or its affiliates.
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this Copyright Arm? Did they contribute code this?

Copy link
Contributor Author

@psiddh psiddh Aug 26, 2025

Choose a reason for hiding this comment

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

Fixed. (removed)

@@ -0,0 +1,59 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, let's put in a dedicated raspberry pi folder under examples so that we can be flexible in the future

examples/rpi (or raspi)

@@ -0,0 +1,21 @@
/* Copyright (c) Meta Platforms, Inc. and affiliates.
* All rights reserved.
* Copyright 2023-2025 Arm Limited and/or its affiliates.
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.

Done

Test Plan:
1 Build pico2 firmware
  executorch/examples/rpi/build_firmware_pico.sh

2. Flash the firmware : 'executorch_pico.uf2'

3. Verify that it successfully runs / infers the simple add module

Reviewers:

Subscribers:

Tasks:

Tags:
@psiddh psiddh merged commit cfd9b68 into main Sep 5, 2025
114 of 116 checks passed
@psiddh psiddh deleted the pico2_poc branch September 5, 2025 07:26
Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

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

Let's try to address these comments before the branch cut?

@@ -0,0 +1,143 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit s/rpi/raspberrypi

@@ -0,0 +1,207 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
Copy link
Contributor

Choose a reason for hiding this comment

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

use the one we already have from arm. if you want you can make it a devtools utils.

# build_firmware_pico.sh
# Simple script to cross-compile ExecuTorch and build Pico2 firmware with optional model input

set -e
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
set -e
set -eu

cmake -B "${BUILD_DIR}" -DPICO_BOARD=pico2 -DINPUT_MODEL="./${DEFAULT_MODEL}" -DCMAKE_BUILD_TYPE=Release
fi

cmake --build "${BUILD_DIR}" -j$(nproc)
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW nproc won't work on a macos

@@ -0,0 +1,45 @@
# Overview
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make a web doc as well? it can be same content. Add a picture or gif too :)

@@ -0,0 +1,18 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to check these model_pte.[ch] files in?

}

void wait_for_usb() {
const int kMaxRetryCount = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

don't hardcode values, default args or consts may be? count as well as delay

executorch::extension::BufferDataLoader loader(model_pte, model_pte_len);
auto program_result = Program::load(&loader);
if (!program_result.ok()) {
printf("Failed to load model: %d\n", (int)program_result.error());
Copy link
Contributor

Choose a reason for hiding this comment

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

use ET_LOG? Do we need any PAL changes?

Comment on lines +97 to +98
float input_data_0[4] = {4.0, 109.0, 13.0, 123.0};
float input_data_1[4] = {9.0, 27.0, 11.0, 8.0};
Copy link
Contributor

Choose a reason for hiding this comment

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

Use bundled IO. Hardcoding input doesn't work well with random PTEs.

blink_indicator(INDICATOR_PIN_1, 10);
return false;
}
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

use bundle io and compare outputs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants