Skip to content

Commit 51b9550

Browse files
committed
flash: remove WouldBlock from wire status and document server-internal retry semantics
1 parent 0bf8035 commit 51b9550

6 files changed

Lines changed: 173 additions & 26 deletions

File tree

drivers/flash/README.md

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Flash Driver Model
2+
3+
This document describes the architecture of the layered flash userspace driver
4+
under drivers/flash and how it integrates with platform server bindings.
5+
6+
## 1. Layer Overview
7+
8+
```
9+
┌──────────────────────────────────────────────────────────┐
10+
│ Application / Client Task │
11+
│ FlashClient (drivers/flash/client) │
12+
│ channel_transact(request) -> response │
13+
└────────────────────────┬─────────────────────────────────┘
14+
│ Pigweed IPC channel
15+
16+
┌──────────────────────────────────────────────────────────┐
17+
│ Server Binary (platform binding) │
18+
│ rust_app wires handles + backend + runtime loop │
19+
└────────────────────────┬─────────────────────────────────┘
20+
21+
22+
┌──────────────────────────────────────────────────────────┐
23+
│ Server Library (platform binding) │
24+
│ dispatch_request: protocol -> backend translation │
25+
│ runtime loop: channel_read -> dispatch -> respond │
26+
└────────────────────────┬─────────────────────────────────┘
27+
│ FlashBackend trait
28+
29+
┌──────────────────────────────────────────────────────────┐
30+
│ Platform Backend (platform binding) │
31+
│ PlatformFlashBackend : FlashBackend │
32+
└────────────────────────┬─────────────────────────────────┘
33+
34+
35+
┌──────────────────────────────────────────────────────────┐
36+
│ Controller Driver (SMC/FMC or equivalent) │
37+
│ Raw MMIO or HAL-level flash controller implementation │
38+
└──────────────────────────────────────────────────────────┘
39+
```
40+
41+
## 2. Crate Map
42+
43+
| Bazel target | Crate | Role |
44+
|---|---|---|
45+
| //drivers/flash/api:flash_api | flash_api | Wire protocol, error/status model, geometry types, backend trait contract |
46+
| //drivers/flash/client:flash_client | flash_client | Userspace IPC facade for read/write/erase/discovery |
47+
48+
The API/client layers are target-agnostic within Pigweed kernel userspace
49+
targets.
50+
51+
## 3. Wire Protocol (flash_api::protocol)
52+
53+
Operations are encoded by FlashOp in FlashRequestHeader (16 bytes,
54+
repr(C, packed), little-endian), with an optional payload up to
55+
MAX_PAYLOAD_SIZE (256 bytes).
56+
57+
| Op | Value | Request shape | Response shape |
58+
|---|---|---|---|
59+
| Exists | 0x01 | header only | value = 0/1 |
60+
| GetCapacity | 0x02 | header only | value = capacity bytes |
61+
| Read | 0x03 | address + length | payload = bytes read, value = byte count |
62+
| Write | 0x04 | address + length + payload | value = byte count |
63+
| Erase | 0x05 | address + length | success/error only |
64+
| GetGeometry | 0x06 | header only | payload = FlashGeometry |
65+
66+
FlashResponseHeader (8 bytes) carries status (FlashError), payload length,
67+
and an op-specific value word.
68+
69+
## 4. Backend Contract (flash_api::backend)
70+
71+
The server-side backend contract is defined by FlashBackend:
72+
73+
- info(route_key) -> FlashInfo
74+
- exists(route_key) -> Result<bool, BackendError>
75+
- read(route_key, address, out) -> Result<usize, BackendError>
76+
- write(route_key, address, data) -> Result<usize, BackendError>
77+
- erase(route_key, address, length) -> Result<(), BackendError>
78+
- enable_interrupts() / disable_interrupts()
79+
80+
Geometry discovery is exposed via FlashGeometryProvider, which can derive a
81+
default geometry from info() or be overridden by backends that need richer
82+
semantics.
83+
84+
## 5. Client Library (flash_client)
85+
86+
FlashClient is a synchronous userspace facade that:
87+
88+
- serializes request headers/payloads into caller-provided request buffers,
89+
- performs channel_transact with optional timeout,
90+
- parses/validates response headers and payload lengths,
91+
- maps transport and wire errors into ClientError.
92+
93+
Current client behavior and constraints:
94+
95+
- no_std, blocking IPC calls.
96+
- single call in flight per client instance (&mut self API).
97+
- per-call data cap is FlashClient::chunk_size() (MAX_PAYLOAD_SIZE).
98+
- explicit support for discovery calls: exists, capacity, geometry.
99+
100+
For detailed usage and method-level behavior, see:
101+
102+
- drivers/flash/api/README.md
103+
- drivers/flash/client/README.md
104+
105+
## 6. Error Surface
106+
107+
Wire-level status is FlashError. Common categories:
108+
109+
- protocol misuse: InvalidOperation, InvalidAddress, InvalidLength
110+
- runtime contention: Busy, Timeout
111+
- media/policy failures: IoError, NotPermitted
112+
- fallback: InternalError
113+
114+
ClientError wraps three classes of failure:
115+
116+
- IpcError(pw_status::Error): transport syscall failure
117+
- ServerError(FlashError): valid response reporting flash-level failure
118+
- InvalidResponse: malformed/truncated response frame
119+
- BufferTooSmall: local request/response buffer constraints
120+
121+
## 7. Integration Model
122+
123+
drivers/flash is split so protocol and client can evolve independently from
124+
platform backend/server bring-up:
125+
126+
- Keep wire schema and trait contract stable in flash_api.
127+
- Keep userspace ergonomics and parsing hardening in flash_client.
128+
- Implement server runtime and concrete backend per target tree.
129+
130+
This separation allows host-side validation of protocol and client logic before
131+
hardware-specific bindings are ready.
132+
133+
## 8. Extension Points
134+
135+
- New backend: implement FlashBackend (and optionally FlashGeometryProvider)
136+
in a platform crate, then bind server channel handles to route keys.
137+
- New operation: add FlashOp variant, define header/payload semantics, extend
138+
backend trait and server dispatch.
139+
- New geometry flags semantics: preserve wire compatibility by treating flags
140+
as opaque at protocol level and documenting interpretation at backend level.
141+
142+
## 9. Testing Focus
143+
144+
Recommended tests by layer:
145+
146+
- flash_api: wire layout, endian correctness, enum/status round-trips,
147+
short-buffer decode rejection.
148+
- flash_client: response validation paths, timeout behavior, chunk-size checks,
149+
and retry handling for Busy.
150+
- platform server/backend: operation dispatch, alignment rules, erase granule
151+
correctness, and hardware fault mapping to FlashError.

