Adds a Medusa image #1806
Adds a Medusa image #1806
Conversation
|
I'm opening this branch up as a draft PR just so we know where this work is at with my leave looming. There is a bunch of copy/paste here that can get addressed properly once #1805 lands. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Overall, this looks good! You know a lot more about Medusa's design than I do, so I focused on the code rather than on what it's doing. I left some style suggestions, most of which aren't really a big deal.
I do think it would be nice to make the task-thermal-api dependency in drv-transcievers-server optional, so that the thermal API doesn't have to be compiled in when thermal control is disabled. This could potentially make Medusa builds slightly faster, which is probably nice to have. Other than that, my suggestions were just minor style nits.
| if ident_valid && checksum_valid { | ||
| ringbuf_entry!(Trace::SkipLoadingFrontIOControllerBitstream { | ||
| fpga_id: i | ||
| }); | ||
| } else { | ||
| ringbuf_entry!(Trace::LoadingFrontIOControllerBitstream { | ||
| fpga_id: i | ||
| }); | ||
|
|
||
| if let Err(e) = controller.load_bitstream(self.auxflash_task) { | ||
| ringbuf_entry!(Trace::FpgaBitstreamError(u32::from(e))); | ||
| return Err(e); | ||
| } | ||
|
|
||
| (ident, ident_valid) = controller.ident_valid()?; | ||
| ringbuf_entry!(Trace::FrontIOControllerIdent { | ||
| fpga_id: i, | ||
| ident | ||
| }); | ||
|
|
||
| controller.write_checksum()?; | ||
| (checksum, checksum_valid) = controller.checksum_valid()?; | ||
| ringbuf_entry!(Trace::FrontIOControllerChecksum { | ||
| fpga_id: i, | ||
| checksum, | ||
| expected: FrontIOController::short_checksum(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
style nit, take it or leave it: i might consider using continue here so that the rest of the code doesn't drift to the right; others may disagree with me on this though:
| if ident_valid && checksum_valid { | |
| ringbuf_entry!(Trace::SkipLoadingFrontIOControllerBitstream { | |
| fpga_id: i | |
| }); | |
| } else { | |
| ringbuf_entry!(Trace::LoadingFrontIOControllerBitstream { | |
| fpga_id: i | |
| }); | |
| if let Err(e) = controller.load_bitstream(self.auxflash_task) { | |
| ringbuf_entry!(Trace::FpgaBitstreamError(u32::from(e))); | |
| return Err(e); | |
| } | |
| (ident, ident_valid) = controller.ident_valid()?; | |
| ringbuf_entry!(Trace::FrontIOControllerIdent { | |
| fpga_id: i, | |
| ident | |
| }); | |
| controller.write_checksum()?; | |
| (checksum, checksum_valid) = controller.checksum_valid()?; | |
| ringbuf_entry!(Trace::FrontIOControllerChecksum { | |
| fpga_id: i, | |
| checksum, | |
| expected: FrontIOController::short_checksum(), | |
| }); | |
| } | |
| if ident_valid && checksum_valid { | |
| ringbuf_entry!(Trace::SkipLoadingFrontIOControllerBitstream { | |
| fpga_id: i | |
| }); | |
| continue; | |
| } | |
| ringbuf_entry!(Trace::LoadingFrontIOControllerBitstream { | |
| fpga_id: i | |
| }); | |
| if let Err(e) = controller.load_bitstream(self.auxflash_task) { | |
| ringbuf_entry!(Trace::FpgaBitstreamError(u32::from(e))); | |
| return Err(e); | |
| } | |
| (ident, ident_valid) = controller.ident_valid()?; | |
| ringbuf_entry!(Trace::FrontIOControllerIdent { | |
| fpga_id: i, | |
| ident | |
| }); | |
| controller.write_checksum()?; | |
| (checksum, checksum_valid) = controller.checksum_valid()?; | |
| ringbuf_entry!(Trace::FrontIOControllerChecksum { | |
| fpga_id: i, | |
| checksum, | |
| expected: FrontIOController::short_checksum(), | |
| }); |
There was a problem hiding this comment.
I hear you on this, but I'm going to opt to keep it the way it is right now. This file is just copy/paste from https://github.com/oxidecomputer/hubris/blob/master/drv/sidecar-mainboard-controller/src/front_io.rs to get a quick solution here. In #1805, which I've not attempted to land before I go on leave, this could get addressed properly.
| if ident_valid && checksum_valid { | ||
| ringbuf_entry!(Trace::SkipLoadingFrontIOControllerBitstream { | ||
| fpga_id: i | ||
| }); |
There was a problem hiding this comment.
it might be nice to have a comment here saying that we don't need to do anything because we are already running the expected bitstream. or, we could rename the ringbuf entry to something like FrontIOControllerBitstreamAlreadyLoaded so that it's clear to the reader why we stop here.
There was a problem hiding this comment.
As I state in the comment above, I'm going to leave this file as pure copy/paste as possible for now just to keep Sidecar/medusa as identical as possible until I can better unify the Front IO control stuff via #1805.
The primary purpose of this PR is to sketch out an initial stab at an image for Medusa, our QSFP front IO test fixture (details on that in RFD 405). The Medusa-specific logic lives in
medusa-seq-server. The current goal is just to support hardware bringup, and thus that sequencer just attempts to power on the front IO board and then program its FPGAs. It also checks the status of the various power rails on Medusa and exposes an interface to control those rails (on Sidecar this functionality is handled by the FPGA).An adjacent piece of work here is that Medusa does not have any fans to control and that broke the previous mold of "everything with
thermalhas fans".thermallogssensordata and wants to control fan speeds off that data. So the Medusa thermal BSP looks a bit odd compared to the others. Additionally, thetransceiverstask had athermal-controlfeature flag added to gate the interactions withthermal(Sidecar will have this feature set, Medusa will not).Lastly, there's some ugliness that I did for the sake of expediency given my personal situation. Until I can actually land #1805, Medusa simply copies some code from Sidecar. Once this PR lands, I will extend #1805 to cleanup Medusa as well.
Closes #1571