Skip to content

Modernize EC to embedded-services main#33

Open
dymk wants to merge 1 commit into
OpenDevicePartnership:mainfrom
dymk:dymk/issue-14-ec-modernization
Open

Modernize EC to embedded-services main#33
dymk wants to merge 1 commit into
OpenDevicePartnership:mainfrom
dymk:dymk/issue-14-ec-modernization

Conversation

@dymk

@dymk dymk commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What

Bump odp-embedded-controller off the stale embedded-services@62d4ea9 pin onto current main (6d7dbbf), porting the shared dev-platform mock to:

  • the new battery Registration model — the OEM owns a Mutex<GlobalRawMutex, MockFuelGauge> and drives it directly via the FuelGauge trait; the service is bs::Service::new(bs::ArrayRegistration { fuel_gauges: [..] }). Replaces the removed Device / MockBattery / execute_event API.
  • the new spawn_service! init-closure form|resources| Service::new(resources, InitParams{..}) for the thermal sensor/fan services, and the positional tas::Service::new(resources, clock, ..) for time-alarm.

Why

The EC was 8 commits behind embedded-services main and could not consume new upstream features (e.g. uart_service::default_mctp_serial). This unblocks adopting the serial MCTP medium for the QEMU SP↔EC link.

Scope

  • Shared code: 4 files in platform/platform-common/src/mock/{battery,thermal,time_alarm,mod}.rs. No per-platform code changes — every dev platform consumes platform_common::mock only.
  • Per-platform: embedded-services lock bump on all 5 dev platforms; cargo-vet exemptions (crc 3.4.0, crc-catalog 2.5.0, embedded-mcu-hal 0.3.0) on the 4 CI platforms.
  • dev-qemu embedded-mcu-hal: time-alarm-service (from embedded-services main), embassy-qemu-riscv, and dev-qemu's own HID dep all resolve embedded-mcu-hal 0.3.0 from crates.io — previously they mixed crates.io and git sources of the identical crate, which collided on defmt's derive symbols under LTO (surfaces only in the release build, not cargo check).

Verification

  • All CI checks green — build/clippy/doc/machete/MSRV-1.83/feature-powerset on all platforms, cargo-vet on the 4 CI platforms, and the dev-qemu integration-test (exercises battery + thermal + time-alarm ec-test-cli commands at runtime over SmbusEspi).
  • Two-QEMU SP↔EC e2e (odp-platform-qemu-sbsa): built this EC --features sp-serial and ran the full UEFI ↔ FFA ↔ SP ↔ DSP0253-serial ↔ EC path — ffa_version, ffa_id_get, partition_discovery, and the EC-origin thermal_get_temperature round-trip all PASS.

Notes

  • dev-mec is not in the CI matrix and has no supply-chain/ store; it's lock-bumped (it consumes the shared platform-common) but gets no vet exemptions.
  • The battery mock now emits MockFuelGauge values rather than the old MockBattery constants — relevant to any test asserting specific battery readings.

Assisted-by: GitHub Copilot:claude-opus-4.8

@dymk dymk requested a review from a team as a code owner June 26, 2026 21:30
@dymk dymk requested review from Copilot, kat-perez, maxperga-msft, philgweber and rogurr and removed request for Copilot June 26, 2026 21:30
philgweber
philgweber previously approved these changes Jun 26, 2026
Comment thread platform/platform-common/src/mock/battery.rs Outdated
Comment thread platform/platform-common/src/mock/battery.rs Outdated
tullom
tullom previously approved these changes Jun 26, 2026

@tullom tullom left a comment

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.

Two nit nb comments, otherwise looks good

Comment thread platform/dev-qemu/Cargo.toml Outdated
Bump the embedded-services git pin off the stale 62d4ea9 onto current
main (6d7dbbf, which carries the new default_mctp_serial constructor),
porting the shared mock to the new battery Registration model and the
new spawn_service! init-closure form.

platform-common/src/mock:
- battery.rs: rewrite to the fuel-gauge Registration model. The OEM
  owns a Mutex<GlobalRawMutex, MockFuelGauge> and drives it directly;
  the service is bs::Service::new(ArrayRegistration{..}). Replaces the
  removed Device / MockBattery / execute_event API.
