add support for PSU firmware update #1934
Conversation
| // | ||
| // The boot loader command -- sent via BOOT_LOADER_CMD -- is unfortunately odd | ||
| // in that its command code is overloaded with BOOT_LOADER_STATUS. (That is, | ||
| // a read to the command code is BOOT_LOADER_STATUS, a write is | ||
| // BOOT_LOADER_CMD.) This is behavior that the PMBus crate didn't necessarily | ||
| // envision, so it can't necessarily help us out; we define the single-byte | ||
| // payload codes here rather than declaratively in the PMBus crate. | ||
| // |
There was a problem hiding this comment.
Nit: triple /// to actually generate a docstring, which is highlighted differently for those of us using syntax highlighting
(Also, leading and trailing empty lines are unidiomatic, but I know it will take some prying to remove them from your hands)
There was a problem hiding this comment.
It's definitely not intended to be a doc comment, just an implementation comment -- which hopefully gives me a stay of execution on the added vertical spacing for readability. ;)
|
|
||
| /// A block has been written; the next offset is at [`offset`], and the | ||
| /// running checksum is in [`checksum`] | ||
| WroteBlock { offset: usize, checksum: u64 }, |
There was a problem hiding this comment.
Nit: it looks like we only use a u16 for the checksum, so we could store a u16 and use wrapping_add (instead of unchecked arithmetic and trusting that we'll never overflow a u64).
There was a problem hiding this comment.
I don't want to chuck the higher bits of the checksum even though the part only wants the low 16 -- but agreed on the unchecked arithmetic; what about using a wrapping_add on the checksum?
| BackingOff(u8), | ||
| UpdateFailed, | ||
| UpdateFailedState(Option<UpdateState>), | ||
| UpdateFailure(Mwocp68Error), |
There was a problem hiding this comment.
Perhaps we ought to put a #[count(skip)] here to count the error variants?
There was a problem hiding this comment.
Slightly confused; do you mean to add a #[count(children)] to the members?
No description provided.