Skip to content

Commit 5ffb76c

Browse files
jianyuewujianyuewu
authored
[cmis] combine cmis eeprom i2c read (sonic-net#563)
* Refactor cmis eeprom i2c read Change multiple eeprom i2c reads into a single i2c read across multiple addresses. 1. Update functions get_tx_power_flag, get_tx_bias_flag and get_rx_power_flag. 2. Add dedicated groups: TX_POWER_ALARM_FLAGS_FIELD, TX_BIAS_ALARM_FLAGS_FIELD, RX_POWER_ALARM_FLAGS_FIELD. 3. Move the MEDIA_TYPE_FIELD read and prefix lookup out of the loop. 4. Performance test result: Before: Running performance tests (3000 iterations each)... get_tx_power_flag: total 32.120s, avg 10.707ms per call get_tx_bias_flag: total 26.635s, avg 8.878ms per call get_rx_power_flag: total 32.306s, avg 10.769ms per call After: Running performance tests (3000 iterations each)... get_tx_power_flag: total 8.213s, avg 2.738ms per call get_tx_bias_flag: total 7.831s, avg 2.610ms per call get_rx_power_flag: total 9.632s, avg 3.211ms per call Signed-off-by: Jianyue Wu <[email protected]> * Refactor CMIS alarm flag parsing into generic helper Add get_alarm_flags helper consolidating TX_POWER, RX_POWER, TX_BIAS flag logic. Simplify get_tx_power_flag, get_rx_power_flag, get_tx_bias_flag to call helper. Signed-off-by: Jianyue Wu <[email protected]> --------- Signed-off-by: Jianyue Wu <[email protected]>
1 parent ca7c79b commit 5ffb76c

File tree

4 files changed

+142
-102
lines changed

4 files changed

+142
-102
lines changed

‎sonic_platform_base/sonic_xcvr/api/public/cmis.py‎

Lines changed: 25 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -643,77 +643,45 @@ def get_rx_cdr_lol(self):
643643
rx_cdr_lol_final.append(bool(rx_cdr_lol[key]))
644644
return rx_cdr_lol_final
645645

646+
def get_alarm_flags(self, alarm_flag):
647+
'''Generic helper to return alarm and warning flags for given type: TX_POWER, TX_BIAS, RX_POWER.'''
648+
flags = self.xcvr_eeprom.read(getattr(consts, f"{alarm_flag}_ALARM_FLAGS_FIELD"))
649+
if flags is None:
650+
return None
651+
high_alarm = flags.get(getattr(consts, f"{alarm_flag}_HIGH_ALARM_FLAG"))
652+
low_alarm = flags.get(getattr(consts, f"{alarm_flag}_LOW_ALARM_FLAG"))
653+
high_warn = flags.get(getattr(consts, f"{alarm_flag}_HIGH_WARN_FLAG"))
654+
low_warn = flags.get(getattr(consts, f"{alarm_flag}_LOW_WARN_FLAG"))
655+
if high_alarm is None or low_alarm is None or high_warn is None or low_warn is None:
656+
return None
657+
for d in (high_alarm, low_alarm, high_warn, low_warn):
658+
for key, value in d.items():
659+
d[key] = bool(value)
660+
prefix = alarm_flag.lower()
661+
return {
662+
f"{prefix}_high_alarm": high_alarm,
663+
f"{prefix}_low_alarm": low_alarm,
664+
f"{prefix}_high_warn": high_warn,
665+
f"{prefix}_low_warn": low_warn,
666+
}
667+
646668
def get_tx_power_flag(self):
647669
'''
648670
This function returns TX power out of range flag on TX media lane
649671
'''
650-
tx_power_high_alarm_dict = self.xcvr_eeprom.read(consts.TX_POWER_HIGH_ALARM_FLAG)
651-
tx_power_low_alarm_dict = self.xcvr_eeprom.read(consts.TX_POWER_LOW_ALARM_FLAG)
652-
tx_power_high_warn_dict = self.xcvr_eeprom.read(consts.TX_POWER_HIGH_WARN_FLAG)
653-
tx_power_low_warn_dict = self.xcvr_eeprom.read(consts.TX_POWER_LOW_WARN_FLAG)
654-
if tx_power_high_alarm_dict is None or tx_power_low_alarm_dict is None or tx_power_high_warn_dict is None or tx_power_low_warn_dict is None:
655-
return None
656-
for key, value in tx_power_high_alarm_dict.items():
657-
tx_power_high_alarm_dict[key] = bool(value)
658-
for key, value in tx_power_low_alarm_dict.items():
659-
tx_power_low_alarm_dict[key] = bool(value)
660-
for key, value in tx_power_high_warn_dict.items():
661-
tx_power_high_warn_dict[key] = bool(value)
662-
for key, value in tx_power_low_warn_dict.items():
663-
tx_power_low_warn_dict[key] = bool(value)
664-
tx_power_flag_dict = {'tx_power_high_alarm': tx_power_high_alarm_dict,
665-
'tx_power_low_alarm': tx_power_low_alarm_dict,
666-
'tx_power_high_warn': tx_power_high_warn_dict,
667-
'tx_power_low_warn': tx_power_low_warn_dict,}
668-
return tx_power_flag_dict
672+
return self.get_alarm_flags("TX_POWER")
669673

670674
def get_tx_bias_flag(self):
671675
'''
672676
This function returns TX bias out of range flag on TX media lane
673677
'''
674-
tx_bias_high_alarm_dict = self.xcvr_eeprom.read(consts.TX_BIAS_HIGH_ALARM_FLAG)
675-
tx_bias_low_alarm_dict = self.xcvr_eeprom.read(consts.TX_BIAS_LOW_ALARM_FLAG)
676-
tx_bias_high_warn_dict = self.xcvr_eeprom.read(consts.TX_BIAS_HIGH_WARN_FLAG)
677-
tx_bias_low_warn_dict = self.xcvr_eeprom.read(consts.TX_BIAS_LOW_WARN_FLAG)
678-
if tx_bias_high_alarm_dict is None or tx_bias_low_alarm_dict is None or tx_bias_high_warn_dict is None or tx_bias_low_warn_dict is None:
679-
return None
680-
for key, value in tx_bias_high_alarm_dict.items():
681-
tx_bias_high_alarm_dict[key] = bool(value)
682-
for key, value in tx_bias_low_alarm_dict.items():
683-
tx_bias_low_alarm_dict[key] = bool(value)
684-
for key, value in tx_bias_high_warn_dict.items():
685-
tx_bias_high_warn_dict[key] = bool(value)
686-
for key, value in tx_bias_low_warn_dict.items():
687-
tx_bias_low_warn_dict[key] = bool(value)
688-
tx_bias_flag_dict = {'tx_bias_high_alarm': tx_bias_high_alarm_dict,
689-
'tx_bias_low_alarm': tx_bias_low_alarm_dict,
690-
'tx_bias_high_warn': tx_bias_high_warn_dict,
691-
'tx_bias_low_warn': tx_bias_low_warn_dict,}
692-
return tx_bias_flag_dict
678+
return self.get_alarm_flags("TX_BIAS")
693679

694680
def get_rx_power_flag(self):
695681
'''
696682
This function returns RX power out of range flag on RX media lane
697683
'''
698-
rx_power_high_alarm_dict = self.xcvr_eeprom.read(consts.RX_POWER_HIGH_ALARM_FLAG)
699-
rx_power_low_alarm_dict = self.xcvr_eeprom.read(consts.RX_POWER_LOW_ALARM_FLAG)
700-
rx_power_high_warn_dict = self.xcvr_eeprom.read(consts.RX_POWER_HIGH_WARN_FLAG)
701-
rx_power_low_warn_dict = self.xcvr_eeprom.read(consts.RX_POWER_LOW_WARN_FLAG)
702-
if rx_power_high_alarm_dict is None or rx_power_low_alarm_dict is None or rx_power_high_warn_dict is None or rx_power_low_warn_dict is None:
703-
return None
704-
for key, value in rx_power_high_alarm_dict.items():
705-
rx_power_high_alarm_dict[key] = bool(value)
706-
for key, value in rx_power_low_alarm_dict.items():
707-
rx_power_low_alarm_dict[key] = bool(value)
708-
for key, value in rx_power_high_warn_dict.items():
709-
rx_power_high_warn_dict[key] = bool(value)
710-
for key, value in rx_power_low_warn_dict.items():
711-
rx_power_low_warn_dict[key] = bool(value)
712-
rx_power_flag_dict = {'rx_power_high_alarm': rx_power_high_alarm_dict,
713-
'rx_power_low_alarm': rx_power_low_alarm_dict,
714-
'rx_power_high_warn': rx_power_high_warn_dict,
715-
'rx_power_low_warn': rx_power_low_warn_dict,}
716-
return rx_power_flag_dict
684+
return self.get_alarm_flags("RX_POWER")
717685

718686
def get_tx_output_status(self):
719687
'''

‎sonic_platform_base/sonic_xcvr/fields/consts.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,9 @@
293293
LANE_MON_ADVT_FIELD = "Supported Lane Monitor Advertisement"
294294
LANE_DATAPATH_CTRL_FIELD = "Lane Control and Data Path Control"
295295
LANE_DATAPATH_STATUS_FIELD = "Lane Status and Data Path Status"
296+
TX_POWER_ALARM_FLAGS_FIELD = "TxPowerAlarmFlags"
297+
TX_BIAS_ALARM_FLAGS_FIELD = "TxBiasAlarmFlags"
298+
RX_POWER_ALARM_FLAGS_FIELD = "RxPowerAlarmFlags"
296299
DATAPATH_DEINIT_FIELD = "Data Path Deinit"
297300
LEN_MULT_FIELD = "LengthMultiplier"
298301
MAX_POWER_FIELD = "MaxPower"

‎sonic_platform_base/sonic_xcvr/mem_maps/public/cmis.py‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,78 @@ def __init__(self, codes):
293293
)
294294
)
295295

296+
# Group for TX power alarm and warning flags
297+
self.TX_POWER_ALARM_FLAGS = RegGroupField(consts.TX_POWER_ALARM_FLAGS_FIELD,
298+
RegGroupField(consts.TX_POWER_HIGH_ALARM_FLAG,
299+
*(NumberRegField("%s%d" % (consts.TX_POWER_HIGH_ALARM_FLAG, lane), self.getaddr(0x11, 139),
300+
RegBitField("Bit%d" % (lane-1), (lane-1))
301+
) for lane in range(1, 9))
302+
),
303+
RegGroupField(consts.TX_POWER_LOW_ALARM_FLAG,
304+
*(NumberRegField("%s%d" % (consts.TX_POWER_LOW_ALARM_FLAG, lane), self.getaddr(0x11, 140),
305+
RegBitField("Bit%d" % (lane-1), (lane-1))
306+
) for lane in range(1, 9))
307+
),
308+
RegGroupField(consts.TX_POWER_HIGH_WARN_FLAG,
309+
*(NumberRegField("%s%d" % (consts.TX_POWER_HIGH_WARN_FLAG, lane), self.getaddr(0x11, 141),
310+
RegBitField("Bit%d" % (lane-1), (lane-1))
311+
) for lane in range(1, 9))
312+
),
313+
RegGroupField(consts.TX_POWER_LOW_WARN_FLAG,
314+
*(NumberRegField("%s%d" % (consts.TX_POWER_LOW_WARN_FLAG, lane), self.getaddr(0x11, 142),
315+
RegBitField("Bit%d" % (lane-1), (lane-1))
316+
) for lane in range(1, 9))
317+
)
318+
)
319+
320+
# Group for TX bias alarm and warning flags
321+
self.TX_BIAS_ALARM_FLAGS = RegGroupField(consts.TX_BIAS_ALARM_FLAGS_FIELD,
322+
RegGroupField(consts.TX_BIAS_HIGH_ALARM_FLAG,
323+
*(NumberRegField("%s%d" % (consts.TX_BIAS_HIGH_ALARM_FLAG, lane), self.getaddr(0x11, 143),
324+
RegBitField("Bit%d" % (lane-1), (lane-1))
325+
) for lane in range(1, 9))
326+
),
327+
RegGroupField(consts.TX_BIAS_LOW_ALARM_FLAG,
328+
*(NumberRegField("%s%d" % (consts.TX_BIAS_LOW_ALARM_FLAG, lane), self.getaddr(0x11, 144),
329+
RegBitField("Bit%d" % (lane-1), (lane-1))
330+
) for lane in range(1, 9))
331+
),
332+
RegGroupField(consts.TX_BIAS_HIGH_WARN_FLAG,
333+
*(NumberRegField("%s%d" % (consts.TX_BIAS_HIGH_WARN_FLAG, lane), self.getaddr(0x11, 145),
334+
RegBitField("Bit%d" % (lane-1), (lane-1))
335+
) for lane in range(1, 9))
336+
),
337+
RegGroupField(consts.TX_BIAS_LOW_WARN_FLAG,
338+
*(NumberRegField("%s%d" % (consts.TX_BIAS_LOW_WARN_FLAG, lane), self.getaddr(0x11, 146),
339+
RegBitField("Bit%d" % (lane-1), (lane-1))
340+
) for lane in range(1, 9))
341+
)
342+
)
343+
344+
# Group for RX power alarm and warning flags
345+
self.RX_POWER_ALARM_FLAGS = RegGroupField(consts.RX_POWER_ALARM_FLAGS_FIELD,
346+
RegGroupField(consts.RX_POWER_HIGH_ALARM_FLAG,
347+
*(NumberRegField("%s%d" % (consts.RX_POWER_HIGH_ALARM_FLAG, lane), self.getaddr(0x11, 149),
348+
RegBitField("Bit%d" % (lane-1), (lane-1))
349+
) for lane in range(1, 9))
350+
),
351+
RegGroupField(consts.RX_POWER_LOW_ALARM_FLAG,
352+
*(NumberRegField("%s%d" % (consts.RX_POWER_LOW_ALARM_FLAG, lane), self.getaddr(0x11, 150),
353+
RegBitField("Bit%d" % (lane-1), (lane-1))
354+
) for lane in range(1, 9))
355+
),
356+
RegGroupField(consts.RX_POWER_HIGH_WARN_FLAG,
357+
*(NumberRegField("%s%d" % (consts.RX_POWER_HIGH_WARN_FLAG, lane), self.getaddr(0x11, 151),
358+
RegBitField("Bit%d" % (lane-1), (lane-1))
359+
) for lane in range(1, 9))
360+
),
361+
RegGroupField(consts.RX_POWER_LOW_WARN_FLAG,
362+
*(NumberRegField("%s%d" % (consts.RX_POWER_LOW_WARN_FLAG, lane), self.getaddr(0x11, 152),
363+
RegBitField("Bit%d" % (lane-1), (lane-1))
364+
) for lane in range(1, 9))
365+
)
366+
)
367+
296368
self.LANE_DATAPATH_STATUS = RegGroupField(consts.LANE_DATAPATH_STATUS_FIELD,
297369
RegGroupField(consts.TX_FAULT_FIELD,
298370
*(NumberRegField("%s%d" % (consts.TX_FAULT_FIELD, lane), self.getaddr(0x11, 135),

‎tests/sonic_xcvr/test_cmis.py‎

Lines changed: 42 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -286,74 +286,71 @@ def test_get_rx_cdr_lol(self, mock_response, expected):
286286
assert result == expected
287287

288288
@pytest.mark.parametrize("mock_response, expected", [
289-
([{'TxPowerHighAlarmFlag1':0}, {'TxPowerLowAlarmFlag1':0}, {'TxPowerHighWarnFlag1':0}, {'TxPowerLowWarnFlag1':0}],
290-
{
291-
'tx_power_high_alarm':{
292-
'TxPowerHighAlarmFlag1': False
293-
},
294-
'tx_power_low_alarm':{
295-
'TxPowerLowAlarmFlag1': False
296-
},
297-
'tx_power_high_warn':{
298-
'TxPowerHighWarnFlag1': False,
289+
(
290+
{
291+
consts.TX_POWER_HIGH_ALARM_FLAG: {'TxPowerHighAlarmFlag1': 0},
292+
consts.TX_POWER_LOW_ALARM_FLAG: {'TxPowerLowAlarmFlag1': 0},
293+
consts.TX_POWER_HIGH_WARN_FLAG: {'TxPowerHighWarnFlag1': 0},
294+
consts.TX_POWER_LOW_WARN_FLAG: {'TxPowerLowWarnFlag1': 0}
299295
},
300-
'tx_power_low_warn':{
301-
'TxPowerLowWarnFlag1': False
296+
{
297+
'tx_power_high_alarm': {'TxPowerHighAlarmFlag1': False},
298+
'tx_power_low_alarm': {'TxPowerLowAlarmFlag1': False},
299+
'tx_power_high_warn': {'TxPowerHighWarnFlag1': False},
300+
'tx_power_low_warn': {'TxPowerLowWarnFlag1': False}
302301
}
303-
}),
304-
([None, None, None, None], None)
302+
),
303+
(None, None)
305304
])
306305
def test_get_tx_power_flag(self, mock_response, expected):
307306
self.api.xcvr_eeprom.read = MagicMock()
308-
self.api.xcvr_eeprom.read.side_effect = mock_response
307+
self.api.xcvr_eeprom.read.return_value = mock_response
309308
result = self.api.get_tx_power_flag()
310309
assert result == expected
311310

312311
@pytest.mark.parametrize("mock_response, expected", [
313-
([{'TxBiasHighAlarmFlag1':0}, {'TxBiasLowAlarmFlag1':0}, {'TxBiasHighWarnFlag1':0}, {'TxBiasLowWarnFlag1':0}],
314-
{
315-
'tx_bias_high_alarm':{
316-
'TxBiasHighAlarmFlag1': False
317-
},
318-
'tx_bias_low_alarm':{
319-
'TxBiasLowAlarmFlag1': False
320-
},
321-
'tx_bias_high_warn':{
322-
'TxBiasHighWarnFlag1': False,
312+
(
313+
{
314+
consts.TX_BIAS_HIGH_ALARM_FLAG: {'TxBiasHighAlarmFlag1': 0},
315+
consts.TX_BIAS_LOW_ALARM_FLAG: {'TxBiasLowAlarmFlag1': 0},
316+
consts.TX_BIAS_HIGH_WARN_FLAG: {'TxBiasHighWarnFlag1': 0},
317+
consts.TX_BIAS_LOW_WARN_FLAG: {'TxBiasLowWarnFlag1': 0}
323318
},
324-
'tx_bias_low_warn':{
325-
'TxBiasLowWarnFlag1': False
319+
{
320+
'tx_bias_high_alarm': {'TxBiasHighAlarmFlag1': False},
321+
'tx_bias_low_alarm': {'TxBiasLowAlarmFlag1': False},
322+
'tx_bias_high_warn': {'TxBiasHighWarnFlag1': False},
323+
'tx_bias_low_warn': {'TxBiasLowWarnFlag1': False}
326324
}
327-
}),
328-
([None, None, None, None], None)
325+
),
326+
(None, None)
329327
])
330328
def test_get_tx_bias_flag(self, mock_response, expected):
331329
self.api.xcvr_eeprom.read = MagicMock()
332-
self.api.xcvr_eeprom.read.side_effect = mock_response
330+
self.api.xcvr_eeprom.read.return_value = mock_response
333331
result = self.api.get_tx_bias_flag()
334332
assert result == expected
335333

336334
@pytest.mark.parametrize("mock_response, expected", [
337-
([{'RxPowerHighAlarmFlag1':0}, {'RxPowerLowAlarmFlag1':0}, {'RxPowerHighWarnFlag1':0}, {'RxPowerLowWarnFlag1':0}],
338-
{
339-
'rx_power_high_alarm':{
340-
'RxPowerHighAlarmFlag1': False
341-
},
342-
'rx_power_low_alarm':{
343-
'RxPowerLowAlarmFlag1': False
344-
},
345-
'rx_power_high_warn':{
346-
'RxPowerHighWarnFlag1': False,
335+
(
336+
{
337+
consts.RX_POWER_HIGH_ALARM_FLAG: {'RxPowerHighAlarmFlag1': 0},
338+
consts.RX_POWER_LOW_ALARM_FLAG: {'RxPowerLowAlarmFlag1': 0},
339+
consts.RX_POWER_HIGH_WARN_FLAG: {'RxPowerHighWarnFlag1': 0},
340+
consts.RX_POWER_LOW_WARN_FLAG: {'RxPowerLowWarnFlag1': 0}
347341
},
348-
'rx_power_low_warn':{
349-
'RxPowerLowWarnFlag1': False
342+
{
343+
'rx_power_high_alarm': {'RxPowerHighAlarmFlag1': False},
344+
'rx_power_low_alarm': {'RxPowerLowAlarmFlag1': False},
345+
'rx_power_high_warn': {'RxPowerHighWarnFlag1': False},
346+
'rx_power_low_warn': {'RxPowerLowWarnFlag1': False}
350347
}
351-
}),
352-
([None, None, None, None], None)
348+
),
349+
(None, None)
353350
])
354351
def test_get_rx_power_flag(self, mock_response, expected):
355352
self.api.xcvr_eeprom.read = MagicMock()
356-
self.api.xcvr_eeprom.read.side_effect = mock_response
353+
self.api.xcvr_eeprom.read.return_value = mock_response
357354
result = self.api.get_rx_power_flag()
358355
assert result == expected
359356

0 commit comments

Comments
 (0)