Gracefully handle CMIS APIs for passive modules (sonic-net#238)

* Gracefully handle CMIS APIs for passive modules

Signed-off-by: Prince George <prgeor@microsoft.com>

* Address review comments

Signed-off-by: Prince George <prgeor@microsoft.com>

* Fix sfputil show eeprom for AOC

* Address review comment

* fix crash

* Fix comments

sonic_platform_base/sonic_xcvr/api/public/cmis.py

@@ -32,7 +32,7 @@ def get_model(self):
 
     def get_vendor_rev(self):
         '''
-        This function returns the revision level for part number provided by vendor 
+        This function returns the revision level for part number provided by vendor
         '''
         return self.xcvr_eeprom.read(consts.VENDOR_REV_FIELD)
 
@@ -58,6 +58,8 @@ def get_module_hardware_revision(self):
         '''
         This function returns the module hardware revision
         '''
+        if self.is_flat_memory():
+            return '0.0'
         hw_major_rev = self.xcvr_eeprom.read(consts.HW_MAJOR_REV)
         hw_minor_rev = self.xcvr_eeprom.read(consts.HW_MAJOR_REV)
         hw_rev = [str(num) for num in [hw_major_rev, hw_minor_rev]]
@@ -98,6 +100,8 @@ def get_module_inactive_firmware(self):
         '''
         This function returns the inactive firmware version
         '''
+        if self.is_flat_memory():
+            return 'N/A'
         inactive_fw_major = self.xcvr_eeprom.read(consts.INACTIVE_FW_MAJOR_REV)
         inactive_fw_minor = self.xcvr_eeprom.read(consts.INACTIVE_FW_MINOR_REV)
         inactive_fw = [str(num) for num in [inactive_fw_major, inactive_fw_minor]]
@@ -115,7 +119,7 @@ def get_transceiver_info(self):
         xcvr_info = {
             "type": admin_info[consts.ID_FIELD],
             "type_abbrv_name": admin_info[consts.ID_ABBRV_FIELD],
-            "hardware_rev": '.'.join([str(admin_info[consts.HW_MAJOR_REV]),str(admin_info[consts.HW_MINOR_REV])]),
+            "hardware_rev": self.get_module_hardware_revision(),
             "serial": admin_info[consts.VENDOR_SERIAL_NO_FIELD],
             "manufacturer": admin_info[consts.VENDOR_NAME_FIELD],
             "model": admin_info[consts.VENDOR_PART_NO_FIELD],
@@ -139,15 +143,9 @@ def get_transceiver_info(self):
         xcvr_info['host_lane_assignment_option'] = self.get_host_lane_assignment_option()
         xcvr_info['media_lane_assignment_option'] = self.get_media_lane_assignment_option()
         apsel_dict = self.get_active_apsel_hostlane()
-        xcvr_info['active_apsel_hostlane1'] = apsel_dict['ActiveAppSelLane1']
-        xcvr_info['active_apsel_hostlane2'] = apsel_dict['ActiveAppSelLane2']
-        xcvr_info['active_apsel_hostlane3'] = apsel_dict['ActiveAppSelLane3']
-        xcvr_info['active_apsel_hostlane4'] = apsel_dict['ActiveAppSelLane4']
-        xcvr_info['active_apsel_hostlane5'] = apsel_dict['ActiveAppSelLane5']
-        xcvr_info['active_apsel_hostlane6'] = apsel_dict['ActiveAppSelLane6']
-        xcvr_info['active_apsel_hostlane7'] = apsel_dict['ActiveAppSelLane7']
-        xcvr_info['active_apsel_hostlane8'] = apsel_dict['ActiveAppSelLane8']
-
+        for lane in range(1, self.NUM_CHANNELS+1):
+            xcvr_info["%s%d" % ("active_apsel_hostlane", lane)] = \
+                    apsel_dict["%s%d" % (consts.ACTIVE_APSEL_HOSTLANE, lane)]
         xcvr_info['media_interface_technology'] = self.get_media_interface_technology()
         xcvr_info['vendor_rev'] = self.get_vendor_rev()
         xcvr_info['cmis_rev'] = self.get_cmis_rev()
@@ -188,9 +186,9 @@ def get_transceiver_bulk_status(self):
         }
 
         for i in range(1, self.NUM_CHANNELS + 1):
-            bulk_status["tx%dbias" % i] = tx_bias['LaserBiasTx%dField' % i]
-            bulk_status["rx%dpower" % i] = rx_power['OpticalPowerRx%dField' %i]
-            bulk_status["tx%dpower" % i] = tx_power['OpticalPowerTx%dField' %i]
+            bulk_status["tx%dbias" % i] = tx_bias[i - 1]
+            bulk_status["rx%dpower" % i] = rx_power[i - 1]
+            bulk_status["tx%dpower" % i] = tx_power[i - 1]
 
         laser_temp_dict = self.get_laser_temperature()
         bulk_status['laser_temperature'] = laser_temp_dict['monitor value']
@@ -257,7 +255,7 @@ def get_transceiver_threshold_info(self):
             threshold_info_dict['prefecberhighalarm'] = self.vdm_dict['Pre-FEC BER Average Media Input'][1][1]
             threshold_info_dict['prefecberlowalarm'] = self.vdm_dict['Pre-FEC BER Average Media Input'][1][2]
             threshold_info_dict['prefecberhighwarning'] = self.vdm_dict['Pre-FEC BER Average Media Input'][1][3]
-            threshold_info_dict['prefecberlowwarning'] = self.vdm_dict['Pre-FEC BER Average Media Input'][1][4]     
+            threshold_info_dict['prefecberlowwarning'] = self.vdm_dict['Pre-FEC BER Average Media Input'][1][4]
             threshold_info_dict['postfecberhighalarm'] = self.vdm_dict['Errored Frames Average Media Input'][1][1]
             threshold_info_dict['postfecberlowalarm'] = self.vdm_dict['Errored Frames Average Media Input'][1][2]
             threshold_info_dict['postfecberhighwarning'] = self.vdm_dict['Errored Frames Average Media Input'][1][3]
@@ -459,9 +457,13 @@ def get_tx_bias(self):
         tx_bias_support = self.get_tx_bias_support()
         if tx_bias_support is None:
             return None
-        if not tx_bias_support:
-            return ["N/A" for _ in range(self.NUM_CHANNELS)]
-        tx_bias = self.xcvr_eeprom.read(consts.TX_BIAS_FIELD)
+        tx_bias = ["N/A" for _ in range(self.NUM_CHANNELS)]
+
+        if tx_bias_support:
+            tx_bias = self.xcvr_eeprom.read(consts.TX_BIAS_FIELD)
+            if tx_bias is not None:
+                tx_bias = [tx_bias['LaserBiasTx%dField' % i] for i in range(1, self.NUM_CHANNELS+1)]
+
         return tx_bias
 
     def get_tx_power(self):
@@ -471,9 +473,14 @@ def get_tx_power(self):
         tx_power_support = self.get_tx_power_support()
         if tx_power_support is None:
             return None
-        if not tx_power_support:
-            return ["N/A" for _ in range(self.NUM_CHANNELS)]
-        tx_power = self.xcvr_eeprom.read(consts.TX_POWER_FIELD)
+
+        tx_power = ["N/A" for _ in range(self.NUM_CHANNELS)]
+
+        if tx_power_support:
+            tx_power = self.xcvr_eeprom.read(consts.TX_POWER_FIELD)
+            if tx_power is not None:
+                tx_power =  [tx_power['OpticalPowerTx%dField' %i] for i in range(1, self.NUM_CHANNELS+1)]
+
         return tx_power
 
     def get_tx_power_support(self):
@@ -486,9 +493,14 @@ def get_rx_power(self):
         rx_power_support = self.get_rx_power_support()
         if rx_power_support is None:
             return None
-        if not rx_power_support:
-            return ["N/A" for _ in range(self.NUM_CHANNELS)]
-        rx_power = self.xcvr_eeprom.read(consts.RX_POWER_FIELD)
+
+        rx_power = ["N/A" for _ in range(self.NUM_CHANNELS)]
+
+        if rx_power_support:
+            rx_power = self.xcvr_eeprom.read(consts.RX_POWER_FIELD)
+            if rx_power is not None:
+                rx_power = [rx_power['OpticalPowerRx%dField' %i] for i in range(1, self.NUM_CHANNELS+1)]
+
         return rx_power
 
     def get_rx_power_support(self):
@@ -596,6 +608,8 @@ def get_module_media_type(self):
         '''
         This function returns module media type: MMF, SMF, Passive Copper Cable, Active Cable Assembly or Base-T.
         '''
+        if self.is_flat_memory():
+            return 'N/A'
         return self.xcvr_eeprom.read(consts.MEDIA_TYPE_FIELD)
 
     def get_host_electrical_interface(self):
@@ -608,6 +622,9 @@ def get_module_media_interface(self):
         '''
         This function returns module media electrical interface. Table 4-6 ~ 4-10 in SFF-8024 Rev4.6
         '''
+        if self.is_flat_memory():
+            return 'N/A'
+
         media_type = self.get_module_media_type()
         if media_type == 'Multimode Fiber (MMF)':
             return self.xcvr_eeprom.read(consts.MODULE_MEDIA_INTERFACE_850NM)
@@ -631,15 +648,16 @@ def is_coherent_module(self):
 
     def get_host_lane_count(self):
         '''
-        This function returns number of host lanes
+        This function returns number of host lanes for default application
         '''
         return self.xcvr_eeprom.read(consts.HOST_LANE_COUNT)
 
-
     def get_media_lane_count(self):
         '''
-        This function returns number of media lanes
+        This function returns number of media lanes for default application
         '''
+        if self.is_flat_memory():
+            return 0
         return self.xcvr_eeprom.read(consts.MEDIA_LANE_COUNT)
 
     def get_media_interface_technology(self):
@@ -658,13 +676,21 @@ def get_media_lane_assignment_option(self):
         '''
         This function returns the media lane that the application is allowed to begin on
         '''
+        if self.is_flat_memory():
+            return 'N/A'
         return self.xcvr_eeprom.read(consts.MEDIA_LANE_ASSIGNMENT_OPTION)
 
     def get_active_apsel_hostlane(self):
         '''
         This function returns the application select code that each host lane has
         '''
-        return self.xcvr_eeprom.read(consts.ACTIVE_APSEL_CODE)
+        apsel_dict = {}
+        if self.is_flat_memory():
+            for lane in range(1, self.NUM_CHANNELS+1):
+                apsel_dict["%s%d" % (consts.ACTIVE_APSEL_HOSTLANE, lane)] = 'N/A'
+        else:
+            apsel_dict = self.xcvr_eeprom.read(consts.ACTIVE_APSEL_CODE)
+        return apsel_dict
 
     def get_tx_config_power(self):
         '''
@@ -730,6 +756,17 @@ def get_laser_temperature(self):
         '''
         This function returns the laser temperature monitor value
         '''
+        laser_temp_dict = {
+            'monitor value' : 'N/A',
+            'high alarm' : 'N/A',
+            'low alarm' : 'N/A',
+            'high warn' : 'N/A',
+            'low warn' : 'N/A'
+        }
+
+        if self.is_flat_memory():
+            return laser_temp_dict
+
         try:
             aux1_mon_type, aux2_mon_type, aux3_mon_type = self.get_aux_mon_type()
         except TypeError:
@@ -748,7 +785,8 @@ def get_laser_temperature(self):
             laser_temp_high_warn = self.xcvr_eeprom.read(consts.AUX3_HIGH_WARN)/LASER_TEMP_SCALE
             laser_temp_low_warn = self.xcvr_eeprom.read(consts.AUX3_LOW_WARN)/LASER_TEMP_SCALE
         else:
-            return None
+            return laser_temp_dict
+
         laser_temp_dict = {'monitor value': laser_temp,
                            'high alarm': laser_temp_high_alarm,
                            'low alarm': laser_temp_low_alarm,
@@ -913,7 +951,7 @@ def get_vdm(self):
             self.vdm
         except AttributeError:
             self.get_vdm_api()
-        vdm = self.vdm.get_vdm_allpage()
+        vdm = self.vdm.get_vdm_allpage() if not self.is_flat_memory() else {}
         return vdm
 
     def get_module_firmware_fault_state_changed(self):
@@ -1100,7 +1138,7 @@ def get_module_fw_info(self):
             else:
                 ImageA = "N/A"
             txt += 'Image A Version: %s\n' %ImageA
-            
+
             if ImageBValid == 0:
                 # Registers 9Fh:174,175; 176.177
                 ImageB = '%d.%d.%d' %(rpl[38], rpl[39], ((rpl[40]<< 8) | rpl[41]))
@@ -1119,7 +1157,7 @@ def get_module_fw_info(self):
             elif ImageBCommitted == 1:
                 CommittedImage = 'B'
             else:
-                CommittedImage = 'N/A'            
+                CommittedImage = 'N/A'
             txt += 'Running Image: %s; Committed Image: %s\n' %(RunningImage, CommittedImage)
         else:
             txt += 'Reply payload check code error\n'
@@ -1142,8 +1180,8 @@ def module_fw_run(self, mode = 0x01):
         02h = Traffic affecting Reset to Running Image.
         03h = Attempt Hitless Reset to Running Image
 
-        This function returns True if firmware run successfully completes. 
-        Otherwise it will return False. 
+        This function returns True if firmware run successfully completes.
+        Otherwise it will return False.
         """
         try:
             self.cdb
@@ -1177,8 +1215,8 @@ def module_fw_commit(self):
         The host uses this command to commit the running image
         so that the module will boot from it on future boots.
 
-        This function returns True if firmware commit successfully completes. 
-        Otherwise it will return False. 
+        This function returns True if firmware commit successfully completes.
+        Otherwise it will return False.
         """
         try:
             self.cdb
@@ -1327,7 +1365,7 @@ def module_fw_upgrade(self, imagepath):
         imagepath specifies where firmware image file is located.
         target_firmware is a string that specifies the firmware version to upgrade to
 
-        This function returns True if download successfully completes. 
+        This function returns True if download successfully completes.
         Otherwise it will return False.
         """
         result = self.get_module_fw_info()
@@ -1336,7 +1374,7 @@ def module_fw_upgrade(self, imagepath):
         except (ValueError, TypeError):
             return result['status'], result['info']
         result = self.get_module_fw_upgrade_feature()
-        try: 
+        try:
             startLPLsize, maxblocksize, lplonly_flag, autopaging_flag, writelength = result['result']
         except (ValueError, TypeError):
             return result['status'], result['info']
@@ -1351,7 +1389,7 @@ def module_fw_upgrade(self, imagepath):
     def module_fw_switch(self):
         """
         This function switch the active/inactive module firmware in the current module memory
-        This function returns True if firmware switch successfully completes. 
+        This function returns True if firmware switch successfully completes.
         Otherwise it will return False.
         If not both images are valid, it will stop firmware switch and return False
         """
@@ -1572,7 +1610,7 @@ def get_transceiver_status(self):
         trans_status['vcclowalarm_flag'] = module_flag['voltage_flags']['voltage_low_alarm_flag']
         trans_status['vcchighwarning_flag'] = module_flag['voltage_flags']['voltage_high_warn_flag']
         trans_status['vcclowwarning_flag'] = module_flag['voltage_flags']['voltage_low_warn_flag']
-        try: 
+        try:
             aux1_mon_type, aux2_mon_type, aux3_mon_type = self.get_aux_mon_type()
             if aux2_mon_type == 0:
                 trans_status['lasertemphighalarm_flag'] = module_flag['aux2_flags']['aux2_high_alarm_flag']

sonic_platform_base/sonic_xcvr/fields/consts.py

@@ -124,7 +124,7 @@
 RX_PWR_1_FIELD = "Rx_PWR(1)"
 RX_PWR_0_FIELD = "Rx_PWR(0)"
 TX_I_SLOPE_FIELD = "Tx_I(Slope)"
-TX_I_OFFSET_FIELD = "Tx_I(Offset)" 
+TX_I_OFFSET_FIELD = "Tx_I(Offset)"
 TX_PWR_SLOPE_FIELD = "Tx_PWR(Slope)"
 TX_PWR_OFFSET_FIELD = "Tx_PWR(Offset)"
 T_SLOPE_FIELD = "T (Slope)"
@@ -143,6 +143,7 @@
 # CMIS
 
 ADMIN_INFO_FIELD = "AdminInfo"
+ADVERTISING_FIELD = "Advertising"
 MEDIA_TYPE_FIELD = "ModuleMediaType"
 HOST_ELECTRICAL_INTERFACE = "HostElectricalInterfaceID"
 MODULE_MEDIA_INTERFACE_850NM = "ModuleMediaInterface850nm"

sonic_platform_base/sonic_xcvr/mem_maps/public/cmis.py

@@ -27,8 +27,9 @@ def __init__(self, codes):
             )
         )
 
+        # Should contain ONLY Lower page fields
         self.ADMIN_INFO = RegGroupField(consts.ADMIN_INFO_FIELD,
-            CodeRegField(consts.ID_FIELD, self.getaddr(0x0, 128), self.codes.XCVR_IDENTIFIERS),
+            CodeRegField(consts.ID_FIELD, self.getaddr(0x0, 0), self.codes.XCVR_IDENTIFIERS),
             CodeRegField(consts.ID_ABBRV_FIELD, self.getaddr(0x0, 128), self.codes.XCVR_IDENTIFIER_ABBRV),
             StringRegField(consts.VENDOR_NAME_FIELD, self.getaddr(0x0, 129), size=16),
             HexRegField(consts.VENDOR_OUI_FIELD, self.getaddr(0x0, 145), size=3),
@@ -57,17 +58,14 @@ def __init__(self, codes):
             CodeRegField(consts.MODULE_MEDIA_INTERFACE_PASSIVE_COPPER, self.getaddr(0x0, 87), self.codes.PASSIVE_COPPER_MEDIA_INTERFACE),
             CodeRegField(consts.MODULE_MEDIA_INTERFACE_ACTIVE_CABLE, self.getaddr(0x0, 87), self.codes.ACTIVE_CABLE_MEDIA_INTERFACE),
             CodeRegField(consts.MODULE_MEDIA_INTERFACE_BASE_T, self.getaddr(0x0, 87), self.codes.BASE_T_MEDIA_INTERFACE),
-            NumberRegField(consts.MEDIA_LANE_COUNT, self.getaddr(0x0, 88), 
+            NumberRegField(consts.MEDIA_LANE_COUNT, self.getaddr(0x0, 88),
                 *(RegBitField("Bit%d" % (bit), bit) for bit in range (0, 4))
             ),
-            NumberRegField(consts.HOST_LANE_COUNT, self.getaddr(0x0, 88), 
+            NumberRegField(consts.HOST_LANE_COUNT, self.getaddr(0x0, 88),
                 *(RegBitField("Bit%d" % (bit), bit) for bit in range (4, 8))
             ),
             NumberRegField(consts.HOST_LANE_ASSIGNMENT_OPTION, self.getaddr(0x0, 89), format="B", size=1),
-            NumberRegField(consts.MEDIA_LANE_ASSIGNMENT_OPTION, self.getaddr(0x1, 176), format="B", size=1),
             CodeRegField(consts.MEDIA_INTERFACE_TECH, self.getaddr(0x0, 212), self.codes.MEDIA_INTERFACE_TECH),
-            NumberRegField(consts.HW_MAJOR_REV, self.getaddr(0x1, 130), size=1),
-            NumberRegField(consts.HW_MINOR_REV, self.getaddr(0x1, 131), size=1),
             NumberRegField(consts.CMIS_MAJOR_REVISION, self.getaddr(0x0, 1),
                 *(RegBitField("Bit%d" % (bit), bit) for bit in range (4, 8))
             ),
@@ -76,6 +74,13 @@ def __init__(self, codes):
             ),
             NumberRegField(consts.ACTIVE_FW_MAJOR_REV, self.getaddr(0x0, 39), format="B", size=1),
             NumberRegField(consts.ACTIVE_FW_MINOR_REV, self.getaddr(0x0, 40), format="B", size=1),
+        )
+
+        # Should contain ONLY upper page fields
+        self.ADVERTISING = RegGroupField(consts.ADVERTISING_FIELD,
+            NumberRegField(consts.HW_MAJOR_REV, self.getaddr(0x1, 130), size=1),
+            NumberRegField(consts.HW_MINOR_REV, self.getaddr(0x1, 131), size=1),
+            NumberRegField(consts.MEDIA_LANE_ASSIGNMENT_OPTION, self.getaddr(0x1, 176), format="B", size=1),
             NumberRegField(consts.INACTIVE_FW_MAJOR_REV, self.getaddr(0x1, 128), format="B", size=1),
             NumberRegField(consts.INACTIVE_FW_MINOR_REV, self.getaddr(0x1, 129), format="B", size=1),