feat(diamond): add security MCU reboot request#475
Conversation
fouge
force-pushed
the
cyril-orbs-943-security-mcu-reboot-request-from-main-mcu
branch
from
115bec4 to
28e8f79
Compare
fouge
force-pushed
the
cyril-orbs-943-security-mcu-reboot-request-from-main-mcu
branch
3 times, most recently
from
b55ff26 to
0797548
Compare
There was a problem hiding this comment.
Pull request overview
Adds a Diamond-specific mechanism for requesting a security MCU reboot by pulsing a new expander GPIO (EXP_MCU_RST_RQST_3V3) and exposes the request via both Jetson messaging and the orb CLI.
Changes:
- Update
orb-messagesmodule revision to pick up the newreboot_security_mcumessage tag. - Add
exp-mcu-rst-rqst-gpiostodiamond_mainDTS underzephyr,user. - Implement
handle_reboot_security_mcu()inrunner.cand addorb reboot_secCLI command (Diamond only).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
west.yml |
Bumps orb-messages revision to include the new reboot request message. |
main_board/src/runner/runner.c |
Adds Diamond-only handler to pulse the reset-request GPIO and wires it into the message callback table. |
main_board/src/cli/cli.c |
Adds Diamond-only orb reboot_sec command that enqueues the new message. |
boards/tfh/diamond_main/diamond_main.dts |
Declares the GPIO used to request the security MCU reboot on Diamond. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ret = gpio_pin_set_dt(&exp_mcu_rst_rqst_gpio_spec, 0); | ||
| if (ret != 0) { | ||
| LOG_ERR("Failed to set GPIO low: %d", ret); | ||
| job_ack(orb_mcu_Ack_ErrorCode_FAIL, job); | ||
| return; | ||
| } |
There was a problem hiding this comment.
If gpio_pin_set_dt(..., 0) fails, the function returns without attempting any further cleanup, leaving the reset-request line potentially stuck asserted. That can continuously request/hold the security MCU in reset (depending on the receiving circuitry) and makes the failure mode worse than the original error.
Smallest safe fix: ensure the GPIO is deasserted in a best-effort cleanup path (e.g., track whether it was driven high and always attempt to drive it low before returning, even when returning an error).
| return; | ||
| } | ||
|
|
||
| k_msleep(100); |
There was a problem hiding this comment.
k_msleep(100) runs inside the single runner_process_jobs_thread() consumer context. This blocks processing of all other incoming jobs for 100ms and can cause the 8-deep process_queue to fill, leading to transient RET_ERROR_BUSY drops for unrelated commands/messages.
Smallest safe fix: avoid sleeping in the runner thread (e.g., assert the GPIO, schedule a delayed work/timer to deassert it after 100ms, and return/ack immediately).
| ret = gpio_pin_configure_dt(&exp_mcu_rst_rqst_gpio_spec, | ||
| GPIO_OUTPUT_INACTIVE); | ||
| if (ret != 0) { |
There was a problem hiding this comment.
This GPIO is only configured at the moment a reboot is requested. Until then, the expander pin may be left in its reset/default state, which can be board-dependent and risks unintended pulses/levels on the reset-request signal at boot.
Smallest safe fix: configure the pin to a known inactive output state during initialization (gated by HW version >= DIAMOND_V4_5), and have the handler only perform the 100ms pulse.
for Diamond v4.5+ Allow the Jetson or CLI to request a security MCU reboot by having the main MCU drive EXP_MCU_RST_RQST_3V3 GPIO high for 1000ms. - Add exp-mcu-rst-rqst-gpios to diamond_main DTS (gpio_exp1 pin 11) - Add handle_reboot_security_mcu handler in runner.c - Add `orb reboot_sec` CLI command (Diamond only)
fouge
force-pushed
the
cyril-orbs-943-security-mcu-reboot-request-from-main-mcu
branch
from
0797548 to
f2b295f
Compare
Allow the Jetson or CLI to request a security MCU reboot by having the main MCU drive EXP_MCU_RST_RQST_3V3 GPIO high for 100ms.
orb reboot_secCLI command (Diamond only)