Skip to content

Add warning for boards missing bootsel resistor #327

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.

Sign up for GitHub

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

Open
will-v-pi wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Add warning for boards missing bootsel resistor #327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43d029c
Add warning for boards missing bootsel resistor
will-v-pi Jun 18, 2026
a91bc96
Apply suggestions from code review
will-v-pi Jun 18, 2026
abee7f1
inline -> in series
will-v-pi Jun 19, 2026
f3799ff
Rename variables
will-v-pi Jun 19, 2026
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
3 changes: 1 addition & 2 deletions bintool/bintool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,11 +920,10 @@ std::vector<uint8_t> hash_andor_sign(std::vector<uint8_t> bin, uint32_t storage_


void verify_block(std::vector<uint8_t> bin, uint32_t storage_addr, uint32_t runtime_addr, block *block, model_t model, verified_t &hash_verified, verified_t &sig_verified, get_more_bin_cb more_cb) {
std::shared_ptr<load_map_item> load_map = block->get_item<load_map_item>();
std::shared_ptr<hash_def_item> hash_def = block->get_item<hash_def_item>();
hash_verified = none;
sig_verified = none;
if (load_map == nullptr || hash_def == nullptr) {
if (hash_def == nullptr) {
return;
}
std::vector<uint8_t> to_hash = get_lm_hash_data(bin, storage_addr, runtime_addr, block, more_cb, model);
Expand Down
38 changes: 34 additions & 4 deletions main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3416,12 +3416,16 @@ static chip_t image_type_exe_chip_to_chip(uint image_type_exe_chip) {
}

#if HAS_LIBUSB
void info_guts(memory_access &raw_access, picoboot::connection *con) {
void info_guts(memory_access &raw_access, picoboot::connection *con, bool no_pt_loaded=false) {
model_t model = raw_access.get_model();
#else
void info_guts(memory_access &raw_access, void *con) {
void info_guts(memory_access &raw_access, void *con, bool no_pt_loaded=false) {
#endif
// Use flash caching
settings.use_flash_cache = true;
// Track if a valid partition table was found in the block loop
bool has_pt = false;
bool pt_is_valid = true;
// Callback to pass to bintool, to get more bin data
get_more_bin_cb more_cb = [&raw_access](std::vector<uint8_t> &bin, uint32_t offset, uint32_t size) {
DEBUG_LOG("Now reading from %x size %x\n", offset, size);
Expand Down Expand Up @@ -3459,6 +3463,7 @@ void info_guts(memory_access &raw_access, void *con) {
}
};
auto info_metadata = [&](block *current_block, bool verbose_metadata = false) {
bool is_pt = false;
verified_t hash_verified = none;
verified_t sig_verified = none;
#if HAS_MBEDTLS
Expand Down Expand Up @@ -3514,6 +3519,8 @@ void info_guts(memory_access &raw_access, void *con) {
// Partition Table
auto partition_table = current_block->get_item<partition_table_item>();
if (partition_table != nullptr) {
is_pt = true; // this block is a partition table
has_pt = true; // assume it is valid - hash and sig checks will invalidate it if not
if (verbose_metadata) info_pair("block type", "partition table");
info_pair("partition table", partition_table->singleton ? "singleton" : "non-singleton");
std::stringstream unpartitioned;
Expand Down Expand Up @@ -3642,6 +3649,9 @@ void info_guts(memory_access &raw_access, void *con) {
}
info_pair("hash value", val.str());
}
if (is_pt && hash_verified != passed) {
pt_is_valid = false; // partition table has bad hash
}
}
if (sig_verified != none) {
info_pair("signature", sig_verified == passed ? "verified" : "incorrect");
Expand All @@ -3659,6 +3669,9 @@ void info_guts(memory_access &raw_access, void *con) {
}
info_pair("public key", pkey.str());
}
if (is_pt && sig_verified != passed) {
pt_is_valid = false; // partition table has bad signature
}
}
};

Expand Down Expand Up @@ -3879,7 +3892,6 @@ void info_guts(memory_access &raw_access, void *con) {
std::vector<std::pair<string,string>> device_state_pairs;
if ((settings.info.show_device || settings.info.all) && raw_access.is_device()) {
select_group(device_info);
model_t model = raw_access.get_model();
uint8_t rom_version;
raw_access.read_raw(0x13, rom_version);
info_pair("type", model->name());
Expand Down Expand Up @@ -4063,6 +4075,24 @@ void info_guts(memory_access &raw_access, void *con) {
} catch (not_mapped_exception&e) {
std::cout << "\nfailed to read memory at " << hex_string(e.addr) << "\n";
}

#if HAS_LIBUSB
if (no_pt_loaded && model->chip() != rp2040) {
if (has_pt && pt_is_valid) {

@lurch lurch Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like this could be

if (has_pt) {
    if (pt_is_valid) {
        ...
    } else {
        ...
    }
}

@will-v-pi will-v-pi Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I think I prefer the has_pt && pt_is_valid and has_pt && (!pt_is_valid) as it's clearer exactly what is being detected (as it's more similar to the variables I was using before) - it also reduces the level of indentation

@lurch lurch Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair enough 🙂

@lurch lurch Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alternatively, I guess the if (no_pt_loaded && model->chip() != rp2040) { line could be changed to if (has_pt && no_pt_loaded && model->chip() != rp2040) { and that way the indentation wouldn't change 😉 (because it seems like has_pt && no_pt_loaded is kinda what this PR is all about?)

fos.first_column(0); fos.hanging_indent(0);
fos << "\nWARNING: Device has no partition table loaded, but has a partition table at the start of flash.\n";
fos.first_column(sizeof("WARNING: ") - 1);
fos << "This may cause picotool to behave unpredictably.\n";
fos << "This could be due to using a custom board which is missing the resistor in series with the BOOTSEL button, ";
fos << "or if BOOT_FLAGS0.HASHED_PARTITION_TABLE or BOOT_FLAGS0.SECURE_PARTITION_TABLE is set in OTP and the partition table is not hashed/signed\n";
} else if (has_pt && (!pt_is_valid)) {
fos.first_column(0); fos.hanging_indent(0);
fos << "\nWARNING: Device has an incorrectly hashed/signed partition table at the start of flash.\n";
fos.first_column(sizeof("WARNING: ") - 1);
fos << "This may cause picotool to behave unpredictably.\n";
}
}
#endif
}

void config_guts(memory_access &raw_access) {
Expand Down Expand Up @@ -4545,7 +4575,7 @@ bool info_command::execute(device_map &devices) {
info_guts(access, &connection);
}
} else {
info_guts(access, &connection);
info_guts(access, &connection, true);
}
}
} else {
Expand Down
Loading