drivers/flash/api/README.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,15 @@ maps channel → backend → `RouteKey`.
192192
| `BufferTooSmall` | 0x04 | Server-side buffer constraint |
193193
| `Busy` | 0x05 | Backend busy |
194194
| `Timeout` | 0x06 | Operation timed out |
195-
| `WouldBlock` | 0x07 | Could not complete synchronously; retry after IRQ |
196-
| `IoError` | 0x08 | Media-level failure |
197-
| `NotPermitted` | 0x09 | Blocked by backend policy/protection |
195+
| `IoError` | 0x07 | Media-level failure |
196+
| `NotPermitted` | 0x08 | Blocked by backend policy/protection |
198197
| `InternalError` | 0xFF | Unclassified server fault |
199198

200199
`BackendError` is the trait-level error backends produce; an `impl
201200
From<BackendError> for FlashError` provides the canonical mapping for
202-
the server's response-encoding path.
201+
the server's response-encoding path. `BackendError::WouldBlock` is
202+
treated as backend-internal and maps to `FlashError::Busy` at the wire
203+
boundary.
203204

204205
## Tests
205206

drivers/flash/api/src/backend.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
//! The shape mirrors the `FlashStorage` HIL from caliptra-mcu-sw but is
77
//! synchronous and buffer-borrowing rather than callback-based: the
88
//! server runtime drives concurrency, so backends only need to expose
9-
//! a blocking-or-`WouldBlock` surface.
9+
//! a blocking-or-`WouldBlock` surface. `WouldBlock` is backend-internal
10+
//! and is not encoded on the wire.
1011
1112
use crate::protocol::{FlashError, FlashGeometry};
1213

