From c65fff66b9c9e353b4bbaa0d323423e7d181e9ba Mon Sep 17 00:00:00 2001 From: lotus-nexthop Date: Wed, 29 Jul 2026 15:04:43 -0700 Subject: [PATCH 1/3] Implement get_transceiver_dom_flags for SFF-8636 modules Signed-off-by: lotus-nexthop Signed-off-by: Lotus Fenn --- .../sonic_xcvr/api/public/sff8636.py | 37 +++++++++++++ .../sonic_xcvr/fields/consts.py | 2 + .../sonic_xcvr/mem_maps/public/sff8636.py | 10 ++++ tests/sonic_xcvr/test_sff8636.py | 53 +++++++++++++++++++ 4 files changed, 102 insertions(+) diff --git a/sonic_platform_base/sonic_xcvr/api/public/sff8636.py b/sonic_platform_base/sonic_xcvr/api/public/sff8636.py index 800367581..d13b080b4 100644 --- a/sonic_platform_base/sonic_xcvr/api/public/sff8636.py +++ b/sonic_platform_base/sonic_xcvr/api/public/sff8636.py @@ -15,6 +15,14 @@ class Sff8636Api(XcvrApi): NUM_CHANNELS = 4 POWER_CLASS_PATTERN = r'^Power Class ([1-8])' + # Bit layout shared by the temperature (byte 6) and Vcc (byte 7) + # free side monitor interrupt flag bytes (SFF-8636 Rev 2.12 + # Table 6-6); bits 3-0 are reserved/other. + FLAG_HIGH_ALARM_BITPOS = 7 + FLAG_LOW_ALARM_BITPOS = 6 + FLAG_HIGH_WARN_BITPOS = 5 + FLAG_LOW_WARN_BITPOS = 4 + def __init__(self, xcvr_eeprom): super(Sff8636Api, self).__init__(xcvr_eeprom) self._temp_support = None @@ -126,6 +134,35 @@ def get_transceiver_status_flags(self): return trans_status_flags + def get_transceiver_dom_flags(self): + """ + Retrieves the DOM flags for this xcvr + + Reads the latched free side monitor interrupt flags (SFF-8636 + Rev 2.12 Table 6-6, lower page 00h bytes 6-7). The latches clear on read, so each flag + byte is read exactly once per call and all bits are decoded from + that single value. Field names match the CMIS + get_transceiver_dom_flags keys so consumers of the + TRANSCEIVER_DOM_FLAG table see a uniform schema across module types. + + Returns: + Dictionary of boolean flags, or None on EEPROM read failure + """ + temp_flags = self.xcvr_eeprom.read(consts.TEMP_FLAGS_FIELD) + vcc_flags = self.xcvr_eeprom.read(consts.VCC_FLAGS_FIELD) + if temp_flags is None or vcc_flags is None: + return None + return { + "tempHAlarm": bool(temp_flags & (1 << self.FLAG_HIGH_ALARM_BITPOS)), + "tempLAlarm": bool(temp_flags & (1 << self.FLAG_LOW_ALARM_BITPOS)), + "tempHWarn": bool(temp_flags & (1 << self.FLAG_HIGH_WARN_BITPOS)), + "tempLWarn": bool(temp_flags & (1 << self.FLAG_LOW_WARN_BITPOS)), + "vccHAlarm": bool(vcc_flags & (1 << self.FLAG_HIGH_ALARM_BITPOS)), + "vccLAlarm": bool(vcc_flags & (1 << self.FLAG_LOW_ALARM_BITPOS)), + "vccHWarn": bool(vcc_flags & (1 << self.FLAG_HIGH_WARN_BITPOS)), + "vccLWarn": bool(vcc_flags & (1 << self.FLAG_LOW_WARN_BITPOS)), + } + def get_transceiver_dom_real_value(self): """ Retrieves DOM sensor values for this transceiver diff --git a/sonic_platform_base/sonic_xcvr/fields/consts.py b/sonic_platform_base/sonic_xcvr/fields/consts.py index c4881da34..6a6ced94b 100644 --- a/sonic_platform_base/sonic_xcvr/fields/consts.py +++ b/sonic_platform_base/sonic_xcvr/fields/consts.py @@ -44,6 +44,7 @@ TEMP_LOW_ALARM_FIELD = "TempLowAlarm" TEMP_HIGH_WARNING_FIELD = "TempHighWarning" TEMP_LOW_WARNING_FIELD = "TempLowWarning" +TEMP_FLAGS_FIELD = "TempFlags" THRESHOLDS_FIELD = "Thresholds" @@ -78,6 +79,7 @@ VOLTAGE_FIELD = "Voltage" VOLTAGE_SUPPORT_FIELD = "Supply Voltage Monitoring Implemented" VOLTAGE_THRESHOLDS_FIELD = "VoltageThresholds" +VCC_FLAGS_FIELD = "VccFlags" VOLTAGE_HIGH_ALARM_FIELD = "VoltageHighAlarm" VOLTAGE_LOW_ALARM_FIELD = "VoltageLowAlarm" VOLTAGE_HIGH_WARNING_FIELD = "VoltageHighWarning" diff --git a/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py b/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py index af5c30731..6c2680ec0 100644 --- a/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py +++ b/sonic_platform_base/sonic_xcvr/mem_maps/public/sff8636.py @@ -100,6 +100,16 @@ def __init__(self, codes): for channel, bitpos in zip(range(1, 5), range(0, 4))) ) + # Latched free side monitor interrupt flag bytes (SFF-8636 + # Rev 2.12 Table 6-6): byte 6 holds temperature alarm/warning + # flags, byte 7 holds supply voltage alarm/warning flags. The + # latches clear on read, so each byte must be read whole (like + # CMIS MODULE_FLAG_BYTE1) and decoded by the caller; RegBitField + # children would trigger one clearing read per bit. + self.TEMP_FLAGS = NumberRegField(consts.TEMP_FLAGS_FIELD, self.get_addr(0, 6), size=1) + + self.VCC_FLAGS = NumberRegField(consts.VCC_FLAGS_FIELD, self.get_addr(0, 7), size=1) + self.TX_DISABLE = NumberRegField(consts.TX_DISABLE_FIELD, self.get_addr(0, 86), *(RegBitField("Tx%dDisable" % channel, bitpos, ro=False) for channel, bitpos in zip(range(1, 5), range(0, 4))), diff --git a/tests/sonic_xcvr/test_sff8636.py b/tests/sonic_xcvr/test_sff8636.py index 67133e4d3..538261cfe 100644 --- a/tests/sonic_xcvr/test_sff8636.py +++ b/tests/sonic_xcvr/test_sff8636.py @@ -257,6 +257,59 @@ def test_get_transceiver_status_flags(self, mock_response, expected): result = self.api.get_transceiver_status_flags() assert result == expected + @pytest.mark.parametrize( + "mock_response, expected", + [ + ( + # SFF-8636 Rev 2.12 Table 6-6 bit layout, shared by both bytes: + # bit 7 = L-High Alarm + # bit 6 = L-Low Alarm + # bit 5 = L-High Warning + # bit 4 = L-Low Warning + # bits 3-0 reserved/other + [ + 0b1010_0000, # temp byte 6: high alarm + high warning + 0b0101_0000, # vcc byte 7: low alarm + low warning + ], + { + "tempHAlarm": True, + "tempLAlarm": False, + "tempHWarn": True, + "tempLWarn": False, + "vccHAlarm": False, + "vccLAlarm": True, + "vccHWarn": False, + "vccLWarn": True, + }, + ), + ( + # no flags asserted on either byte + [0b0000_0000, 0b0000_0000], + { + "tempHAlarm": False, + "tempLAlarm": False, + "tempHWarn": False, + "tempLWarn": False, + "vccHAlarm": False, + "vccLAlarm": False, + "vccHWarn": False, + "vccLWarn": False, + }, + ), + # EEPROM read failure of either byte returns None + ([None, 0b0000_0000], None), + ([0b0000_0000, None], None), + ], + ) + def test_get_transceiver_dom_flags(self, mock_response, expected): + self.api.xcvr_eeprom.read.side_effect = mock_response + self.api.xcvr_eeprom.read = MagicMock() + result = self.api.get_transceiver_dom_flags() + assert result == expected + # The flag latches clear on read: each byte must be read exactly + # once per call, in a single whole-byte access. + assert self.api.xcvr_eeprom.read.call_count == 2 + @pytest.mark.parametrize("mock_response, expected",[ ( [ From 3acb73aff31b31f0bb9948e51f8b3df2f1de2345 Mon Sep 17 00:00:00 2001 From: Lotus Fenn Date: Sat, 1 Aug 2026 19:43:54 +0000 Subject: [PATCH 2/3] fix unit test Signed-off-by: Lotus Fenn --- tests/sonic_xcvr/test_sff8636.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sonic_xcvr/test_sff8636.py b/tests/sonic_xcvr/test_sff8636.py index 538261cfe..af20f72c1 100644 --- a/tests/sonic_xcvr/test_sff8636.py +++ b/tests/sonic_xcvr/test_sff8636.py @@ -302,8 +302,8 @@ def test_get_transceiver_status_flags(self, mock_response, expected): ], ) def test_get_transceiver_dom_flags(self, mock_response, expected): - self.api.xcvr_eeprom.read.side_effect = mock_response self.api.xcvr_eeprom.read = MagicMock() + self.api.xcvr_eeprom.read.side_effect = mock_response result = self.api.get_transceiver_dom_flags() assert result == expected # The flag latches clear on read: each byte must be read exactly From 9ffaa52fca82f2db8e67044fadf64817157e8903 Mon Sep 17 00:00:00 2001 From: Lotus Fenn Date: Thu, 6 Aug 2026 01:09:04 +0000 Subject: [PATCH 3/3] Gate Vcc and temp flags behind monitor support Signed-off-by: Lotus Fenn --- .../sonic_xcvr/api/public/sff8636.py | 69 ++++++--- tests/sonic_xcvr/test_sff8636.py | 144 +++++++++++++++--- 2 files changed, 169 insertions(+), 44 deletions(-) diff --git a/sonic_platform_base/sonic_xcvr/api/public/sff8636.py b/sonic_platform_base/sonic_xcvr/api/public/sff8636.py index d13b080b4..88423098d 100644 --- a/sonic_platform_base/sonic_xcvr/api/public/sff8636.py +++ b/sonic_platform_base/sonic_xcvr/api/public/sff8636.py @@ -17,7 +17,9 @@ class Sff8636Api(XcvrApi): # Bit layout shared by the temperature (byte 6) and Vcc (byte 7) # free side monitor interrupt flag bytes (SFF-8636 Rev 2.12 - # Table 6-6); bits 3-0 are reserved/other. + # Table 6-6). In byte 6, bits 3-2 are reserved and bits 1-0 are the + # TC readiness and initialization complete flags; in byte 7, bits 3-0 + # are reserved. FLAG_HIGH_ALARM_BITPOS = 7 FLAG_LOW_ALARM_BITPOS = 6 FLAG_HIGH_WARN_BITPOS = 5 @@ -138,30 +140,53 @@ def get_transceiver_dom_flags(self): """ Retrieves the DOM flags for this xcvr - Reads the latched free side monitor interrupt flags (SFF-8636 - Rev 2.12 Table 6-6, lower page 00h bytes 6-7). The latches clear on read, so each flag - byte is read exactly once per call and all bits are decoded from - that single value. Field names match the CMIS - get_transceiver_dom_flags keys so consumers of the - TRANSCEIVER_DOM_FLAG table see a uniform schema across module types. + Reads the clear on read latched free side monitor interrupt flags. + Refer to: SFF-8636 Rev 2.12 Table 6-6, lower page 00h bytes 6-7 + + Field names match the CMIS get_transceiver_dom_flags keys, so consumers of the + TRANSCEIVER_DOM_FLAG table see the same key names across module + types. + + Every flag in Table 6-6 is optional (only L-Temp High Alarm is + required, and only for separable modules), and the spec provides no + per-flag advertisement. We gate each flag group behind its associated monitor. + + A group's keys are omitted rather than reported as False whenever its + data is not trustworthy: the monitor is not advertised, the + advertisement could not be read, or the flag byte itself could not be + read. Consumers render an absent flag as N/A, which is the honest + answer in all three cases. Note the flag bytes must be compared + against None and not tested for truth: 0x00 is the normal state of a + healthy module and its four flags are a real "no excursion" result. Returns: - Dictionary of boolean flags, or None on EEPROM read failure + Dictionary of boolean flags, containing only the groups that + yielded trustworthy data, and empty if neither did -- in which + case xcvrd posts no DOM flags for the port """ - temp_flags = self.xcvr_eeprom.read(consts.TEMP_FLAGS_FIELD) - vcc_flags = self.xcvr_eeprom.read(consts.VCC_FLAGS_FIELD) - if temp_flags is None or vcc_flags is None: - return None - return { - "tempHAlarm": bool(temp_flags & (1 << self.FLAG_HIGH_ALARM_BITPOS)), - "tempLAlarm": bool(temp_flags & (1 << self.FLAG_LOW_ALARM_BITPOS)), - "tempHWarn": bool(temp_flags & (1 << self.FLAG_HIGH_WARN_BITPOS)), - "tempLWarn": bool(temp_flags & (1 << self.FLAG_LOW_WARN_BITPOS)), - "vccHAlarm": bool(vcc_flags & (1 << self.FLAG_HIGH_ALARM_BITPOS)), - "vccLAlarm": bool(vcc_flags & (1 << self.FLAG_LOW_ALARM_BITPOS)), - "vccHWarn": bool(vcc_flags & (1 << self.FLAG_HIGH_WARN_BITPOS)), - "vccLWarn": bool(vcc_flags & (1 << self.FLAG_LOW_WARN_BITPOS)), - } + dom_flags = {} + + if self.get_temperature_support(): + temp_flags = self.xcvr_eeprom.read(consts.TEMP_FLAGS_FIELD) + if temp_flags is not None: + dom_flags.update({ + "tempHAlarm": bool(temp_flags & (1 << self.FLAG_HIGH_ALARM_BITPOS)), + "tempLAlarm": bool(temp_flags & (1 << self.FLAG_LOW_ALARM_BITPOS)), + "tempHWarn": bool(temp_flags & (1 << self.FLAG_HIGH_WARN_BITPOS)), + "tempLWarn": bool(temp_flags & (1 << self.FLAG_LOW_WARN_BITPOS)), + }) + + if self.get_voltage_support(): + vcc_flags = self.xcvr_eeprom.read(consts.VCC_FLAGS_FIELD) + if vcc_flags is not None: + dom_flags.update({ + "vccHAlarm": bool(vcc_flags & (1 << self.FLAG_HIGH_ALARM_BITPOS)), + "vccLAlarm": bool(vcc_flags & (1 << self.FLAG_LOW_ALARM_BITPOS)), + "vccHWarn": bool(vcc_flags & (1 << self.FLAG_HIGH_WARN_BITPOS)), + "vccLWarn": bool(vcc_flags & (1 << self.FLAG_LOW_WARN_BITPOS)), + }) + + return dom_flags def get_transceiver_dom_real_value(self): """ diff --git a/tests/sonic_xcvr/test_sff8636.py b/tests/sonic_xcvr/test_sff8636.py index af20f72c1..d61c2efb1 100644 --- a/tests/sonic_xcvr/test_sff8636.py +++ b/tests/sonic_xcvr/test_sff8636.py @@ -257,20 +257,23 @@ def test_get_transceiver_status_flags(self, mock_response, expected): result = self.api.get_transceiver_status_flags() assert result == expected + # SFF-8636 Rev 2.12 Table 6-6 bit layout, shared by both flag bytes: + # bit 7 = L-High Alarm + # bit 6 = L-Low Alarm + # bit 5 = L-High Warning + # bit 4 = L-Low Warning + # Byte 6 bits 3-2 are reserved and bits 1-0 are TC readiness / + # initialization complete; byte 7 bits 3-0 are reserved. @pytest.mark.parametrize( - "mock_response, expected", + "temp_support, vcc_support, eeprom, expected, expected_reads", [ ( - # SFF-8636 Rev 2.12 Table 6-6 bit layout, shared by both bytes: - # bit 7 = L-High Alarm - # bit 6 = L-Low Alarm - # bit 5 = L-High Warning - # bit 4 = L-Low Warning - # bits 3-0 reserved/other - [ - 0b1010_0000, # temp byte 6: high alarm + high warning - 0b0101_0000, # vcc byte 7: low alarm + low warning - ], + # both monitors advertised (byte 220 bits 5 and 4 set) + True, True, + { + consts.TEMP_FLAGS_FIELD: 0b1010_0000, # high alarm + high warning + consts.VCC_FLAGS_FIELD: 0b0101_0000, # low alarm + low warning + }, { "tempHAlarm": True, "tempLAlarm": False, @@ -281,10 +284,13 @@ def test_get_transceiver_status_flags(self, mock_response, expected): "vccHWarn": False, "vccLWarn": True, }, + [consts.TEMP_FLAGS_FIELD, consts.VCC_FLAGS_FIELD], ), ( - # no flags asserted on either byte - [0b0000_0000, 0b0000_0000], + # both monitors advertised, no flags asserted: 0x00 is a real + # "no excursion" result and must still report all eight keys + True, True, + {consts.TEMP_FLAGS_FIELD: 0b0000_0000, consts.VCC_FLAGS_FIELD: 0b0000_0000}, { "tempHAlarm": False, "tempLAlarm": False, @@ -295,20 +301,114 @@ def test_get_transceiver_status_flags(self, mock_response, expected): "vccHWarn": False, "vccLWarn": False, }, + [consts.TEMP_FLAGS_FIELD, consts.VCC_FLAGS_FIELD], + ), + ( + # temperature monitoring not implemented (byte 220 bit 5 clear): + # the temp flag byte must not be read or reported at all + False, True, + {consts.VCC_FLAGS_FIELD: 0b1000_0000}, + { + "vccHAlarm": True, + "vccLAlarm": False, + "vccHWarn": False, + "vccLWarn": False, + }, + [consts.VCC_FLAGS_FIELD], + ), + ( + # supply voltage monitoring not implemented (byte 220 bit 4 clear) + True, False, + {consts.TEMP_FLAGS_FIELD: 0b0001_0000}, + { + "tempHAlarm": False, + "tempLAlarm": False, + "tempHWarn": False, + "tempLWarn": True, + }, + [consts.TEMP_FLAGS_FIELD], + ), + # neither monitor implemented (e.g. a copper cable): nothing is + # read and nothing is claimed, so xcvrd posts no DOM flags + (False, False, {}, {}, []), + # EEPROM read failure of a flag byte drops only that group; the + # other group is still reported and the absent keys render as N/A + # rather than as a False that was never measured + ( + True, True, + {consts.TEMP_FLAGS_FIELD: None, consts.VCC_FLAGS_FIELD: 0}, + { + "vccHAlarm": False, + "vccLAlarm": False, + "vccHWarn": False, + "vccLWarn": False, + }, + [consts.TEMP_FLAGS_FIELD, consts.VCC_FLAGS_FIELD], + ), + ( + True, True, + {consts.TEMP_FLAGS_FIELD: 0, consts.VCC_FLAGS_FIELD: None}, + { + "tempHAlarm": False, + "tempLAlarm": False, + "tempHWarn": False, + "tempLWarn": False, + }, + [consts.TEMP_FLAGS_FIELD, consts.VCC_FLAGS_FIELD], + ), + # read failure of the monitor advertisement itself is treated as + # "not implemented": that group is skipped, its flag byte is never + # read, and both cases render as N/A, so an unreadable + # advertisement cannot be mistaken for a measured in-limits result + ( + None, True, + {consts.VCC_FLAGS_FIELD: 0b0000_0000}, + { + "vccHAlarm": False, + "vccLAlarm": False, + "vccHWarn": False, + "vccLWarn": False, + }, + [consts.VCC_FLAGS_FIELD], + ), + ( + # a latched temperature alarm survives an unrelated failure of + # the voltage advertisement: no group is discarded on account + # of another group's failure + True, None, + {consts.TEMP_FLAGS_FIELD: 0b1000_0000}, + { + "tempHAlarm": True, + "tempLAlarm": False, + "tempHWarn": False, + "tempLWarn": False, + }, + [consts.TEMP_FLAGS_FIELD], ), - # EEPROM read failure of either byte returns None - ([None, 0b0000_0000], None), - ([0b0000_0000, None], None), ], ) - def test_get_transceiver_dom_flags(self, mock_response, expected): - self.api.xcvr_eeprom.read = MagicMock() - self.api.xcvr_eeprom.read.side_effect = mock_response + def test_get_transceiver_dom_flags(self, temp_support, vcc_support, eeprom, + expected, expected_reads): + self.api.get_temperature_support = MagicMock(return_value=temp_support) + self.api.get_voltage_support = MagicMock(return_value=vcc_support) + # Key the mock on the field name rather than call order, so a swapped + # or mis-mapped field would fail instead of silently passing. + self.api.xcvr_eeprom.read = MagicMock(side_effect=lambda field: eeprom[field]) + result = self.api.get_transceiver_dom_flags() + assert result == expected - # The flag latches clear on read: each byte must be read exactly - # once per call, in a single whole-byte access. - assert self.api.xcvr_eeprom.read.call_count == 2 + # The flag latches clear on read: each advertised byte must be read + # exactly once per call, in a single whole-byte access, and a byte + # whose monitor is not advertised must not be read at all. + assert [c.args[0] for c in self.api.xcvr_eeprom.read.call_args_list] == expected_reads + + def test_dom_flag_fields_map_to_table_6_6_bytes(self): + """TempFlags/VccFlags must resolve to lower page 00h bytes 6 and 7.""" + assert self.mem_map.get_field(consts.TEMP_FLAGS_FIELD).get_offset() == 6 + assert self.mem_map.get_field(consts.TEMP_FLAGS_FIELD).get_size() == 1 + assert self.mem_map.get_field(consts.VCC_FLAGS_FIELD).get_offset() == 7 + assert self.mem_map.get_field(consts.VCC_FLAGS_FIELD).get_size() == 1 @pytest.mark.parametrize("mock_response, expected",[ (