fwmanager: Add BootMonitor capability and GpioBootMonitor adapter

[chrysh]

BootMonitor reports a device's boot liveness as BootStatus in the api leaf crate; GpioBootMonitor implements it over one HAL GpioPort input line. MonitorError meets the core::error::Error bound and keeps the concrete HAL error reachable through source(). new() rejects empty pin masks; boot latches are cleared by the reset path, never the observer.

Closes 9elements#6.

(Replaces 9elements#20, which was opened against the fork's main by mistake.)

[embediver]

Lets make this #[non_exhaustive].

Also, do we need an Unknown variant?

[chrysh]

non_exhaustive: Don't we want every consumer to get a compile error when a value is added, so that they must consciously decide how to handle the new state. non_exhaustive forces consumers to write a _ arm, and since every BootStatus variant is decision-relevant for the orchestrator, a future variant silently falling into that arm seems worse than a breaking change.

[chrysh]

Same reasoning for Unknown: a hung device reports nothing, so "unknown" is already covered by the timeout path (Booting until the orchestrator gives up). It would hide a bad state.

Do you have strong opinions for either?

[embediver]

No strong opinions.
I'm fine with unknown always represented by the error path.
Maybe AMD folks have an opinion whether it should be non_exhaustive or not.

[rusty1968]

non_exhaustive: Don't we want every consumer to get a compile error when a value is added, so that they must consciously decide how to handle the new state. non_exhaustive forces consumers to write a _ arm, and since every BootStatus variant is decision-relevant for the orchestrator, a future variant silently falling into that arm seems worse than a breaking change.

I thought for a bit and exhaustive is right call here. Adding another variant should be a build error. The change should be one atomic commit that touches not only the enum but every consumer the compiler flags, reviewed together in a single PR.

[chrysh]

Agreed

[rusty1968]

BootStatus has four values: InReset, Booting,
Booted, and Failed. But this monitor only looks at a single wire and asks
one question — "is it on or off?" — so it can only ever answer Booting or
Booted:

Ok(if booted { BootStatus::Booted } else { BootStatus::Booting })

It never returns InReset or Failed. A device that's still held in reset looks exactly the same on that one wire as a device that's been let go but hasn't finished starting up — both read as Booting. And there's no way for a device to say "I tried to boot and failed" over a single ready wire, so Failed never comes up either. A consumer would assume from the return type that this API returns the wide enum.

[chrysh]

That's not wrong. How would you solve this issue best? Make it return a bool?

[rusty1968]

This can be solved with the rustdoc so the reader knows the intent. It is not a bug it is just for clarity.

[rusty1968]

BootStatus is the little list of values a boot monitor can report about a device: InReset, Booting, Booted, Failed. That said InReset doesn't belong on that list. It's not something a monitor can see — it's something the orchestrator already did. The list should just be Booting, Booted, Failed.

[rusty1968]

Document this only returns a subset.

/// Any given monitor may only ever produce a *subset* of [`BootStatus`],
/// depending on the signals it can access: a single ready pin yields only
/// `Booting`/`Booted`, while a fault-channel backend can also report
/// `Failed`. This is a capability difference between backends, not an
/// incomplete implementation. Consumers must still handle the full set —
/// they cannot know statically which backend they hold.

[rusty1968]

When you build a GpioBootMonitor, it takes the GPIO port and keeps it for itself — and that "port" is a whole bank of pins, not the single pin the
monitor actually watches. So the first monitor you build swallows the entire bank, and there's nothing left for a second one.

That's a problem whenever two devices put their boot-complete signals on different pins of the same bank, which is a normal way to wire a board. You'd
naturally want one monitor per device, but you can't: after the first GpioBootMonitor::new, the bank is gone.

It's not hypothetical. Our immediate target, the AST1060 SGPIOM, is a serial GPIO expander whose P is SgpiomBankPort — a whole 32-line register
bank, not a single pin. Because 32 unrelated lines are packed into each fixed bank, different devices' ready lines routinely land in the same bank by
construction.

[chrysh]

Good point. Fixed.

[rusty1968]

Consider borrowing the port instead of owning it.

Make the monitor hold a shared reference:

pub struct GpioBootMonitor<'a, P: GpioPort> {
    port: &'a P,
    ready_pin: P::Mask,
    active: ActivePolarity,
}

Because read_input(&self) already needs only &self, nothing else in the monitor has to change. Several monitors can now borrow the same bank at once:

let mon_a = GpioBootMonitor::new(&bank, pin3, ActiveHigh);
let mon_b = GpioBootMonitor::new(&bank, pin5, ActiveHigh);
let mon_c = GpioBootMonitor::new(&bank, pin7, ActiveHigh);

This directly matches the read-only nature of the adapter; removes the single-owner constraint with the smallest possible change.

The issue is it Introduces a lifetime parameter on the type, so the platform must keep the bank alive somewhere for at least as long as the monitors.

What is your take?

[chrysh]

That's a very good point. Shared &P only works because monitors read. If the same bank also carries output lines it will need &mut P. In this case the &mut of the controller and the & the monitor borrows conflict. In case this happens, we will have to do what SplitPort is doing.