@@ -18,8 +19,11 @@ pub enum BackendError {
1819
BufferTooSmall,
1920
Busy,
2021
Timeout,
21-
/// Backend cannot complete synchronously at this time; the server
22-
/// runtime should retry after `OPERATION_COMPLETE` fires.
22+
/// Operation cannot complete synchronously now.
23+
///
24+
/// This is an internal backend/server scheduling signal. The server
25+
/// runtime should defer and retry after a progress signal, typically
26+
/// a completion interrupt, rather than exposing it directly on the wire.
2327
WouldBlock,
2428
/// Media-level failure (program/erase verify fail, ECC uncorrectable, …).
2529
IoError,
@@ -37,7 +41,7 @@ impl From<BackendError> for FlashError {
3741
BackendError::BufferTooSmall => FlashError::BufferTooSmall,
3842
BackendError::Busy => FlashError::Busy,
3943
BackendError::Timeout => FlashError::Timeout,
40-
BackendError::WouldBlock => FlashError::WouldBlock,
44+
BackendError::WouldBlock => FlashError::Busy,
4145
BackendError::IoError => FlashError::IoError,
4246
BackendError::NotPermitted => FlashError::NotPermitted,
4347
BackendError::InternalError => FlashError::InternalError,

drivers/flash/api/src/protocol.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,11 @@ pub enum FlashError {
6666
BufferTooSmall = 0x04,
6767
Busy = 0x05,
6868
Timeout = 0x06,
69-
/// Operation cannot complete right now; the server/runtime may defer
70-
/// completion until the backend signals progress via interrupt.
71-
WouldBlock = 0x07,
7269
/// Underlying media reported an I/O error (e.g. flash program failure).
73-
IoError = 0x08,
70+
IoError = 0x07,
7471
/// Address/length straddles a region the backend refuses to touch
7572
/// (e.g. write-protected partition).
76-
NotPermitted = 0x09,
73+
NotPermitted = 0x08,
7774
InternalError = 0xFF,
7875
}
7976

@@ -87,9 +84,8 @@ impl From<u8> for FlashError {
8784
0x04 => Self::BufferTooSmall,
8885
0x05 => Self::Busy,
8986
0x06 => Self::Timeout,
90-
0x07 => Self::WouldBlock,
91-
0x08 => Self::IoError,
92-
0x09 => Self::NotPermitted,
87+
0x07 => Self::IoError,
88+
0x08 => Self::NotPermitted,
9389
_ => Self::InternalError,
9490
}
9591
}
@@ -297,9 +293,8 @@ mod tests {
297293
(0x04, FlashError::BufferTooSmall),
298294
(0x05, FlashError::Busy),
299295
(0x06, FlashError::Timeout),
300-
(0x07, FlashError::WouldBlock),
301-
(0x08, FlashError::IoError),
302-
(0x09, FlashError::NotPermitted),
296+
(0x07, FlashError::IoError),
297+
(0x08, FlashError::NotPermitted),
303298
(0xFF, FlashError::InternalError),
304299
];
305300
for (byte, err) in cases {
@@ -313,6 +308,7 @@ mod tests {
313308
for byte in [0x0Au8, 0x10, 0x42, 0x80, 0xFE] {
314309
assert_eq!(FlashError::from(byte), FlashError::InternalError);
315310
}
311+
assert_eq!(FlashError::from(0x09), FlashError::InternalError);
316312
}
317313

318314
#[test]
@@ -416,7 +412,6 @@ mod tests {
416412
for err in [
417413
FlashError::Success,
418414
FlashError::InvalidAddress,
419-
FlashError::WouldBlock,
420415
FlashError::NotPermitted,
421416
FlashError::InternalError,
422417
] {

drivers/flash/client/README.md

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,11 @@ pub enum ClientError {
144144
| `BufferTooSmall` | Server-side buffer constraint |
145145
| `Busy` | Backend busy |
146146
| `Timeout` | Operation timed out |
147-
| `WouldBlock` | Operation could not be completed immediately |
148147
| `IoError` | Media-level failure |
149148
| `NotPermitted` | Blocked by backend policy/protection |
150149
| `InternalError` | Unclassified server fault |
151150

152-
### `Busy` vs `WouldBlock`
153-
154-
- **`Busy`**: The backend is actively performing another operation and cannot accept new requests. Retry the entire call after waiting.
155-
- **`WouldBlock`**: This specific operation could not be completed immediately due to resource constraints or contention, but is not blocked by global device activity. Callers can implement smarter retry logic or adjust request parameters.
151+
Client policy should treat `Busy` as the retryable contention signal.
156152

157153
## Constraints
158154

drivers/flash/client/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use flash_api::{
1010
};
1111
pub use flash_api::MAX_PAYLOAD_SIZE;
1212
use userspace::syscall;
13-
use userspace::time::{Duration as KDuration, Instant, SystemClock};
13+
use userspace::time::{Clock, Duration as KDuration, Instant, SystemClock};
1414
use zerocopy::FromBytes;
1515

1616
/// Minimum buffer size required for Flash protocol messages.

0 commit comments

Comments
 (0)