X300: Prevent MB EEPROM Corruption

- Load EEPROM data into firmware memory to access from there instead of driving the I2C bus directly
- Fixed firmware performance issues by removing the popcntll() function and reducing frequency of background tasks to once every 10ms
- Added x300_mb_eeprom_iface to handle cases of devices with older and newer firmware
- Added checks for claim to device before driving the I2C bus

7 files changed, 333 insertions, 31 deletions

diff --git a/host/lib/usrp/x300/CMakeLists.txt b/host/lib/usrp/x300/CMakeLists.txt
index ea237b008..5f8e83e1c 100644
--- a/host/lib/usrp/x300/CMakeLists.txt
+++ b/host/lib/usrp/x300/CMakeLists.txt
@@ -34,6 +34,7 @@ IF(ENABLE_X300)
         ${CMAKE_CURRENT_SOURCE_DIR}/x300_dboard_iface.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/x300_clock_ctrl.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/x300_image_loader.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/x300_mb_eeprom.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/cdecode.c
     )
 ENDIF(ENABLE_X300)
diff --git a/host/lib/usrp/x300/x300_fw_common.h b/host/lib/usrp/x300/x300_fw_common.h
index aae0f769c..b83449666 100644
--- a/host/lib/usrp/x300/x300_fw_common.h
+++ b/host/lib/usrp/x300/x300_fw_common.h
@@ -32,7 +32,7 @@ extern "C" {
 #define X300_REVISION_COMPAT 7
 #define X300_REVISION_MIN    2
 #define X300_FW_COMPAT_MAJOR 5
-#define X300_FW_COMPAT_MINOR 0
+#define X300_FW_COMPAT_MINOR 1
 #define X300_FPGA_COMPAT_MAJOR 0x21
 
 //shared memory sections - in between the stack and the program space
@@ -49,6 +49,9 @@ extern "C" {
 #define X300_FW_SHMEM_UART_WORDS32 10
 #define X300_FW_SHMEM_ROUTE_MAP_ADDR 11
 #define X300_FW_SHMEM_ROUTE_MAP_LEN 12
+#define X300_FW_SHMEM_IDENT 13 // (13-39) EEPROM values in use
+#define X300_FW_SHMEM_DEBUG 128
+#define X300_FW_SHMEM_ADDR(offset) X300_FW_SHMEM_BASE + (4 * (offset))
 
 #define X300_FW_NUM_BYTES (1 << 15) //64k
 #define X300_FW_COMMS_MTU (1 << 13) //8k
@@ -94,6 +97,30 @@ extern "C" {
 
 typedef struct
 {
+    //indentifying numbers
+    unsigned char revision[2];
+    unsigned char product[2];
+    uint8_t _pad0[4];
+
+    //all the mac addrs
+    uint8_t mac_addr0[6];
+    uint8_t _pad1[2];
+    uint8_t mac_addr1[6];
+    uint8_t _pad2[2];
+
+    //all the IP addrs
+    uint32_t gateway;
+    uint32_t subnet[4];
+    uint32_t ip_addr[4];
+    uint8_t _pad3[16];
+
+    //names and serials
+    unsigned char name[23];
+    unsigned char serial[9];
+} x300_eeprom_map_t;
+
+typedef struct
+{
     uint32_t flags;
     uint32_t sequence;
     uint32_t addr;
diff --git a/host/lib/usrp/x300/x300_impl.cpp b/host/lib/usrp/x300/x300_impl.cpp
index 50a7f3ded..d6e2a6fdc 100644
--- a/host/lib/usrp/x300/x300_impl.cpp
+++ b/host/lib/usrp/x300/x300_impl.cpp
@@ -18,6 +18,7 @@
 #include "x300_impl.hpp"
 #include "x300_lvbitx.hpp"
 #include "x310_lvbitx.hpp"
+#include "x300_mb_eeprom.hpp"
 #include "apply_corrections.hpp"
 #include <boost/algorithm/string.hpp>
 #include <boost/asio.hpp>
@@ -121,12 +122,16 @@ static device_addrs_t x300_find_with_addr(const device_addr_t &hint)
                 false /* Suppress timeout errors */
             );
 
-            if (x300_impl::is_claimed(zpu_ctrl)) continue; //claimed by another process
             new_addr["fpga"] = get_fpga_option(zpu_ctrl);
 
             i2c_core_100_wb32::sptr zpu_i2c = i2c_core_100_wb32::make(zpu_ctrl, I2C1_BASE);
-            i2c_iface::sptr eeprom16 = zpu_i2c->eeprom16();
-            const mboard_eeprom_t mb_eeprom(*eeprom16, "X300");
+            x300_mb_eeprom_iface::sptr eeprom_iface = x300_mb_eeprom_iface::make(zpu_ctrl, zpu_i2c);
+            const mboard_eeprom_t mb_eeprom(*eeprom_iface, "X300");
+            if (mb_eeprom.size() == 0 or x300_impl::claim_status(zpu_ctrl) == x300_impl::CLAIMED_BY_OTHER)
+            {
+                // Skip device claimed by another process
+                continue;
+            }
             new_addr["name"] = mb_eeprom["name"];
             new_addr["serial"] = mb_eeprom["serial"];
             switch (x300_impl::get_mb_type_from_eeprom(mb_eeprom)) {
@@ -221,7 +226,6 @@ static device_addrs_t x300_find_pcie(const device_addr_t &hint, bool explicit_qu
                 //We don't put this zpu_ctrl in the registry because we need
                 //a persistent niriok_proxy associated with the object
             }
-            if (x300_impl::is_claimed(zpu_ctrl)) continue; //claimed by another process
 
             //Attempt to autodetect the FPGA type
             if (not hint.has_key("fpga")) {
@@ -229,8 +233,13 @@ static device_addrs_t x300_find_pcie(const device_addr_t &hint, bool explicit_qu
             }
 
             i2c_core_100_wb32::sptr zpu_i2c = i2c_core_100_wb32::make(zpu_ctrl, I2C1_BASE);
-            i2c_iface::sptr eeprom16 = zpu_i2c->eeprom16();
-            const mboard_eeprom_t mb_eeprom(*eeprom16, "X300");
+            x300_mb_eeprom_iface::sptr eeprom_iface = x300_mb_eeprom_iface::make(zpu_ctrl, zpu_i2c);
+            const mboard_eeprom_t mb_eeprom(*eeprom_iface, "X300");
+            if (mb_eeprom.size() == 0 or x300_impl::claim_status(zpu_ctrl) == x300_impl::CLAIMED_BY_OTHER)
+            {
+                // Skip device claimed by another process
+                continue;
+            }
             new_addr["name"] = mb_eeprom["name"];
             new_addr["serial"] = mb_eeprom["serial"];
         }
@@ -307,12 +316,7 @@ device_addrs_t x300_find(const device_addr_t &hint_)
         {
             UHD_MSG(error) << "X300 Network discovery unknown error " << std::endl;
         }
-        BOOST_FOREACH(const device_addr_t &reply_addr, reply_addrs)
-        {
-            device_addrs_t new_addrs = x300_find_with_addr(reply_addr);
-            addrs.insert(addrs.begin(), new_addrs.begin(), new_addrs.end());
-        }
-        return addrs;
+        return reply_addrs;
     }
 
     if (!hint.has_key("resource"))
@@ -671,6 +675,10 @@ void x300_impl::setup_mb(const size_t mb_i, const uhd::device_addr_t &dev_addr)
                     mb.get_pri_eth().addr, BOOST_STRINGIZE(X300_FW_COMMS_UDP_PORT)));
     }
 
