[xcvr]: Support 150GHz grid in get_laser_config_freq/set_laser_freq#718
[xcvr]: Support 150GHz grid in get_laser_config_freq/set_laser_freq#718gs1571 wants to merge 2 commits into
Conversation
* get_freq_grid() already maps GridSpacing code 8 to 150GHz, but get_laser_config_freq() and set_laser_freq() were never updated to match: get_laser_config_freq() fell through to the generic branch and computed channel * 150 instead of channel * 25, and set_laser_freq() had no grid==150 case at all and returned False for any 150GHz-only module. * get_laser_config_freq(): treat grid 150 like grid 75 (channel step is 25GHz for both; 150GHz channels are simply every 6th 25GHz step). * set_laser_freq(): add a grid==150 branch, GridSpacingTx1 code 0x80, channel number required to be a multiple of 6. Signed-off-by: Grigory Solovyev <gs1571@gmail.com>
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
| channel_number = int(round((freq - 193100)/100)) | ||
| elif grid == 150: | ||
| freq_grid = 0x80 | ||
| channel_number = int(round((freq - 193100)/25)) |
There was a problem hiding this comment.
Not sure, but both here and here:
if freq_grid in (75, 150):
config_freq = 193100 + channel * 25
we haven't accounted for the +3 in the spec:
There was a problem hiding this comment.
My bad, missed the offset entirely — should have checked the spec table more carefully before submitting. Fixed both directions: get_laser_config_freq() adds 3 to the channel before scaling, set_laser_freq() subtracts 3 before the % 6 check and the write.
One thing worth flagging: this shifts which frequencies are valid on the 150GHz grid — 193175/193325/193475/... instead of 193100/193250/193400/.... Any config targeting the old values will now correctly fail the channel_number % 6 == 0 check, since those were never real grid points; the old code just silently accepted them at the wrong frequency.
Pushed the fix with updated/added tests in test_ccmis.py, including an explicit assertion on the channel number actually written.
* OIF-CMIS 5.3 Table 8-66 defines the 150GHz grid as Frequency = 193.1 + (n+3) x 0.025 THz, unlike the 75GHz grid (Frequency = 193.1 + n x 0.025 THz) which has no additive offset. get_laser_config_freq()/set_laser_freq() treated both grids identically, so every 150GHz frequency was off by exactly +75GHz (3 channels x 25GHz) from what was requested. * get_laser_config_freq() now adds 3 to the channel before scaling for grid 150; set_laser_freq() subtracts 3 from the computed channel number before the "multiple of 6" validation and before writing it to the module. * Add/update tests in test_ccmis.py covering the corrected 150GHz formula and asserting the exact channel number written to the module. Signed-off-by: Grigory Solovyev <gs1571@gmail.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
3 participants
Description
get_freq_grid()already mapsGridSpacingcode8to150(GHz), but thetwo functions that actually use the frequency grid for laser tuning were never
updated to match:
get_laser_config_freq()fell through to the genericelsebranch andcomputed
channel * 150instead ofchannel * 25— producing a frequencyup to several hundred GHz off, and sometimes outside the module's own
supported range.
set_laser_freq()had nogrid == 150branch at all and unconditionallyreturned
Falsefor any request on a 150GHz-only module.Fixed both:
get_laser_config_freq()now treats grid 150 the same as grid 75(channel step is 25GHz for both — a 150GHz channel is simply every 6th 25GHz
step).
set_laser_freq()gained agrid == 150branch (GridSpacingTx1code0x80, channel number required to be a multiple of 6).Motivation and Context
Found bringing up an 800G ZR+ coherent module (CMIS 5.3) that only supports
the 150GHz DWDM grid. Without this fix such a module can never be tuned to a
target frequency —
set_laser_freq()always fails, and even reading back theconfigured frequency is wrong.