Hostboot HDAT: update to match section 19 in HDAT spec

Add new i2cLabel string

Change-Id: I007441e3973a16eaae4dbdbe703297f0f6978c8f
RTC: 176759
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/44301
Reviewed-by: Martin Gloff <mgloff@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

9 files changed, 378 insertions, 106 deletions

diff --git a/src/include/usr/i2c/i2cif.H b/src/include/usr/i2c/i2cif.H
index c0bee8be2..583eb5083 100644
--- a/src/include/usr/i2c/i2cif.H
+++ b/src/include/usr/i2c/i2cif.H
@@ -204,18 +204,6 @@ errlHndl_t i2cEnableSensorCache( TARGETING::Target * i_target );
 #endif //__HOSTBOOT_RUNTIME
 
 
-/**
- * Enumeration that defines possible I2C device types
- */
-enum DeviceType_t
-{
-    DEVTYPE_955x            = 0x01,
-    DEVTYPE_SEEPROM         = 0x02,
-    DEVTYPE_NUVOTON_TPM     = 0x03,
-    DEVTYPE_MEX_FPGA        = 0x04,
-    DEVTYPE_UCX90XX         = 0x05,
-    DEVTYPE_UNKNOWN         = 0xFF,
-};
 
 /**
  * Structure to return I2C information in
@@ -225,13 +213,19 @@ struct DeviceInfo_t
     TARGETING::Target* masterChip; //< I2C Master Chip
     uint8_t engine; //< I2C engine (relative to master chip)
     uint8_t masterPort; //< I2C port (relative to engine)
-    uint8_t deviceType; //< Slave device Type (from enum)
+    TARGETING::HDAT_I2C_DEVICE_TYPE deviceType; //< Slave device Type
     uint8_t addr; //< I2C Slave device address (relative to port)
     uint8_t slavePort; //< I2C Slave Port. 0xFF if N/A
     uint16_t busFreqKhz; //< Bus speed in KHz
-    uint8_t devicePurpose; //< Slave device purpose (from enum)
+    TARGETING::HDAT_I2C_DEVICE_PURPOSE devicePurpose; //< Slave device purpose
     TARGETING::ATTR_ORDINAL_ID_type assocNode; //< Upstream node's ordinal ID
     TARGETING::ATTR_POSITION_type assocProc; //< Upstream proc's position
+
+    // describes i2c device, null-terminated string
+    // SEEPROMS:        <vendor>,<device type>,<data type>,<hw subsystem>
+    // GPIO expanders:  <vendor>,<device type>,<domain>,<purpose>
+    // TPM:             <vendor>,<device type>,<purpose>,<scope>
+    char deviceLabel[64];
 };
 
 /**
diff --git a/src/usr/hdat/hdatutil.C b/src/usr/hdat/hdatutil.C
index 0e2ba3605..f13a4e89e 100644
--- a/src/usr/hdat/hdatutil.C
+++ b/src/usr/hdat/hdatutil.C
@@ -290,7 +290,7 @@ void hdatPopulateMTMAndSerialNumber()
         {
             const uint8_t l_mtmSize= 0x08;
             //phyp would requre just 8 character of MTM
-            strncpy(l_rawMTM,reinterpret_cast<const char*>(l_vpddata), 
+            strncpy(l_rawMTM,reinterpret_cast<const char*>(l_vpddata),
                     l_mtmSize);
             for(uint8_t i=0; i<sizeof(l_rawMTM); i++)
             {
@@ -612,7 +612,7 @@ errlHndl_t hdatGetAsciiKwdForPvpd(TARGETING::Target * i_target,
             theKeyword = i_fetchVpd[curCmd].keyword;
 
             //this conidtion is , if in the top loop there is a fail then
-            //theSize[curCmd] will be 0. 
+            //theSize[curCmd] will be 0.
             if( theSize[curCmd] == 0)
             {
                 continue;
@@ -1183,7 +1183,7 @@ errlHndl_t hdatGetAsciiKwdForCvpd(TARGETING::Target * i_target,
             theKeyword = i_fetchVpd[curCmd].keyword;
 
             //this conidtion is , if in the top loop there is a fail then
-            //theSize[curCmd] will be 0. 
+            //theSize[curCmd] will be 0.
             if( theSize[curCmd] == 0)
             {
                 continue;
@@ -1372,9 +1372,9 @@ void hdatPrintKwd(const char *i_kwd,
         for (l_cnt = 0; l_cnt < l_lines; l_cnt++)
         {
             HDAT_INF( "0X %08X %08X %08X %08X",
-                    (*(reinterpret_cast<uint32_t *>(l_kwd    ))) , 
+                    (*(reinterpret_cast<uint32_t *>(l_kwd    ))) ,
                     (*(reinterpret_cast<uint32_t *>(l_kwd +  4))),
-                    (*(reinterpret_cast<uint32_t *>(l_kwd + 8))) , 
+                    (*(reinterpret_cast<uint32_t *>(l_kwd + 8))) ,
                     (*(reinterpret_cast<uint32_t *>(l_kwd + 12)))  );
 
             i_kwd += 16;
@@ -1392,22 +1392,22 @@ void hdatPrintKwd(const char *i_kwd,
           else if ( l_rem < 9 )
           {
               HDAT_INF( "0X %08X %08X ",
-                   (*(reinterpret_cast<uint32_t *>(l_kwd    ))) , 
+                   (*(reinterpret_cast<uint32_t *>(l_kwd    ))) ,
                    (*(reinterpret_cast<uint32_t *>(l_kwd +  4))) );
           }
           else if ( l_rem < 13 )
           {
               HDAT_INF( "0X %08X %08X %08X ",
-                      (*(reinterpret_cast<uint32_t *>(l_kwd    ))) , 
+                      (*(reinterpret_cast<uint32_t *>(l_kwd    ))) ,
                       (*(reinterpret_cast<uint32_t *>(l_kwd +  4))),
                       (*(reinterpret_cast<uint32_t *>(l_kwd + 8))) );
           }
           else
           { // remainder is up to 15 bytes
               HDAT_INF( "0X %08X %08X %08X %08X",
-                    (*(reinterpret_cast<uint32_t *>(l_kwd    ))) , 
+                    (*(reinterpret_cast<uint32_t *>(l_kwd    ))) ,
                     (*(reinterpret_cast<uint32_t *>(l_kwd +  4))),
-                    (*(reinterpret_cast<uint32_t *>(l_kwd + 8))) , 
+                    (*(reinterpret_cast<uint32_t *>(l_kwd + 8))) ,
                     (*(reinterpret_cast<uint32_t *>(l_kwd + 12)))  );
           }
 
@@ -1592,7 +1592,7 @@ errlHndl_t hdatformatAsciiKwd(const struct vpdData i_fetchVpd[],
             l_loc += 2;
 
             uint8_t l_var = theSize[curCmd];
-            memcpy(reinterpret_cast<void *>(o_fmtKwd + l_loc),&l_var, 
+            memcpy(reinterpret_cast<void *>(o_fmtKwd + l_loc),&l_var,
                    sizeof(uint8_t));
 
             l_loc += sizeof(uint8_t);
@@ -1624,7 +1624,7 @@ errlHndl_t hdatGetFullEepromVpd(TARGETING::Target * i_target,
     HDAT_ENTER();
     if(i_target != NULL)
     {
-        o_data = new char[io_dataSize];   
+        o_data = new char[io_dataSize];
 
         //Collecting Full module VPD data
         err = deviceOp( DeviceFW::READ,
@@ -1760,11 +1760,10 @@ void hdatGetI2cDeviceInfo(
             l_hostI2cObj.hdatI2cSlavePort    = i2cDevice.slavePort;
             l_hostI2cObj.hdatI2cSlaveDevPurp = i2cDevice.devicePurpose;
             l_hostI2cObj.hdatI2cLinkId       = linkId.val;
-
-            // @TODO RTC 176759 Populate SLCA and I2C label
-            l_hostI2cObj.hdatI2cSlcaIndex    = 0;
-            memset(&l_hostI2cObj.hdatI2cLabel,0x00,
-                   sizeof(l_hostI2cObj.hdatI2cLabel));
+            strncpy(l_hostI2cObj.hdatI2cLabel,
+                    i2cDevice.deviceLabel,
+                    sizeof(l_hostI2cObj.hdatI2cLabel)-1);
+            // SLCA Index will be filled in by HDAT code
 
             // Don't include the device if the slave address is
             // invalid
diff --git a/src/usr/i2c/i2c.C b/src/usr/i2c/i2c.C
index 6a8463eae..307bef806 100755
--- a/src/usr/i2c/i2c.C
+++ b/src/usr/i2c/i2c.C
@@ -55,6 +55,7 @@
 #include <i2c/eepromif.H>
 #include <i2c/tpmddif.H>
 
+
 // ----------------------------------------------
 // Globals
 // ----------------------------------------------
@@ -3840,6 +3841,7 @@ void getMasterInfo( const TARGETING::Target* i_chip,
     }
 }
 
+
 //******************************************************************************
 // areI2cDevicesLogicallyEqual (std::unique equality comparison)
 //******************************************************************************
@@ -3883,6 +3885,67 @@ bool byI2cDeviceOrder(
     return lhsLogicallyBeforeRhs;
 }
 
+//******************************************************************************
+// removeI2CDeviceDuplicates
+//******************************************************************************
+void removeI2cDeviceDuplicates(std::vector<DeviceInfo_t>& io_deviceInfo)
+{
+    std::vector<DeviceInfo_t> l_unique_deviceInfo;
+
+    // Begin by sorting the list
+    // Order I2C devices by chip, engine, port, address, slave port
+    std::sort(io_deviceInfo.begin(), io_deviceInfo.end(),
+        byI2cDeviceOrder);
+
+    // Build up new unique list (thus removing duplicates)
+    if (io_deviceInfo.size() > 1)
+    {
+        auto currentItr = io_deviceInfo.begin();
+        auto nextItr = currentItr + 1;
+
+        do {
+            if (nextItr != io_deviceInfo.end() && (currentItr != NULL))
+            {
+                if (areI2cDevicesLogicallyEqual(*currentItr, *nextItr))
+                {
+                    // skip if first letter is ?, these are guessed defaults
+                    if (currentItr->deviceLabel[0] == '?')
+                    {
+                        // don't save currentItr as it is a guessed default
+                        currentItr = nextItr;
+                    }
+                }
+                else
+                {
+                    // Save currentItr as nextItr isn't the same logical device
+                    if (currentItr != NULL)
+                    {
+                        l_unique_deviceInfo.push_back(*currentItr);
+                    }
+                    currentItr = nextItr;
+                }
+            }
+            else
+            {
+                // Save currentItr if pointing at something valid
+                if (currentItr != NULL)
+                {
+                    l_unique_deviceInfo.push_back(*currentItr);
+                    currentItr = nextItr;
+                }
+            }
+
+            if (nextItr != io_deviceInfo.end())
+            {
+                ++nextItr;
+            }
+
+        } while (currentItr != io_deviceInfo.end());
+
+        io_deviceInfo = l_unique_deviceInfo;
+    }
+}
+
 /**
  * Retrieve some information about I2C devices that the Host
  * needs to know about
@@ -4029,11 +4092,15 @@ void getDeviceInfo( TARGETING::Target* i_i2cMaster,
                         //TODO RTC:165485 this isn't currently right. we'll need
                         //to add the changes in the enum and possibly the other
                         //struct/attribute.
+                        strcpy(l_currentDI.deviceLabel,
+                               "?atmel,28c128,vpd,module");
                         break;
                     case EEPROM::SBE_PRIMARY:
                     case EEPROM::SBE_BACKUP:
                         l_currentDI.devicePurpose =
                                 TARGETING::HDAT_I2C_DEVICE_PURPOSE_SBE_SEEPROM;
+                        strcpy(l_currentDI.deviceLabel,
+                               "?atmel,28c128,unknown,unknown");
                         break;
                     case EEPROM::LAST_CHIP_TYPE:
                         break;
@@ -4083,7 +4150,7 @@ void getDeviceInfo( TARGETING::Target* i_i2cMaster,
                         TARGETING::HDAT_I2C_DEVICE_TYPE_NUVOTON_TPM;
                 l_currentDI.devicePurpose =
                     TARGETING::HDAT_I2C_DEVICE_PURPOSE_TPM;
-
+                strcpy(l_currentDI.deviceLabel,"?nuvoton,npct601,tpm,host");
                 o_deviceInfo.push_back(l_currentDI);
 
             } //end of tpm iter
@@ -4150,6 +4217,12 @@ void getDeviceInfo( TARGETING::Target* i_i2cMaster,
             "ATTR_HDAT_I2C_DEVICE_PURPOSE attribute",
             TARGETING::get_huid(pChipTarget));
 
+        TARGETING::ATTR_HDAT_I2C_DEVICE_LABEL_type l_i2cDevLabel;
+        present = pChipTarget->tryGetAttr<
+            TARGETING::ATTR_HDAT_I2C_DEVICE_LABEL>(l_i2cDevLabel);
+        assert(present,"Target 0x%08X does not have ATTR_HDAT_I2C_DEVICE_LABEL "
+            "attribute",TARGETING::get_huid(pChipTarget));
+
         for(TARGETING::ATTR_HDAT_I2C_ELEMENTS_type l_idx=0;
             l_idx < l_arrayLength;
             ++l_idx)
@@ -4178,8 +4251,17 @@ void getDeviceInfo( TARGETING::Target* i_i2cMaster,
             l_currentDevice.slavePort = l_i2cSlavePort[l_idx];
             l_currentDevice.busFreqKhz = l_i2cBusFreq[l_idx]
                 / FREQ_CONVERSION::HZ_PER_KHZ;
-            l_currentDevice.deviceType = l_i2cDevType[l_idx];
-            l_currentDevice.devicePurpose = l_i2cDevPurpose[l_idx];
+            l_currentDevice.deviceType =
+                                static_cast<TARGETING::HDAT_I2C_DEVICE_TYPE>(
+                                                        l_i2cDevType[l_idx]);
+            l_currentDevice.devicePurpose =
+                                static_cast<TARGETING::HDAT_I2C_DEVICE_PURPOSE>(
+                                                        l_i2cDevPurpose[l_idx]);
+
+            memcpy(l_currentDevice.deviceLabel, l_i2cDevLabel[l_idx],
+                sizeof(l_currentDevice.deviceLabel));
+            l_currentDevice.deviceLabel[sizeof(l_currentDevice.deviceLabel) - 1]
+                = '\0';
 
             o_deviceInfo.push_back(l_currentDevice);
         }
@@ -4187,23 +4269,13 @@ void getDeviceInfo( TARGETING::Target* i_i2cMaster,
 
     } //end of per chip loop
 
-    // Order I2C devices by chip, engine, port, address, slave port
-    std::sort(o_deviceInfo.begin(), o_deviceInfo.end(),
-        byI2cDeviceOrder);
-
-    // Move logical duplicates to end
-    std::vector<DeviceInfo_t>::iterator
-        pInvalidEntries = std::unique(
-            o_deviceInfo.begin(),
-            o_deviceInfo.end(),
-            areI2cDevicesLogicallyEqual);
-
-    // Erase the duplicates
-    o_deviceInfo.erase(pInvalidEntries,o_deviceInfo.end());
+    // remove duplicates (also use deviceLabel from MRW, when possible)
+    removeI2cDeviceDuplicates(o_deviceInfo);
 
     TRACFCOMP(g_trac_i2c,"<<getDeviceInfo");
     return;
-};
+}
+
 
 /**
  * @brief Utility Function to capture error log user data consisting of
diff --git a/src/usr/runtime/populate_hbruntime.C b/src/usr/runtime/populate_hbruntime.C
index eb92807fe..74eef3b88 100644
--- a/src/usr/runtime/populate_hbruntime.C
+++ b/src/usr/runtime/populate_hbruntime.C
@@ -1487,7 +1487,9 @@ errlHndl_t populate_TpmInfoByNode()
                     l_i2cDev->hdatI2cSlaveDevType == i_i2cDevMrw.deviceType &&
                     l_i2cDev->hdatI2cSlaveDevAddr == i_i2cDevMrw.addr &&
                     l_i2cDev->hdatI2cSlavePort == i_i2cDevMrw.slavePort &&
-                    l_i2cDev->hdatI2cSlaveDevPurp == i_i2cDevMrw.devicePurpose;
+                    l_i2cDev->hdatI2cSlaveDevPurp == i_i2cDevMrw.devicePurpose
+                    &&
+                    !strcmp(l_i2cDev->hdatI2cLabel, i_i2cDevMrw.deviceLabel);
             });
 
             if (itr == l_i2cTargetList.end())
diff --git a/src/usr/targeting/common/processMrw.pl b/src/usr/targeting/common/processMrw.pl
index 70318f595..8cd17ed64 100755
--- a/src/usr/targeting/common/processMrw.pl
+++ b/src/usr/targeting/common/processMrw.pl
@@ -628,6 +628,7 @@ sub processProcessor
     my @speed = ();
     my @type = ();
     my @purpose = ();
+    my @label = ();
 
     $targetObj->log($target, "Processing PROC");
     foreach my $child (@{ $targetObj->getTargetChildren($target) })
@@ -696,7 +697,7 @@ sub processProcessor
         elsif (index($child,"i2c-master") != -1)
         {
             my ($i2cEngine, $i2cPort, $i2cSlavePort, $i2cAddr,
-                $i2cSpeed, $i2cType, $i2cPurpose) =
+                $i2cSpeed, $i2cType, $i2cPurpose, $i2cLabel) =
                     processI2C($targetObj, $child, $target);
 
             # Add this I2C device's information to the proc array
@@ -707,6 +708,7 @@ sub processProcessor
             push(@speed,@$i2cSpeed);
             push(@type,@$i2cType);
             push(@purpose,@$i2cPurpose);
+            push(@label, @$i2cLabel);
 
         }
     }
@@ -720,6 +722,7 @@ sub processProcessor
     my $speed_attr   = $speed[0];
     my $type_attr    = $type[0];
     my $purpose_attr = $purpose[0];
+    my $label_attr   = $label[0];
 
     # Parse out array to print as a string
     foreach my $n (1..($size-1))
@@ -731,6 +734,7 @@ sub processProcessor
         $speed_attr     .= ",".$speed[$n];
         $type_attr      .= ",".$type[$n];
         $purpose_attr   .= ",".$purpose[$n];
+        $label_attr     .= ",".$label[$n];
     }
 
     # Set the arrays to the corresponding attribute on the proc
@@ -741,6 +745,7 @@ sub processProcessor
     $targetObj->setAttribute($target,"HDAT_I2C_BUS_FREQ",$speed_attr);
     $targetObj->setAttribute($target,"HDAT_I2C_DEVICE_TYPE",$type_attr);
     $targetObj->setAttribute($target,"HDAT_I2C_DEVICE_PURPOSE",$purpose_attr);
+    $targetObj->setAttribute($target,"HDAT_I2C_DEVICE_LABEL", $label_attr);
     $targetObj->setAttribute($target,"HDAT_I2C_ELEMENTS",$size);
 
     ## update path for mvpd's and sbe's
@@ -1737,6 +1742,7 @@ sub processI2C
     my @i2cSpeed = ();
     my @i2cType = ();
     my @i2cPurpose = ();
+    my @i2cLabel = ();
 
     # Step 1: get I2C_ENGINE and PORT from <targetInstance>
 
@@ -1765,16 +1771,16 @@ sub processI2C
             # Most I2C devices will default the slave port, it is only valid
             # for gpio expanders.
             my $slavePort = "0xFF";
-            my $purpose = undef;
+            my $purpose_str = undef;
             if ($targetObj->isBusAttributeDefined(
                     $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_PURPOSE"))
             {
-                $purpose = $targetObj->getBusAttribute(
+                $purpose_str = $targetObj->getBusAttribute(
                                $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_PURPOSE");
             }
 
-            if(   defined $purpose
-               && $purpose ne "")
+            if(   defined $purpose_str
+               && $purpose_str ne "")
             {
                 my $parent = $targetObj->getTargetParent($i2c->{DEST});
                 foreach my $aTarget ( sort keys %{ $targetObj->getAllTargets()})
@@ -1788,7 +1794,7 @@ sub processI2C
 
                         my $pin = $targetObj->getAttribute($aTarget,
                                                            "PIN_NAME");
-                        if($pin eq $purpose)
+                        if($pin eq $purpose_str)
                         {
                             ($slavePort) = $aTarget =~ m/\-([0-9]+)$/g;
                             last;
@@ -1797,10 +1803,12 @@ sub processI2C
                 }
             }
 
+            my $type_str;
+            my $purpose;
             my $addr;
             my $speed;
             my $type;
-            my $purpose;
+            my $label;
 
             # For all these attributes, we need to check if they're defined,
             # and if not we set them to a default value.
@@ -1825,36 +1833,164 @@ sub processI2C
             if ($targetObj->isBusAttributeDefined(
                      $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_TYPE"))
             {
-                $type = $targetObj->getBusAttribute(
-                           $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_TYPE");
+                $type_str = $targetObj->getBusAttribute(
+                                $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_TYPE");
             }
 
-            if ($type eq "")
+            if ($type_str eq "")
             {
                 $type = "0xFF";
             }
             else
             {
-                $type = $targetObj->getEnumValue("HDAT_I2C_DEVICE_TYPE",$type);
+                $type = $targetObj->getEnumValue("HDAT_I2C_DEVICE_TYPE",$type_str);
             }
 
             if ($targetObj->isBusAttributeDefined(
                      $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_PURPOSE"))
             {
-                $purpose = $targetObj->getBusAttribute(
-                           $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_PURPOSE");
+                $purpose_str = $targetObj->getBusAttribute(
+                                $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_PURPOSE");
             }
 
-            if ($purpose eq "")
+            if ($purpose_str eq "")
             {
                 $purpose = "0xFF";
             }
             else
             {
                 $purpose = $targetObj->getEnumValue("HDAT_I2C_DEVICE_PURPOSE",
-                                                    $purpose);
+                                                    $purpose_str);
             }
 
+
+            if ($targetObj->isBusAttributeDefined(
+                     $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_LABEL"))
+            {
+                $label = $targetObj->getBusAttribute(
+                           $i2c->{SOURCE},$i2c->{BUS_NUM},"I2C_LABEL");
+            }
+
+            if ($label eq "")
+            {
+                # For SEEPROMS:
+                # <vendor>,<device type>, <data type>, <hw subsystem>
+                if (($type_str eq  "SEEPROM") ||
+                    ($type_str =~ m/SEEPROM_Atmel28c128/i))
+                {
+                    $label = "atmel,28c128,";
+                }
+                elsif($type_str =~ m/SEEPROM_Atmel28c256/i)
+                {
+                    $label = "atmel,28c256,";
+                }
+                if ($label ne "")
+                {
+                    if ($purpose_str =~ m/MODULE_VPD/)
+                    {
+                        $label .= "vpd,module";
+                    }
+                    elsif ($purpose_str =~ m/DIMM_SPD/)
+                    {
+                        $label .= "spd,dimm";
+                    }
+                    elsif ($purpose_str =~ m/PROC_MODULE_VPD/)
+                    {
+                        $label .= "vpd,module";
+                    }
+                    elsif ($purpose_str =~ m/SBE_SEEPROM/)
+                    {
+                        $label .= "image,sbe";
+                    }
+                    elsif ($purpose_str =~ m/PLANAR_VPD/)
+                    {
+                        $label .= "vpd,planar";
+                    }
+                    else
+                    {
+                        $label .= "unknown,unknown";
+                    }
+                }
+                # For GPIO expanders:
+                # <vendor>,<device type>,<domain>,<purpose>
+                if ($label eq "")
+                {
+                    if ($type_str =~ m/9551/)
+                    {
+                        $label = "nxp,pca9551,";
+                    }
+                    elsif ($type_str =~ m/9552/)
+                    {
+                        $label = "nxp,pca9552,";
+                    }
+                    elsif ($type_str =~ m/9553/)
+                    {
+                        $label = "nxp,pca9553,";
+                    }
+                    elsif ($type_str =~ m/9554/)
+                    {
+                        $label = "nxp,pca9554,";
+                    }
+                    elsif ($type_str =~ m/9555/)
+                    {
+                        $label = "nxp,pca9555,";
+                    }
+                    elsif($type_str =~ m/UCX90XX/)
+                    {
+                        $label = "ti,ucx90xx,";
+                    }
+
+                    if ($label ne "")
+                    {
+                        if ($purpose_str =~ m/CABLE_CARD_PRES/)
+                        {
+                            $label .= "cablecard,presence";
+                        }
+                        elsif ($purpose_str =~ m/PCI_HOTPLUG_PGOOD/)
+                        {
+                            $label .= "pcie-hotplug,pgood";
+                        }
+                        elsif ($purpose_str =~ m/PCI_HOTPLUG_CONTROL/)
+                        {
+                            $label .= "pcie-hotplug,control";
+                        }
+                        elsif ($purpose_str =~ m/WINDOW_OPEN/)
+                        {
+                            $label .= "secure-boot,window-open";
+                        }
+                        elsif ($purpose_str =~ m/PHYSICAL_PRESENCE/)
+                        {
+                            $label .= "secure-boot,physical-presence";
+                        }
+                        else
+                        {
+                            $label .= "unknown,unknown";
+                        }
+                    }
+                }
+
+                # For TPM:
+                # <vendor>,<device type>,<purpose>,<scope>
+                if ($type_str eq "NUVOTON_TPM")
+                {
+                    $label = "nuvoton,npct601,tpm,host";
+                }
+
+                if ($label eq "")
+                {
+                    $label = "unknown,unknown,unknown,unknown"
+                }
+
+                $label = '"' . $label . '"';
+
+            } # end of filling in default label values
+            elsif ($label !~ m/^\".*\"$/)
+            {
+                # add quotes around label
+                $label = '"' . $label . '"';
+            }
+
+
             # Step 3: For each connection, create an instance in the array
             #         for the DeviceInfo_t struct.
             push @i2cEngine, $engine;
@@ -1864,13 +2000,14 @@ sub processI2C
             push @i2cSpeed, $speed;
             push @i2cType, $type;
             push @i2cPurpose, $purpose;
+            push @i2cLabel, $label;
 
         }
     }
 
     # Return this i2c device's information back to the processor
     return (\@i2cEngine, \@i2cPort, \@i2cSlave, \@i2cAddr,
-            \@i2cSpeed, \@i2cType, \@i2cPurpose);
+            \@i2cSpeed, \@i2cType, \@i2cPurpose, \@i2cLabel);
 }
 
 
diff --git a/src/usr/targeting/common/xmltohb/attribute_types.xml b/src/usr/targeting/common/xmltohb/attribute_types.xml
index 357ffb31a..6b31664da 100755
--- a/src/usr/targeting/common/xmltohb/attribute_types.xml
+++ b/src/usr/targeting/common/xmltohb/attribute_types.xml
@@ -5738,84 +5738,123 @@
             type to the HDAT.  This is for I2C devices only.
     </description>
     <enumerator>
-      <name>955X</name>
-      <value>1</value>
+        <name>9551</name>
+        <value>0x1</value>
     </enumerator>
     <enumerator>
-      <name>SEEPROM</name>
-      <value>2</value>
+        <name>955x</name> <!-- deprecated -->
+        <value>0x1</value>
     </enumerator>
     <enumerator>
-      <name>NUVOTON_TPM</name>
-      <value>3</value>
+        <name>SEEPROM</name> <!-- deprecated -->
+        <value>0x2</value>
     </enumerator>
     <enumerator>
-      <name>MEX_FPGA</name>
-      <value>4</value>
+        <name>SEEPROM_Atmel28c128</name>
+        <value>0x2</value>
     </enumerator>
     <enumerator>
-      <name>UCX90XX</name>
-      <value>5</value>
+        <name>NUVOTON_TPM</name>
+        <value>0x3</value>
     </enumerator>
     <enumerator>
-      <name>NVLINK</name>
-      <value>6</value>
+        <name>MEX_FPGA</name>
+        <value>0x4</value>
     </enumerator>
     <enumerator>
-      <name>UNKNOWN</name>
-      <value>FF</value>
+        <name>UCX90XX</name>
+        <value>0x5</value>
+    </enumerator>
+    <enumerator>
+        <name>NVLINK</name> <!-- deprecated -->
+        <value>0x6</value>
+    </enumerator>
+    <enumerator>
+        <name>9552</name>
+        <value>0x6</value>
+    </enumerator>
+    <enumerator>
+        <name>9553</name>
+        <value>0x7</value>
+    </enumerator>
+    <enumerator>
+        <name>9554</name>
+        <value>0x8</value>
+    </enumerator>
+    <enumerator>
+        <name>9555</name>
+        <value>0x9</value>
+    </enumerator>
+    <enumerator>
+        <name>SMP_or_OpenCAPI_Cable</name>
+        <value>0xA</value>
+    </enumerator>
+    <enumerator>
+        <name>SEEPROM_Atmel28c256</name>
+        <value>0xB</value>
+    </enumerator>
+    <enumerator>
+        <name>UNKNOWN</name>
+        <value>0xFF</value>
     </enumerator>
     <id>HDAT_I2C_DEVICE_TYPE</id>
   </enumerationType>
 
   <enumerationType>
-    <id>HDAT_I2C_DEVICE_PURPOSE</id>
     <description>Pulled from the MRW, this describes the device
             purpose to the HDAT.  This is for I2C devices only.
     </description>
     <enumerator>
         <name>CABLE_CARD_PRES</name>
-        <value>1</value>
+        <value>0x1</value>
     </enumerator>
     <enumerator>
-        <name>CABLE_CARD_POWER_SENSE</name>
-        <value>2</value>
+        <name>PCI_HOTPLUG_PGOOD</name>
+        <value>0x2</value>
     </enumerator>
     <enumerator>
-        <name>CABLE_CARD_POWER_CONTROL</name>
-        <value>3</value>
+        <name>PCI_HOTPLUG_CONTROL</name>
+        <value>0x3</value>
     </enumerator>
     <enumerator>
         <name>TPM</name>
-        <value>4</value>
+        <value>0x4</value>
     </enumerator>
     <enumerator>
         <name>MODULE_VPD</name>
-        <value>5</value>
+        <value>0x5</value>
     </enumerator>
     <enumerator>
         <name>DIMM_SPD</name>
-        <value>6</value>
+        <value>0x6</value>
     </enumerator>
     <enumerator>
         <name>PROC_MODULE_VPD</name>
-        <value>7</value>
+        <value>0x7</value>
     </enumerator>
     <enumerator>
         <name>SBE_SEEPROM</name>
-        <value>8</value>
+        <value>0x8</value>
     </enumerator>
     <enumerator>
         <name>PLANAR_VPD</name>
-        <value>9</value>
+        <value>0x9</value>
+    </enumerator>
+    <enumerator>
+        <name>NVLINK_CABLE_TOPOLOGY_VERIFICATION</name>
+        <value>0xA</value>
+    </enumerator>
+    <enumerator>
+        <name>NVLINK</name> <!-- deprecated -->
+        <value>0xB</value>
     </enumerator>
     <enumerator>
-        <name>PCI_HOTPLUG</name>
-        <value>A</value>
+        <name>NVLINK_CABLE_MICRO_RESET</name>
+        <value>0xB</value>
     </enumerator>
     <enumerator>
-        <name>NVLINK</name>
-        <value>B</value>
+        <name>I2C_ASSOC_WITH_NVLINK_CABLE</name>
+        <value>0xC</value>
     </enumerator>
     <enumerator>
         <name>WINDOW_OPEN</name>
@@ -5826,11 +5865,17 @@
         <value>0xE</value>
     </enumerator>
     <enumerator>
+        <name>MEX_FPGA</name>
+        <value>0xF</value>
+    </enumerator>
+    <enumerator>
         <name>UNKNOWN</name>
-        <value>FF</value>
+        <value>0xFF</value>
     </enumerator>
+    <id>HDAT_I2C_DEVICE_PURPOSE</id>
   </enumerationType>
 
+
 <attribute>
     <id>FREQ_CORE_MAX</id>
     <description>
diff --git a/src/usr/targeting/common/xmltohb/attribute_types_openpower.xml b/src/usr/targeting/common/xmltohb/attribute_types_openpower.xml
index 387eb03be..44d48c70d 100644
--- a/src/usr/targeting/common/xmltohb/attribute_types_openpower.xml
+++ b/src/usr/targeting/common/xmltohb/attribute_types_openpower.xml
@@ -171,7 +171,6 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
 </attribute>
 
 <attribute>
@@ -187,7 +186,6 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
 </attribute>
 
 <attribute>
@@ -203,7 +201,6 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
 </attribute>
 
 <attribute>
@@ -219,7 +216,6 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
 </attribute>
 
 <attribute>
@@ -235,7 +231,6 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
 </attribute>
 
 <attribute>
@@ -251,7 +246,6 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
 </attribute>
 
 <attribute>
@@ -267,7 +261,23 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
+</attribute>
+
+<attribute>
+    <id>HDAT_I2C_DEVICE_LABEL</id>
+    <description>
+        This attribute holds the string descriptions of the I2C devices from
+        the i2c device connections as defined in the MRW.  It is parsed into
+        a struct in i2c.C
+    </description>
+    <simpleType>
+        <string>
+            <sizeInclNull>64</sizeInclNull>
+        </string>
+        <array>32</array>
+    </simpleType>
+    <persistency>non-volatile</persistency>
+    <readable/>
 </attribute>
 
 <attribute>
@@ -281,7 +291,6 @@
     </simpleType>
     <persistency>non-volatile</persistency>
     <readable/>
-    <writeable/>
 </attribute>
 
 <attribute>
diff --git a/src/usr/targeting/common/xmltohb/target_types_openpower.xml b/src/usr/targeting/common/xmltohb/target_types_openpower.xml
index f4b5c7208..1bc9bb654 100644
--- a/src/usr/targeting/common/xmltohb/target_types_openpower.xml
+++ b/src/usr/targeting/common/xmltohb/target_types_openpower.xml
@@ -40,6 +40,7 @@
     <attribute><id>HDAT_I2C_SLAVE_PORT</id></attribute>
     <attribute><id>HDAT_I2C_BUS_FREQ</id></attribute>
     <attribute><id>HDAT_I2C_DEVICE_PURPOSE</id></attribute>
+    <attribute><id>HDAT_I2C_DEVICE_LABEL</id></attribute>
     <attribute><id>HDAT_I2C_ELEMENTS</id></attribute>
     <attribute>
         <id>IPMI_INSTANCE</id>
diff --git a/src/usr/targeting/common/xmltohb/xmltohb.pl b/src/usr/targeting/common/xmltohb/xmltohb.pl
index 1d1370ecb..ba3f6e67d 100755
--- a/src/usr/targeting/common/xmltohb/xmltohb.pl
+++ b/src/usr/targeting/common/xmltohb/xmltohb.pl
@@ -2372,6 +2372,7 @@ sub writeTraitFileTraits {
                         $type = $simpleTypeProperties->{$typeName}{typeName};
                     }
 
+
                     if(   (exists $simpleType->{array})
                         && ($simpleTypeProperties->{$typeName}{supportsArray}) )
                     {
@@ -2381,7 +2382,8 @@ sub writeTraitFileTraits {
                             $dimensions .= "[$bound]";
                         }
                     }
-                    elsif(exists $simpleType->{string})
+
+                    if(exists $simpleType->{string})
                     {
                         # Create the string dimension
                         if(exists $simpleType->{string}->{sizeInclNull})
@@ -2390,6 +2392,7 @@ sub writeTraitFileTraits {
                             "[$simpleType->{string}->{sizeInclNull}]";
                         }
                     }
+
                     last;
                 }
             }
@@ -4608,7 +4611,7 @@ sub simpleTypeProperties {
 
     # Intentionally didn't wrap these to 80 columns to keep them lined up and
     # more readable/editable
-    $typesHoH{"string"}      = { supportsArray => 0, canBeHex => 0, complexTypeSupport => 0, typeName => "char"                       , bytes => 1, bits => 8 , default => \&defaultString, alignment => 1, specialPolicies =>\&enforceString,  packfmt =>\&packString};
+    $typesHoH{"string"}      = { supportsArray => 1, canBeHex => 0, complexTypeSupport => 0, typeName => "char"                       , bytes => 1, bits => 8 , default => \&defaultString, alignment => 1, specialPolicies =>\&enforceString,  packfmt =>\&packString};
     $typesHoH{"int8_t"}      = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 1, typeName => "int8_t"                     , bytes => 1, bits => 8 , default => \&defaultZero  , alignment => 1, specialPolicies =>\&null,           packfmt => "C" };
     $typesHoH{"int16_t"}     = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 1, typeName => "int16_t"                    , bytes => 2, bits => 16, default => \&defaultZero  , alignment => 1, specialPolicies =>\&null,           packfmt =>\&pack2byte};
     $typesHoH{"int32_t"}     = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 1, typeName => "int32_t"                    , bytes => 4, bits => 32, default => \&defaultZero  , alignment => 1, specialPolicies =>\&null,           packfmt =>\&pack4byte};
@@ -5327,6 +5330,7 @@ sub packAttribute {
                     # an array, if there are not enough values for the whole
                     # array then use the last value to fill in the remainder
 
+
                     # Figure out the array size (possibly multidimensional)
                     my $arraySize = 1;
                     my @bounds = split(/,/,$simpleType->{array});
@@ -5336,7 +5340,16 @@ sub packAttribute {
                     }
 
                     # Split the values into an array
-                    my @values = split(/,/,$value);
+                    my @values;
+                    if ($typeName eq "string")
+                    {
+                        # using "" around strings for array of strings
+                        (@values) = $value =~ m/"(.*?)"/g;
+                    }
+                    else
+                    {
+                        @values = split(/,/,$value);
+                    }
                     my $valueArraySize = scalar(@values);
 
                     # Iterate over the entire array creating values