- thermal.rs: wrap the sensor and fan spawn_service! args in the new
  |resources| Service::new(resources, InitParams{..}) closures.
- time_alarm.rs: switch to the positional tas::Service::new closure.
- mod.rs: update the relay-handler battery type argument.

Bump all five platform locks (dev-mec was at 5cc395f1) and add
crc / crc-catalog / embedded-mcu-hal 0.3.0 cargo-vet exemptions to the
four CI platforms (dev-mec has no supply-chain store). No per-platform
code changes: every platform consumes platform_common::mock only.

dev-qemu unifies embedded-mcu-hal on crates.io: time-alarm-service
(from embedded-services main), embassy-qemu-riscv, and dev-qemu's own
HID dep all use the crates.io 0.3.0 rather than mixing crates.io and
git sources of the identical crate, which collide on defmt's derive
symbols under LTO (surfaces only in the release build, not cargo check).

Unblocks dropping the F1.1 fork [patch] override.

Assisted-by: GitHub Copilot:claude-opus-4.8
Copilot AI review requested due to automatic review settings June 26, 2026 22:12
@dymk dymk dismissed stale reviews from tullom and philgweber via 3563771 June 26, 2026 22:12
@dymk dymk force-pushed the dymk/issue-14-ec-modernization branch from aef5429 to 3563771 Compare June 26, 2026 22:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the EC firmware to track the current embedded-services main and ports the shared dev-platform mocks in platform-common to newer upstream service initialization APIs (battery registration + spawn_service! init-closure form), while updating platform lockfiles and cargo-vet supply-chain exemptions accordingly.

Changes:

  • Port platform_common::mock battery to the new fuel-gauge registration model (ArrayRegistration + OEM-owned Mutex<GlobalRawMutex, MockFuelGauge> driving via FuelGauge trait).
  • Update thermal and time-alarm mock service initialization to the new spawn_service! closure-based constructor pattern.
  • Bump embedded-services (and related git deps) across platform Cargo.lock files, plus add cargo-vet exemptions for new crates; switch dev-qemu to embedded-mcu-hal = "0.3.0".

Reviewed changes

Copilot reviewed 9 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
platform/platform-common/src/mock/battery.rs Replace removed mock battery/device API usage with mutex-backed MockFuelGauge registration + update task driving the gauge directly.
platform/platform-common/src/mock/thermal.rs Update spawn_service! calls to the new init-closure form for sensor + fan services.
platform/platform-common/src/mock/time_alarm.rs Update spawn_service! call to the new init-closure form and new tas::Service::new(...) signature.
platform/platform-common/src/mock/mod.rs Update relay handler macro instantiation to use the new BatteryService type alias.
platform/dev-qemu/Cargo.toml Switch embedded-mcu-hal dependency to crates.io 0.3.0.
platform/dev-qemu/Cargo.lock Update embedded-services pin to 6d7dbbf... and reflect updated dependency graph (including registry embedded-mcu-hal 0.3.0).
platform/dev-qemu/supply-chain/config.toml Add cargo-vet exemptions for crc 3.4.0, crc-catalog 2.5.0, and embedded-mcu-hal 0.3.0.
platform/dev-npcx/Cargo.lock Update embedded-services pin and dependency graph.
platform/dev-npcx/supply-chain/config.toml Add cargo-vet exemptions for crc, crc-catalog, embedded-mcu-hal.
platform/dev-mec/Cargo.lock Update embedded-services pin and dependency graph (non-CI platform noted in PR description).
platform/dev-mcxa/Cargo.lock Update embedded-services pin and dependency graph.
platform/dev-mcxa/supply-chain/config.toml Add cargo-vet exemptions for crc, crc-catalog, embedded-mcu-hal.
platform/dev-imxrt/Cargo.lock Update embedded-services pin and dependency graph.
platform/dev-imxrt/supply-chain/config.toml Add cargo-vet exemptions for crc, crc-catalog, embedded-mcu-hal.

Comment thread platform/dev-qemu/Cargo.toml
@dymk dymk enabled auto-merge (squash) June 26, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants