Skip to content

Add sequencer timeout ereports#2488

Open
mkeeter wants to merge 5 commits into
masterfrom
mkeeter/seq-ereports
Open

Add sequencer timeout ereports#2488
mkeeter wants to merge 5 commits into
masterfrom
mkeeter/seq-ereports

Conversation

@mkeeter
Copy link
Copy Markdown
Collaborator

@mkeeter mkeeter commented Apr 23, 2026

This PR adds two ereport flavors for Cosmo sequencer issues:

  • hw.seq.regs is a raw dump of the registers, along with a reason why it was produced (MAPO, timeout, or "suddenly we're not in A0")
  • hw.seq.timeout.* is a family of ereports representing timeouts at each stage of sequencing, along with a diagnosis. It includes an Option<u64> that refers to a hw.seq.regs ENA, so that we can also look at raw register values (if the diagnosis is not helpful).

For failure modes other than timeout, this PR doesn't send any additional ereports (just the hw.seq.regs value).

Consider this a starting point for brainstorming how we want to organize this data (e.g. one alternative is sending the high-level ereport first, then have the hw.seq.regs refer to it).

@mkeeter mkeeter requested review from hawkw and rmustacc April 23, 2026 14:43
@mkeeter mkeeter force-pushed the mkeeter/seq-ereports branch from 0b1af44 to 6b3f76f Compare April 23, 2026 19:26
@mkeeter mkeeter force-pushed the mkeeter/seq-ereports branch from 6b3f76f to cdb5340 Compare April 27, 2026 14:15
@mkeeter mkeeter marked this pull request as ready for review May 26, 2026 20:16
Comment on lines +109 to +110
seq_api_status: u32,
seq_raw_status: u32,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

take it or leave it: i think we could scrape off a couple bytes of ereport if this was:

seq_status: SeqStatusRegs

// ...

struct SeqStatusRegs {
   api: u32,
   raw: u32,
}

since it trades a couple bytes of nested K/V pairs for 11 bytes of repeating seq_ and _status for both fields?

seq_raw_status: u32,
early_power_rdbks: u32,
ifr: u32,
debug_enables: u32,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit, take it or leave it: could save 2 bytes if this was

Suggested change
debug_enables: u32,
dbg_enables: u32,

Comment on lines +115 to +117
rail_enables: u32,
rail_pgs: u32,
rail_pgs_max_hold: u32,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could save a few bytes if this was like

    rails: RailRegs,
    // ...
}
    
// ...

struct RailRegs {
    enables: u32,
    pgs: u32,
    pgs_max_hold: u32,
}

sp5_readbacks: u32,
status: u32,

reason: DiagnoseReason,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

part of me wants to say that these should be part of the ereport's class, but...i get why they're not. hm. 🤔

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.

2 participants