Refactor CMIS memory map into pages#667
Conversation
Signed-off-by: Abhi Singh <abhi@nexthop.ai>
…on easier Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
a9cfe9a to
1c4c917
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
page model to handle bank calculation correctly. Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
@aditya-nexthop may I suggest to have following dir style?
../public/cmis/pages/
page00.py
page01.py
page02.py
...
...
...
page2f.py
page9f.py
../public/cmis/cmis.py
The page file anywas has comment to indicate what this memmap corresponds to.
There was a problem hiding this comment.
I realized that this changes the location of cmis.py that holds theCmisFlatMemMap and CmisMemMap from sonic_platform_base/sonic_xcvr/mem_maps/public/cmis.py to sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/cmis.py.
We now have a divergence where, cmis.py is inside the cmis directory while cdb.py and c_cmis.py is still in sonic_platform_base/sonic_xcvr/mem_maps/public/. I recommend all of them are at the same depth.
Where should we put them? Inside cmis or outside?
prgeor
left a comment
There was a problem hiding this comment.
@aditya-nexthop have we considered if any impact to link up time (fully loaded system), dom polling interval, link initialization when single optics insertion. A comparison will be good.
Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
…e class Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Cherry pick commits from nexthop-ai#1 to public fork since it wasn't possible to move the original PR to
sonic-netThis PR implements part 1 of 2 of the HLD described here.
It introduces the CmisPage class and refactors CMIS Mem Map into pages
Description
Refactor the CMIS memory map from a single monolithic class into per-page
classes that each own their own field definitions.
sonic_platform_base/sonic_xcvr/mem_maps/public/cmis_pages/package with one module per CMIS page:
pg_00_administrative_lower.py/pg_00_administrative_upper.pypg_01_advertising.pypg_02_thresholds.pypg_10_lane_datapath_config.pypg_11_lane_datapath_status.pypg_12_tunable_laser_ctrl_status.pypg_13_module_perf_diag_ctrl.pypg_2f_vdm_advertising_ctrl.pypg_9f_cdb_message.pyCmisPagebase class (cmis_pages/base.py) that:pageandbanknumbers.getaddr(offset, page_size)so each page computes linearEEPROM offsets locally instead of relying on
CmisFlatMemMap.getaddr.register_fields(memmap), which composes the page's fieldsonto the parent memory map and merges shared
RegGroupFields (e.g.ADMIN_INFO_FIELD,ADVERTISING_FIELD) that span multiple pages,re-sorting members by offset to preserve the
RegGroupFieldinvariant.cmis_pages/cmis_page_consts.pywith CMIS layout constants(
CMIS_EEPROM_PAGE_SIZE,CMIS_ARCH_PAGES,CMIS_NUM_NON_BANKED_PAGES, and page-number constants).cmis.py:CmisFlatMemMapnow exposes anadd_pages(*pages)helper that appendsto
self.pagesand callsregister_fieldson each page.CmisFlatMemMapcomposes only the lower and upper halves of page 0x00.CmisMemMapextends it by adding pages 0x01, 0x02, 0x10–0x13, 0x2F,and 0x9F.
getaddrnow takespageas an explicit parameter and clampsbankto 0 for non-banked pages (00h–0Fh) and CDB pages (9Fh–AFh).
cmis_pagespackage insetup.py.Net diff:
cmis.pyshrinks from ~720 lines to ~120 lines, with the fielddefinitions redistributed across the per-page modules.
Motivation and Context
The previous
CmisFlatMemMap/CmisMemMapimplementation defined everyfield for every CMIS page inside a single ~700-line class. This made it
hard to:
without duplicating large blocks of field definitions.
revisions) without editing the monolithic class.
Splitting the map into per-page classes makes each page self-contained
and addressable, lets
CmisFlatMemMapandCmisMemMapdeclare theircontents by listing the pages they include, and allows shared field
groups that cross page boundaries (like
ADMIN_INFO_FIELDspanning lowerand upper page 00h, or
ADVERTISING_FIELDspanning pages 01h and 11h)to be merged automatically by
register_fields.Exposing the page number as a constructor parameter on each page class
also enables entire pages to be remapped easily without duplicating field
declarations.
How Has This Been Tested?
This change introduces no new fields or changes in behavior.
All the existing unit tests pass.
Additional Information (Optional)
This change is intended to be behavior-preserving: the resulting
CmisMemMapexposes the same set of named fields at the same EEPROMoffsets as before the refactor. Only the internal organization of the
memory map definition changes.