Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

basic osfp support, combine the powers of reads/parse #305

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rcgoodfellow
Copy link

@rcgoodfellow rcgoodfellow commented Dec 13, 2024

This PR adds

  • Some basic support for OSFP transceivers, these are always CMIS format, I believe, as the OSFP MSA states that the management interface is CMIS in the opening paragraph.
  • A from_buf convenience method to ParseFromModule to simplify client code a bit.
  • "No separable connector" type from SFF-8024
  • OSFP-XD identifier type

Also makes a correction to Identifier value 0x19 QSFP 8X -> OSFP 8X from SFF-8024 table 4.2.

Example use can be found here. I've tested this on an Nvidia MCP7Y00-N001 DAC and it seems to be working well.

@rcgoodfellow rcgoodfellow marked this pull request as ready for review December 13, 2024 04:17
Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

Thanks for putting this up! I've got a few nits and one bigger question, let me know if something isn't clear.

decode/src/datapath.rs Outdated Show resolved Hide resolved
decode/src/datapath.rs Show resolved Hide resolved
decode/src/ident.rs Outdated Show resolved Hide resolved
@@ -57,4 +57,14 @@ pub trait ParseFromModule: Sized {

/// Parse the result of the above reads into `Self`.
fn parse<'a>(id: Identifier, reads: impl Iterator<Item = &'a [u8]>) -> Result<Self, Error>;

/// Use `reads` and `parse` together to collect data from a buffer.
fn from_buf(id: Identifier, data: &[u8]) -> Result<Self, Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How is this being or going to be used? It looks like data needs to already have the entire memory map of the module in order for the offsets in the reads to correctly index it. Also, this doesn't take into account the page information in the reads, so I'm not sure how this would be used to parse that spans more than one page.

Copy link
Author

Choose a reason for hiding this comment

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

I'm currently using this to parse EEPROM data made available by a Linux kernel driver as a file in sysfs.

https://github.com/oxidecomputer/x2/blob/860a02811c57fcff8fa5e281c8baf788ae944838/x2-evb/src/lib.rs#L43-L53

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know the details of the Linux implementation there, but I don't understand how that can work. The datapath information is spread across many different pages, for example. How does the driver expose the idea of a page?

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