-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CPO Community adaption #2152
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
base: master
Are you sure you want to change the base?
CPO Community adaption #2152
Conversation
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
| OE: Optical Engine | ||
|
|
||
| PLS : Pluggable Laser Sources | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's align on industry-standard term - ELS
| CPO primarily adds new OE and PLS devices. At the software level, control of the OE/PLS devices is mainly achieved by accessing their EEPROMs. The host has two main ways to access the EEPROMs of the OE/PLS devices | ||
|
|
||
| Jonit Mode: The host does not directly access the EEPROMs of the OE/PLS devices. Instead, a CMIS controller merges the EEPROMs of the OE and PLS into a single unit, allowing the host board to access them indirectly | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Joint" - typo. Please mention that MCU can be either HW instance of instance implemented in FW (vMCU).
Best to mention that the figure is for illustration only since a number of ELSs and OEs per CPO module can vary for different vendors
| The main revisions are as follows: | ||
|
|
||
| 1. Add cpo_eeprom_mode to determine the current device's EEPROM mounting mode. separate mode: OE and PLS access EEPROM through independent CMIS interfaces.joint mode: The EEPROMs of OE and PLS are merged into a unified CMIS interface. | ||
| 2. Add a new `oes` configuration file to obtain information such as the OE bank count and OE I2C bus.According to the CMIS protocol, the Optoe driver accesses the device at address 0x50 for all cases; only the I2C bus number differs depending on the OE/Pls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you meant the configuration section in platform.json rather that configuration file ?
| "cpo_eeprom_mode":"joint" | ||
| "oes":{ | ||
| "oe0": { | ||
| "oe_bank_count": 8, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it for OE supporting 64 lanes (8 banks x 8 lanes) ?
Don't we need also mapping of lanes of a specific CPO module (e.g. all 32 or 64) to triple of OE # / ELS # / laser # ?
| "oes":{ | ||
| "oe0": { | ||
| "oe_bank_count": 8, | ||
| "oe_i2c_path": "/sys/bus/i2c/devices/i2c-24/24-0050/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed for a joint mode ? Specified today for a regular CMIS pluggable module ?
|
|
||
| #### 6.2.6. XcvrApiFactory | ||
|
|
||
| The CpoXcvrApiFactory class inherits from the original XcvrApiFactory class to initialize CPO OE/PLS EEPROM and the CpoCmisApi. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to create a new API Factory or the existing one can create a new CpoCmisApi object when detecting the module with ID equal to 0x80 (vendor-specific for CPO module unless it is stnardatized) ?
|
|
||
| #### 6.2.7. CmisManagerTask | ||
|
|
||
| Both the ElsfpCmisApi used in Joint Mode and the CpoCmisApi used in Separate Mode are encapsulated according to the functionality of the previous CmisApi, so CmisManagerTask does not need to be modified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least it should handle a new Form-Factor ID (0x80) in addition to OSFP, QSFP-DD ...
Can we create CpoCmisManagerTask inheriting most of CmisManagerTask and just overloading some methods or it will be copy/paste of the whole CMIS DPSM code (where bank support of lane portion anyway should be added) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is the claim that the PLS/ELSFP is fully automatic (or identical in management to a pluggable transceiver) when in separate mode? Do we not foresee the need to read/write any control registers specified in the OIF-ELSFP-CMIS-01.0. spec?
|
|
||
| 1) The algorithm in the optoe_translate_offset function may contain issues and needs to be fixed. This has already been discussed in the PR. This issue was resolved by applying special handling to getaddr within the CmisMemMap class. | ||
|
|
||
| It is recommended to use the driver that supports bank switching in optoe. If there are unavoidable special requirements, also may choose not to load the optoe driver and instead load the standard at24 driver. In that case, the upper-layer software would need to handle bank and page switching on its own, as well as manage conflict avoidance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vendor-specific decision which driver to use
|
|
||
| In the custom `Chassis(ChassisBase)` class get_transceiver_change_event interface, call the get_presence interface whose inheritance relationship is `CpoOptoeBase(OptoeBase)`. | ||
|
|
||
| This part of the logic is consistent with that of ordinary optical modules. In the get_presence interface of `CpoOptoeBase`, it is necessary to query the corresponding PLS information according to the port. Then, obtain its presence information according to pls_id. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Platform/vendor-specific logic how presence is detected (FPGA, GPIO, any other mechanism)
| lpmode setting flow in joint mode: | ||
|
|
||
|  | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
part of CMIS DPSM code, right ?
|
|
||
| ## 2. Scope | ||
|
|
||
| This section describes the implementation of cpo in community frame. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"CPO" instead of 'cpo' please as it is an acronym
|
|
||
|  | ||
|
|
||
| This design takes Separate Mode as an example to introduce the EEPROM specifications for standard OE and PLS. It integrates the control of the relevant OE and PLS with the traditional optical module control flow of the Sonic community, and explains how to implement the functions of traditional optical modules by controlling the OE and PLS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sonic = SONiC please
| 2. CPO devices use multiple banks in the CMIS memory map. However, there is currently no multi-bank processing logic in the community code. | ||
| Therefore, it is necessary to add a multi-bank management process. | ||
| 3. There is a many-to-one mapping relationship between CPO device ports and OE or PLS. Manufacturers need to maintain the mapping entries between ports and OE/PLS when adapting devices. | ||
| 4. Community xcvrd management is triggered based on module plug-in events. CPO has no plug-in events, so the xcvrd triggering method needs to be redesigned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I think we can use the the ELS plug-in, plug-out.
Please can you explain what we would need to redesign here?
| | platform.json | platform.json | Modified | Vendors uniformly configure relevant information via the platform.json configuration file.<br />This information includes the EEPROM access path and bank count for OE and PLS, among others.<br />It also covers the mapping information of ports corresponding to OE and PLS. | | ||
| | SfpOptoeBase | CpoOptoeBase | New | It is an abstract port management class, used for port management by the xcvrd framework.<br /> It mainly provides EEPROM access interfaces for OE and PLS.<br />Vendors are required to instantiate it and use it with revisions to the platform.json configuration file. | | ||
| | ChassisBase | Chassis | Modified | It is an original vendor-implemented class, used during initialization to determine<br />whether a port uses the CpoOptoeBase type or the original OptoeBase type. | | ||
| | CmisMemMap | ElsfpCmisMemMap | New | Provides the EEPROM memory map address for PLS, based on the OIF-ELSFP-CMIS-01.0 standard,<br />and offers multi-bank functionality. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<br /> mid sentence
| | :-------------- | ----------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | platform.json | platform.json | Modified | Vendors uniformly configure relevant information via the platform.json configuration file.<br />This information includes the EEPROM access path and bank count for OE and PLS, among others.<br />It also covers the mapping information of ports corresponding to OE and PLS. | | ||
| | SfpOptoeBase | CpoOptoeBase | New | It is an abstract port management class, used for port management by the xcvrd framework.<br /> It mainly provides EEPROM access interfaces for OE and PLS.<br />Vendors are required to instantiate it and use it with revisions to the platform.json configuration file. | | ||
| | ChassisBase | Chassis | Modified | It is an original vendor-implemented class, used during initialization to determine<br />whether a port uses the CpoOptoeBase type or the original OptoeBase type. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<br /> mid sentence
| oe_bank_count = oe_config.get("oe_bank_count", None) | ||
| if oe_bank_count == None: | ||
| return | ||
| # writh oe_bank_count into bank_count_bus_path file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit typo writh to write. Please also search for more occurrences in the file
| class ElsfpCmisMemMap(CmisMemMap): # new | ||
| def __init__(self, codes): | ||
| super(CmisMemMap, self).__init__(codes) | ||
| self._bank = bank |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please help explain this choice. Where does bank come from? Perhaps you're missing a bank param in __init__?
Are we saying that accesses are restricted to a specific bank for a given instance of the ElsfpCmisMemMap?
| class OeCmisMemMap(CmisMemMap): # new | ||
| def __init__(self, codes, bank): | ||
| super().__init__(codes) | ||
| self._bank = bank |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here.
Please help explain this choice.
Are we saying that accesses are restricted to a specific bank for a given instance of the OeCmisMemMap?
| def get_elsfp_bias_alarm_flag(self): | ||
| return self._elsfp_eeprom.read(consts.ELSFP_TX_BIAS_ALARM_FLAGS) | ||
| # The CmisApi already has functions, while the esfp needs to modify the interface name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit esfp = elsfp ?
|
|
||
| #### 6.2.7. CmisManagerTask | ||
|
|
||
| Both the ElsfpCmisApi used in Joint Mode and the CpoCmisApi used in Separate Mode are encapsulated according to the functionality of the previous CmisApi, so CmisManagerTask does not need to be modified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is the claim that the PLS/ELSFP is fully automatic (or identical in management to a pluggable transceiver) when in separate mode? Do we not foresee the need to read/write any control registers specified in the OIF-ELSFP-CMIS-01.0. spec?
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
Describe how the community framework adapts to CPO devices.