+    // Claim device
+    if (not try_to_claim(mb.zpu_ctrl)) {
+        throw uhd::runtime_error("Failed to claim device");
+    }
     mb.claimer_task = uhd::task::make(boost::bind(&x300_impl::claimer_loop, this, mb.zpu_ctrl));
 
     //extract the FW path for the X300
@@ -726,7 +734,7 @@ void x300_impl::setup_mb(const size_t mb_i, const uhd::device_addr_t &dev_addr)
     // setup the mboard eeprom
     ////////////////////////////////////////////////////////////////////
     UHD_MSG(status) << "Loading values from EEPROM..." << std::endl;
-    i2c_iface::sptr eeprom16 = mb.zpu_i2c->eeprom16();
+    x300_mb_eeprom_iface::sptr eeprom16 = x300_mb_eeprom_iface::make(mb.zpu_ctrl, mb.zpu_i2c);
     if (dev_addr.has_key("blank_eeprom"))
     {
         UHD_MSG(warning) << "Obliterating the motherboard EEPROM..." << std::endl;
@@ -1011,8 +1019,7 @@ x300_impl::~x300_impl(void)
             mb.claimer_task.reset();
             {   //Critical section
                 boost::mutex::scoped_lock(pcie_zpu_iface_registry_mutex);
-                mb.zpu_ctrl->poke32(SR_ADDR(X300_FW_SHMEM_BASE, X300_FW_SHMEM_CLAIM_TIME), 0);
-                mb.zpu_ctrl->poke32(SR_ADDR(X300_FW_SHMEM_BASE, X300_FW_SHMEM_CLAIM_SRC), 0);
+                release(mb.zpu_ctrl);
                 //If the process is killed, the entire registry will disappear so we
                 //don't need to worry about unclean shutdowns here.
                 get_pcie_zpu_iface_registry().pop(mb.get_pri_eth().addr);
@@ -1465,24 +1472,58 @@ void x300_impl::set_mb_eeprom(i2c_iface::sptr i2c, const mboard_eeprom_t &mb_eep
 
 void x300_impl::claimer_loop(wb_iface::sptr iface)
 {
-    {   //Critical section
-        boost::mutex::scoped_lock(claimer_mutex);
-        iface->poke32(SR_ADDR(X300_FW_SHMEM_BASE, X300_FW_SHMEM_CLAIM_TIME), uint32_t(time(NULL)));
-        iface->poke32(SR_ADDR(X300_FW_SHMEM_BASE, X300_FW_SHMEM_CLAIM_SRC), get_process_hash());
-    }
+    claim(iface);
     boost::this_thread::sleep(boost::posix_time::milliseconds(1000)); //1 second
 }
 
-bool x300_impl::is_claimed(wb_iface::sptr iface)
+x300_impl::claim_status_t x300_impl::claim_status(wb_iface::sptr iface)
 {
-    boost::mutex::scoped_lock(claimer_mutex);
-
-    //If timed out then device is definitely unclaimed
-    if (iface->peek32(SR_ADDR(X300_FW_SHMEM_BASE, X300_FW_SHMEM_CLAIM_STATUS)) == 0)
-        return false;
+    //If timed out, then device is definitely unclaimed
+    if (iface->peek32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_CLAIM_STATUS)) == 0)
+        return UNCLAIMED;
 
     //otherwise check claim src to determine if another thread with the same src has claimed the device
-    return iface->peek32(SR_ADDR(X300_FW_SHMEM_BASE, X300_FW_SHMEM_CLAIM_SRC)) != get_process_hash();
+    uint32_t hash = iface->peek32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_CLAIM_SRC));
+    return (hash == get_process_hash() ? CLAIMED_BY_US : CLAIMED_BY_OTHER);
+}
+
+void x300_impl::claim(wb_iface::sptr iface)
+{
+    iface->poke32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_CLAIM_TIME), uint32_t(time(NULL)));
+    iface->poke32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_CLAIM_SRC), get_process_hash());
+}
+
+bool x300_impl::try_to_claim(wb_iface::sptr iface, long timeout)
+{
+    boost::system_time start_time = boost::get_system_time();
+    while (1)
+    {
+        claim_status_t status = claim_status(iface);
+        if (status == UNCLAIMED)
+        {
+            claim(iface);
+            // It takes the claimer 10ms to update status, so wait 20ms before verifying claim
+            boost::this_thread::sleep(boost::posix_time::milliseconds(20));
+            continue;
+        }
+        if (status == CLAIMED_BY_US)
+        {
+            break;
+        }
+        if (boost::get_system_time() - start_time > boost::posix_time::milliseconds(timeout))
+        {
+            // Another process owns the device - give up
+            return false;
+        }
+        boost::this_thread::sleep(boost::posix_time::milliseconds(100));
+    }
+    return true;
+}
+
+void x300_impl::release(wb_iface::sptr iface)
+{
+    iface->poke32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_CLAIM_TIME), 0);
+    iface->poke32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_CLAIM_SRC), 0);
 }
 
 /***********************************************************************
diff --git a/host/lib/usrp/x300/x300_impl.hpp b/host/lib/usrp/x300/x300_impl.hpp
index e292e2d68..81d8cd3ba 100644
--- a/host/lib/usrp/x300/x300_impl.hpp
+++ b/host/lib/usrp/x300/x300_impl.hpp
@@ -129,9 +129,12 @@ public:
     void setup_mb(const size_t which, const uhd::device_addr_t &);
     ~x300_impl(void);
 
-    // used by x300_find_with_addr to find X300 devices.
-    static boost::mutex claimer_mutex;  //All claims and checks in this process are serialized
-    static bool is_claimed(uhd::wb_iface::sptr);
+    // device claim functions
+    enum claim_status_t {UNCLAIMED, CLAIMED_BY_US, CLAIMED_BY_OTHER};
+    static claim_status_t claim_status(uhd::wb_iface::sptr iface);
+    static void claim(uhd::wb_iface::sptr iface);
+    static bool try_to_claim(uhd::wb_iface::sptr iface, long timeout = 2000);
+    static void release(uhd::wb_iface::sptr iface);
 
     enum x300_mboard_t {
         USRP_X300_MB, USRP_X310_MB, UNKNOWN
diff --git a/host/lib/usrp/x300/x300_mb_eeprom.cpp b/host/lib/usrp/x300/x300_mb_eeprom.cpp
new file mode 100644
index 000000000..e39b36af8
--- /dev/null
+++ b/host/lib/usrp/x300/x300_mb_eeprom.cpp
@@ -0,0 +1,190 @@
+//
+// Copyright 2013-2016 Ettus Research LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+//
+
+/*
+ * x300_mb_eeprom_iface
+ * This interface was created to prevent MB EEPROM corruption while reading
+ * data during discovery.  For devices with firmware version newer than 5.0,
+ * the EEPROM data is read into firmware memory and available without
+ * claiming the device.  For devices with firmware versions 5.0 and older,
+ * the code makes sure to claim the device before driving the I2C bus.  This
+ * has the unfortunate side effect of preventing multiple processes from
+ * discovering the device simultaneously, but is far better than having EEPROM
+ * corruption.
+ */
+
+#include "x300_mb_eeprom.hpp"
+#include "x300_fw_common.h"
+#include "x300_regs.hpp"
+#include "x300_impl.hpp"
+#include <uhd/exception.hpp>
+#include <uhd/utils/platform.hpp>
+#include <uhd/utils/msg.hpp>
+#include <uhd/utils/byteswap.hpp>
+#include <boost/thread.hpp>
+
+using namespace uhd;
+
+static const uint32_t X300_FW_SHMEM_IDENT_MIN_VERSION = 0x50001;
+
+class x300_mb_eeprom_iface_impl : public x300_mb_eeprom_iface
+{
+public:
+
+    x300_mb_eeprom_iface_impl(wb_iface::sptr wb, i2c_iface::sptr i2c) : _wb(wb), _i2c(i2c)
+    {
+        _compat_num = _wb->peek32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_COMPAT_NUM));
+    }
+
+    ~x300_mb_eeprom_iface_impl()
+    {
+        /* NOP */
+    }
+
+    /*!
+     * Write bytes over the i2c.
+     * \param addr the address
+     * \param buf the vector of bytes
+     */
+    void write_i2c(
+        uint16_t addr,
+        const byte_vector_t &buf
+    )
+    {
+        UHD_ASSERT_THROW(addr == MBOARD_EEPROM_ADDR);
+        if (x300_impl::claim_status(_wb) != x300_impl::CLAIMED_BY_US)
+        {
+            throw uhd::io_error("Attempted to write MB EEPROM without claim to device.");
+        }
+        _i2c->write_i2c(addr, buf);
+    }
+
+    /*!
+     * Read bytes over the i2c.
+     * \param addr the address
+     * \param num_bytes number of bytes to read
+     * \return a vector of bytes
+     */
+    byte_vector_t read_i2c(
+        uint16_t addr,
+        size_t num_bytes
+    )
+    {
+        UHD_ASSERT_THROW(addr == MBOARD_EEPROM_ADDR);
+        byte_vector_t bytes;
+        if (_compat_num > X300_FW_SHMEM_IDENT_MIN_VERSION)
+        {
+            bytes = read_eeprom(addr, 0, num_bytes);
+        } else {
+            x300_impl::claim_status_t status = x300_impl::claim_status(_wb);
+            // Claim device before driving the I2C bus
+            if (status == x300_impl::CLAIMED_BY_US or x300_impl::try_to_claim(_wb))
+            {
+                bytes = _i2c->read_i2c(addr, num_bytes);
+                if (status != x300_impl::CLAIMED_BY_US)
+                {
+                    // We didn't originally have the claim, so give it up
+                    x300_impl::release(_wb);
+                }
+            }
+        }
+        return bytes;
+    }
+
+    /*!
+     * Write bytes to an eeprom.
+     * \param addr the address
+     * \param offset byte offset
+     * \param buf the vector of bytes
+     */
+    void write_eeprom(
+        uint16_t addr,
+        uint16_t offset,
+        const byte_vector_t &buf
+    )
+    {
+        UHD_ASSERT_THROW(addr == MBOARD_EEPROM_ADDR);
+        if (x300_impl::claim_status(_wb) != x300_impl::CLAIMED_BY_US)
+        {
+            throw uhd::io_error("Attempted to write MB EEPROM without claim to device.");
+        }
+        _i2c->write_eeprom(addr, offset, buf);
+    }
+
+    /*!
+     * Read bytes from an eeprom.
+     * \param addr the address
+     * \param offset byte offset
+     * \param num_bytes number of bytes to read
+     * \return a vector of bytes
+     */
+    byte_vector_t read_eeprom(
+        uint16_t addr,
+        uint16_t offset,
+        size_t num_bytes
+    )
+    {
+        UHD_ASSERT_THROW(addr == MBOARD_EEPROM_ADDR);
+        byte_vector_t bytes;
+        x300_impl::claim_status_t status = x300_impl::claim_status(_wb);
+        if (_compat_num >= X300_FW_SHMEM_IDENT_MIN_VERSION)
+        {
+            // Get MB EEPROM data from firmware memory
+            if (num_bytes == 0) return bytes;
+
+            size_t bytes_read = 0;
+            for (size_t word = offset / 4; bytes_read < num_bytes; word++)
+            {
+                uint32_t value = byteswap(_wb->peek32(X300_FW_SHMEM_ADDR(X300_FW_SHMEM_IDENT + word)));
+                for (size_t byte = offset % 4; byte < 4 and bytes_read < num_bytes; byte++)
+                {
+                    bytes.push_back(uint8_t((value >> (byte * 8)) & 0xff));
+                    bytes_read++;
+                }
+            }
+        } else {
+            // Claim device before driving the I2C bus
+            if (status == x300_impl::CLAIMED_BY_US or x300_impl::try_to_claim(_wb))
+            {
+                bytes = _i2c->read_eeprom(addr, offset, num_bytes);
+                if (status != x300_impl::CLAIMED_BY_US)
+                {
+                    // We didn't originally have the claim, so give it up
+                    x300_impl::release(_wb);
+                }
+            }
+        }
+        return bytes;
+    }
+
+
+private:
+    wb_iface::sptr _wb;
+    i2c_iface::sptr _i2c;
+    uint32_t _compat_num;
+};
+
+x300_mb_eeprom_iface::~x300_mb_eeprom_iface(void)
+{
+    /* NOP */
+}
+
+x300_mb_eeprom_iface::sptr x300_mb_eeprom_iface::make(wb_iface::sptr wb, i2c_iface::sptr i2c)
+{
+    return boost::make_shared<x300_mb_eeprom_iface_impl>(wb, i2c->eeprom16());
+}
+
diff --git a/host/lib/usrp/x300/x300_mb_eeprom.hpp b/host/lib/usrp/x300/x300_mb_eeprom.hpp
new file mode 100644
index 000000000..0649855c6
--- /dev/null
+++ b/host/lib/usrp/x300/x300_mb_eeprom.hpp
@@ -0,0 +1,37 @@
+//
+// Copyright 2016 Ettus Research LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+//
+
+#ifndef INCLUDED_X300_MB_EEPROM_HPP
+#define INCLUDED_X300_MB_EEPROM_HPP
+
+#include <uhd/config.hpp>
+#include <uhd/types/serial.hpp>
+#include <boost/utility.hpp>
+#include <boost/shared_ptr.hpp>
+#include <uhd/types/wb_iface.hpp>
+
+class x300_mb_eeprom_iface : public uhd::i2c_iface
+{
+public:
+    typedef boost::shared_ptr<x300_mb_eeprom_iface> sptr;
+
+    virtual ~x300_mb_eeprom_iface(void) = 0;
+
+    static sptr make(uhd::wb_iface::sptr wb, uhd::i2c_iface::sptr i2c);
+};
+
+#endif /* INCLUDED_X300_MB_EEPROM_HPP */
diff --git a/host/lib/usrp/x300/x300_regs.hpp b/host/lib/usrp/x300/x300_regs.hpp
index 6e7c47a64..d908bfb34 100644
--- a/host/lib/usrp/x300/x300_regs.hpp
+++ b/host/lib/usrp/x300/x300_regs.hpp
@@ -33,6 +33,9 @@ static const int BL_DATA       = 1;
 #define I2C1_BASE     0xff00
 #define SR_ADDR(base, offset) ((base) + (offset)*4)
 
+//I2C1 device addresses
+#define MBOARD_EEPROM_ADDR  0x50
+
 static const int ZPU_SR_LEDS       = 00;
 static const int ZPU_SR_SW_RST     = 01;
 static const int ZPU_SR_CLOCK_CTRL = 02;