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

[sonic_xcvr]Enhance to support QSFP port_id=0xC for access eeprom #334

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jostar-yang
Copy link

Signed-off-by: jostar-yang [email protected]

Description

This PR is for fix issue:#330
Cmd, sfputil show eeprom, will get fail when insert QSFP(port_id=0xC).
root@sonic:/home/admin# sfputil show eeprom
Traceback (most recent call last):
File "/usr/local/bin/sfputil", line 8, in
sys.exit(cli())
File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 764, in call
return self.main(*args, **kwargs)
File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 717, in main
rv = self.invoke(ctx)
File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 956, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 555, in invoke
return callback(*args, **kwargs)
File "/usr/local/lib/python3.9/dist-packages/sfputil/main.py", line 673, in eeprom
output += convert_sfp_info_to_output_string(xcvr_info)
File "/usr/local/lib/python3.9/dist-packages/sfputil/main.py", line 336, in convert_sfp_info_to_output_string
sfp_type = sfp_info_dict['type']

Motivation and Context

Check code and found below code in ,

    elif id == 0x0D
        codes = Sff8436Codes
        mem_map = Sff8436MemMap(codes)
        xcvr_eeprom = XcvrEeprom(self.reader, self.writer, mem_map)
        api = Sff8436Api(xcvr_eeprom)

QSFP port id=0x0C should follow SPEC8436.
So modify to below ,

    elif id == 0x0D or id == 0x0C:
        codes = Sff8436Codes

How Has This Been Tested?

After patch the code, sfputul show eeprom will fine as below,
Ethernet0: SFP EEPROM detected
Application Advertisement: N/A
Connector: No separable connector
Encoding: Unspecified
Extended Identifier: Power Class 1 Module (1.5W max. Power consumption), No CLEI code present in Page 02h, No CDR in TX, No CDR in RX
Extended RateSelect Compliance: QSFP+ Rate Select Version 1
Identifier: QSFP
Length Cable Assembly(m): 1.0
Nominal Bit Rate(100Mbs): 103
Specification compliance:

Additional Information (Optional)

@prgeor
Copy link
Collaborator

prgeor commented Dec 8, 2022

@jostar-yang please fix the code coverage PR check failure

@prgeor
Copy link
Collaborator

prgeor commented Dec 8, 2022

@jostar-yang AFAIK, id=0x0C should follow Spec INF8438 which is currently not added in SONiC. Why do you think SFF-8436 can manage the module compliant with INF8438?

@jostar-yang
Copy link
Author

I check sonic_xcvr code and there is no sff-8438 lib. Will sonic_xcvr support sff-8438 in the future?

@prgeor prgeor requested a review from mihirpat1 January 24, 2024 18:07
@prgeor
Copy link
Collaborator

prgeor commented Jan 24, 2024

I check sonic_xcvr code and there is no sff-8438 lib. Will sonic_xcvr support sff-8438 in the future?

@jostar-yang please feel free to add support

tigerfu000 pushed a commit to tigerfu000/ec-sonic-buildimage that referenced this pull request May 29, 2024
Why I did it:
    Update submodule commit-id due to sync PRs:
    - sonic-net/sonic-platform-common#327
    - sonic-net/sonic-platform-common#334

How I dit it:
    Update sonic-platform-common to latest commit-id due to sync PRs.

How to verify it:
    Git-clone to check if download the correct codes.
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