host: gpio: Create gpio_atr_offsets to store GPIO registers

Refactors register addresses into a gpio_atr_offsets structure which
contains the various register addresses. This allows creating other
devices with different GPIO register layouts with greater ease, and
eliminates the use of macros (yay!)

7 files changed, 136 insertions, 94 deletions

diff --git a/host/lib/include/uhdlib/usrp/cores/gpio_atr_3000.hpp b/host/lib/include/uhdlib/usrp/cores/gpio_atr_3000.hpp
index aa266cbcb..85f22d967 100644
--- a/host/lib/include/uhdlib/usrp/cores/gpio_atr_3000.hpp
+++ b/host/lib/include/uhdlib/usrp/cores/gpio_atr_3000.hpp
@@ -17,38 +17,66 @@
 
 namespace uhd { namespace usrp { namespace gpio_atr {
 
-class gpio_atr_3000 : uhd::noncopyable
+struct gpio_atr_offsets
 {
-public:
-    typedef std::shared_ptr<gpio_atr_3000> sptr;
-
-    static const uint32_t MASK_SET_ALL = 0xFFFFFFFF;
+    uhd::wb_iface::wb_addr_type idle;
+    uhd::wb_iface::wb_addr_type rx;
+    uhd::wb_iface::wb_addr_type tx;
+    uhd::wb_iface::wb_addr_type duplex;
+    uhd::wb_iface::wb_addr_type ddr;
+    uhd::wb_iface::wb_addr_type disable;
+    uhd::wb_iface::wb_addr_type readback;
 
-    virtual ~gpio_atr_3000(void) {}
+    /*!
+     * Returns whether this GPIO regmap is write-only.
+     *
+     * \return whether the readback register is valid
+     */
+    bool is_writeonly() const;
 
     /*!
-     * Create a read-write GPIO ATR interface object
+     * Create a GPIO regmap according to the typical "defaults": Four
+     * sequential ATR registers followed immediately by, in order, duplex,
+     * direction, disable, and an explicitly specified readback address.
      *
-     * \param iface register iface to GPIO ATR registers
      * \param base base settings offset for GPIO ATR registers
      * \param rb_addr readback offset for GPIO ATR registers
-     * \param reg_offset Delta between the register addresses
+     * \param stride Delta between the register addresses
      */
-    static sptr make(uhd::wb_iface::sptr iface,
+    static gpio_atr_offsets make_default(
         const uhd::wb_iface::wb_addr_type base,
         const uhd::wb_iface::wb_addr_type rb_addr,
-        const size_t reg_offset = 4);
+        const size_t stride = 4);
 
     /*!
-     * Create a write-only GPIO ATR interface object
+     * Create a GPIO regmap according to the typical "defaults" (see
+     * make_default), with the readback register disabled.
      *
-     * \param iface register iface to GPIO ATR registers
      * \param base base settings offset for GPIO ATR registers
-     * \param reg_offset Delta between the register addresses
+     * \param rb_addr readback offset for GPIO ATR registers
+     * \param stride Delta between the register addresses
      */
-    static sptr make_write_only(uhd::wb_iface::sptr iface,
+    static gpio_atr_offsets make_write_only(
         const uhd::wb_iface::wb_addr_type base,
-        const size_t reg_offset = 4);
+        const size_t stride = 4);
+};
+
+class gpio_atr_3000 : uhd::noncopyable
+{
+public:
+    typedef std::shared_ptr<gpio_atr_3000> sptr;
+
+    static const uint32_t MASK_SET_ALL = 0xFFFFFFFF;
+
+    virtual ~gpio_atr_3000(void) {}
+
+    /*!
+     * Create a GPIO ATR interface object using the given registers
+     *
+     * \param iface register iface to GPIO ATR registers
+     * \param registers Register offsets
+     */
+    static sptr make(uhd::wb_iface::sptr iface, gpio_atr_offsets registers);
 
     /*!
      * Select the ATR mode for all bits in the mask
@@ -124,17 +152,12 @@ public:
     virtual ~db_gpio_atr_3000(void) {}
 
     /*!
-     * Create a read-write GPIO ATR interface object for a daughterboard connector
+     * Create a GPIO ATR interface object for a daughterboard connector
      *
      * \param iface register iface to GPIO ATR registers
-     * \param base base settings offset for GPIO ATR registers
-     * \param rb_addr readback offset for GPIO ATR registers
-     * \param reg_offset Delta between the register addresses
+     * \param registers Register offsets
      */
-    static sptr make(uhd::wb_iface::sptr iface,
-        const uhd::wb_iface::wb_addr_type base,
-        const uhd::wb_iface::wb_addr_type rb_addr,
-        const size_t reg_offset = 4);
+    static sptr make(uhd::wb_iface::sptr iface, gpio_atr_offsets registers);
 
     /*!
      * Configure the GPIO mode for all pins in the daughterboard connector
diff --git a/host/lib/usrp/b200/b200_impl.cpp b/host/lib/usrp/b200/b200_impl.cpp
index 2e2ed5a98..ed685d536 100644
--- a/host/lib/usrp/b200/b200_impl.cpp
+++ b/host/lib/usrp/b200/b200_impl.cpp
@@ -749,7 +749,8 @@ b200_impl::b200_impl(
     // front panel gpio
     ////////////////////////////////////////////////////////////////////
     _radio_perifs[0].fp_gpio =
-        gpio_atr_3000::make(_radio_perifs[0].ctrl, TOREG(SR_FP_GPIO), RB32_FP_GPIO);
+        gpio_atr_3000::make(_radio_perifs[0].ctrl,
+            gpio_atr_offsets::make_default(TOREG(SR_FP_GPIO), RB32_FP_GPIO));
     for (const auto& attr : gpio_attr_map) {
         switch (attr.first) {
             case usrp::gpio_atr::GPIO_SRC:
@@ -883,7 +884,7 @@ void b200_impl::setup_radio(const size_t dspno)
     ////////////////////////////////////////////////////////////////////
     // Set up peripherals
     ////////////////////////////////////////////////////////////////////
-    perif.atr = gpio_atr_3000::make_write_only(perif.ctrl, TOREG(SR_ATR));
+    perif.atr = gpio_atr_3000::make(perif.ctrl, gpio_atr_offsets::make_write_only(TOREG(SR_ATR)));
     perif.atr->set_atr_mode(MODE_ATR, 0xFFFFFFFF);
     // create rx dsp control objects
     perif.framer = rx_vita_core_3000::make(perif.ctrl, TOREG(SR_RX_CTRL));
diff --git a/host/lib/usrp/cores/gpio_atr_3000.cpp b/host/lib/usrp/cores/gpio_atr_3000.cpp
index 0c7eeebda..e4c8c5154 100644
--- a/host/lib/usrp/cores/gpio_atr_3000.cpp
+++ b/host/lib/usrp/cores/gpio_atr_3000.cpp
@@ -17,13 +17,6 @@ using namespace usrp;
 // gpio_atr_3000
 //-------------------------------------------------------------
 
-#define REG_ATR_IDLE_OFFSET (base + reg_offset * 0)
-#define REG_ATR_RX_OFFSET (base + reg_offset * 1)
-#define REG_ATR_TX_OFFSET (base + reg_offset * 2)
-#define REG_ATR_FDX_OFFSET (base + reg_offset * 3)
-#define REG_DDR_OFFSET (base + reg_offset * 4)
-#define REG_ATR_DISABLE_OFFSET (base + reg_offset * 5)
-
 namespace {
 // Special RB addr value to indicate no readback
 // This value is invalid as a real address because it is not a multiple of 4
@@ -32,21 +25,48 @@ static constexpr wb_iface::wb_addr_type READBACK_DISABLED = 0xFFFFFFFF;
 
 namespace uhd { namespace usrp { namespace gpio_atr {
 
+bool gpio_atr_offsets::is_writeonly() const
+{
+    return readback == READBACK_DISABLED;
+}
+
+gpio_atr_offsets gpio_atr_offsets::make_default(
+    const uhd::wb_iface::wb_addr_type base,
+    const uhd::wb_iface::wb_addr_type rb_addr,
+    const size_t stride)
+{
+    gpio_atr_offsets offsets {
+        base, // Idle
+        static_cast<uhd::wb_iface::wb_addr_type>(base + stride), // RX
+        static_cast<uhd::wb_iface::wb_addr_type>(base + stride * 2), // TX
+        static_cast<uhd::wb_iface::wb_addr_type>(base + stride * 3), // Full Duplex
+        static_cast<uhd::wb_iface::wb_addr_type>(base + stride * 4), // DDR
+        static_cast<uhd::wb_iface::wb_addr_type>(base + stride * 5), // Disabled
+        rb_addr, // Readback
+    };
+    return offsets;
+}
+
+gpio_atr_offsets gpio_atr_offsets::make_write_only(
+    const uhd::wb_iface::wb_addr_type base,
+    const size_t stride)
+{
+    return make_default(base, READBACK_DISABLED, stride);
+}
+
 class gpio_atr_3000_impl : public gpio_atr_3000
 {
 public:
     gpio_atr_3000_impl(wb_iface::sptr iface,
-        const wb_iface::wb_addr_type base,
-        const wb_iface::wb_addr_type rb_addr,
-        const size_t reg_offset)
+        const gpio_atr_offsets registers)
         : _iface(iface)
-        , _rb_addr(rb_addr)
-        , _atr_idle_reg(REG_ATR_IDLE_OFFSET, _atr_disable_reg)
-        , _atr_rx_reg(REG_ATR_RX_OFFSET)
-        , _atr_tx_reg(REG_ATR_TX_OFFSET)
-        , _atr_fdx_reg(REG_ATR_FDX_OFFSET)
-        , _ddr_reg(REG_DDR_OFFSET)
-        , _atr_disable_reg(REG_ATR_DISABLE_OFFSET)
+        , _rb_addr(registers.readback)
+        , _atr_idle_reg(registers.idle, _atr_disable_reg)
+        , _atr_rx_reg(registers.rx)
+        , _atr_tx_reg(registers.tx)
+        , _atr_fdx_reg(registers.duplex)
+        , _ddr_reg(registers.ddr)
+        , _atr_disable_reg(registers.disable)
     {
         _atr_idle_reg.initialize(*_iface, true);
         _atr_rx_reg.initialize(*_iface, true);
@@ -311,19 +331,13 @@ protected:
     }
 };
 
-gpio_atr_3000::sptr gpio_atr_3000::make(wb_iface::sptr iface,
-    const wb_iface::wb_addr_type base,
-    const wb_iface::wb_addr_type rb_addr,
-    const size_t reg_offset)
+gpio_atr_3000::sptr gpio_atr_3000::make(wb_iface::sptr iface, gpio_atr_offsets registers)
 {
-    return sptr(new gpio_atr_3000_impl(iface, base, rb_addr, reg_offset));
-}
-
-gpio_atr_3000::sptr gpio_atr_3000::make_write_only(
-    wb_iface::sptr iface, const wb_iface::wb_addr_type base, const size_t reg_offset)
-{
-    gpio_atr_3000::sptr gpio_iface(
-        new gpio_atr_3000_impl(iface, base, READBACK_DISABLED, reg_offset));
+    gpio_atr_3000::sptr gpio_iface = std::make_shared<gpio_atr_3000_impl>(
+        iface, registers);
+    if (registers.is_writeonly()) {
+        gpio_iface->set_gpio_ddr(DDR_OUTPUT, MASK_SET_ALL);
+    }
     gpio_iface->set_gpio_ddr(DDR_OUTPUT, MASK_SET_ALL);
     return gpio_iface;
 }
@@ -335,11 +349,8 @@ gpio_atr_3000::sptr gpio_atr_3000::make_write_only(
 class db_gpio_atr_3000_impl : public gpio_atr_3000_impl, public db_gpio_atr_3000
 {
 public:
-    db_gpio_atr_3000_impl(wb_iface::sptr iface,
-        const wb_iface::wb_addr_type base,
-        const wb_iface::wb_addr_type rb_addr,
-        const size_t reg_offset)
-        : gpio_atr_3000_impl(iface, base, rb_addr, reg_offset)
+    db_gpio_atr_3000_impl(wb_iface::sptr iface, const gpio_atr_offsets registers)
+        : gpio_atr_3000_impl(iface, registers)
     { /* NOP */
     }
 
@@ -443,12 +454,9 @@ private:
     }
 };
 
-db_gpio_atr_3000::sptr db_gpio_atr_3000::make(wb_iface::sptr iface,
-    const wb_iface::wb_addr_type base,
-    const wb_iface::wb_addr_type rb_addr,
-    const size_t reg_offset)
+db_gpio_atr_3000::sptr db_gpio_atr_3000::make(wb_iface::sptr iface, gpio_atr_offsets registers)
 {
-    return sptr(new db_gpio_atr_3000_impl(iface, base, rb_addr, reg_offset));
+    return std::make_shared<db_gpio_atr_3000_impl>(iface, registers);
 }
 
 }}} // namespace uhd::usrp::gpio_atr
diff --git a/host/lib/usrp/dboard/e3xx/e3xx_radio_control_init.cpp b/host/lib/usrp/dboard/e3xx/e3xx_radio_control_init.cpp
index 995133442..95f6301f2 100644
--- a/host/lib/usrp/dboard/e3xx/e3xx_radio_control_init.cpp
+++ b/host/lib/usrp/dboard/e3xx/e3xx_radio_control_init.cpp
@@ -86,9 +86,10 @@ void e3xx_radio_control_impl::_init_peripherals()
         // Note: The register offset is baked into the different _wb_iface
         // objects!
         _db_gpio.emplace_back(
-            usrp::gpio_atr::gpio_atr_3000::make_write_only(_wb_ifaces.at(radio_idx),
-                e3xx_regs::SR_DB_GPIO + (radio_idx * e3xx_regs::PERIPH_REG_CHAN_OFFSET),
-                e3xx_regs::PERIPH_REG_OFFSET));
+            usrp::gpio_atr::gpio_atr_3000::make(_wb_ifaces.at(radio_idx),
+                usrp::gpio_atr::gpio_atr_offsets::make_write_only(
+                    e3xx_regs::SR_DB_GPIO + (radio_idx * e3xx_regs::PERIPH_REG_CHAN_OFFSET),
+                    e3xx_regs::PERIPH_REG_OFFSET)));
         _db_gpio[radio_idx]->set_atr_mode(
             usrp::gpio_atr::MODE_ATR, usrp::gpio_atr::gpio_atr_3000::MASK_SET_ALL);
     }
@@ -96,17 +97,19 @@ void e3xx_radio_control_impl::_init_peripherals()
     for (size_t radio_idx = 0; radio_idx < E3XX_NUM_CHANS; radio_idx++) {
         RFNOC_LOG_TRACE("Initializing LED GPIOs for channel " << radio_idx);
         _leds_gpio.emplace_back(
-            usrp::gpio_atr::gpio_atr_3000::make_write_only(_wb_ifaces.at(radio_idx),
-                e3xx_regs::SR_LEDS + (radio_idx * e3xx_regs::PERIPH_REG_CHAN_OFFSET),
-                e3xx_regs::PERIPH_REG_OFFSET));
+            usrp::gpio_atr::gpio_atr_3000::make(_wb_ifaces.at(radio_idx),
+                usrp::gpio_atr::gpio_atr_offsets::make_write_only(
+                    e3xx_regs::SR_LEDS + (radio_idx * e3xx_regs::PERIPH_REG_CHAN_OFFSET),
+                    e3xx_regs::PERIPH_REG_OFFSET)));
         _leds_gpio[radio_idx]->set_atr_mode(
             usrp::gpio_atr::MODE_ATR, usrp::gpio_atr::gpio_atr_3000::MASK_SET_ALL);
     }
     RFNOC_LOG_TRACE("Initializing front-panel GPIO control...")
     _fp_gpio = usrp::gpio_atr::gpio_atr_3000::make(_wb_ifaces.at(0),
-        e3xx_regs::SR_FP_GPIO,
-        e3xx_regs::RB_FP_GPIO,
-        e3xx_regs::PERIPH_REG_OFFSET);
+        usrp::gpio_atr::gpio_atr_offsets::make_default(
+            e3xx_regs::SR_FP_GPIO,
+            e3xx_regs::RB_FP_GPIO,
+            e3xx_regs::PERIPH_REG_OFFSET));
 
 
     auto block_args = get_block_args();
diff --git a/host/lib/usrp/dboard/magnesium/magnesium_radio_control_init.cpp b/host/lib/usrp/dboard/magnesium/magnesium_radio_control_init.cpp
index 98c370275..95c5dcc52 100644
--- a/host/lib/usrp/dboard/magnesium/magnesium_radio_control_init.cpp
+++ b/host/lib/usrp/dboard/magnesium/magnesium_radio_control_init.cpp
@@ -120,9 +120,10 @@ void magnesium_radio_control_impl::_init_peripherals()
         _wb_ifaces.push_back(RFNOC_MAKE_WB_IFACE(0, radio_idx));
         RFNOC_LOG_TRACE("Initializing GPIOs for channel " << radio_idx);
         _gpio.emplace_back(usrp::gpio_atr::gpio_atr_3000::make(_wb_ifaces.back(),
-            n310_regs::SR_DB_GPIO + radio_idx * n310_regs::CHAN_REG_OFFSET,
-            n310_regs::RB_DB_GPIO + radio_idx * n310_regs::CHAN_REG_OFFSET,
-            n310_regs::PERIPH_REG_OFFSET));
+            usrp::gpio_atr::gpio_atr_offsets::make_default(
+                n310_regs::SR_DB_GPIO + radio_idx * n310_regs::CHAN_REG_OFFSET,
+                n310_regs::RB_DB_GPIO + radio_idx * n310_regs::CHAN_REG_OFFSET,
+                n310_regs::PERIPH_REG_OFFSET)));
         // DSA and AD9371 gain bits do *not* toggle on ATR modes. If we ever
         // connect anything else to this core, we might need to set_atr_mode()
         // to MODE_ATR on those bits. For now, all bits simply do what they're
@@ -134,9 +135,10 @@ void magnesium_radio_control_impl::_init_peripherals()
     }
     RFNOC_LOG_TRACE("Initializing front-panel GPIO control...")
     _fp_gpio = usrp::gpio_atr::gpio_atr_3000::make(_wb_ifaces.front(),
-        n310_regs::SR_FP_GPIO,
-        n310_regs::RB_FP_GPIO,
-        n310_regs::PERIPH_REG_OFFSET);
+        usrp::gpio_atr::gpio_atr_offsets::make_default(
+            n310_regs::SR_FP_GPIO,
+            n310_regs::RB_FP_GPIO,
+            n310_regs::PERIPH_REG_OFFSET));
 }
 
 void magnesium_radio_control_impl::_init_frontend_subtree(
diff --git a/host/lib/usrp/dboard/rhodium/rhodium_radio_control_init.cpp b/host/lib/usrp/dboard/rhodium/rhodium_radio_control_init.cpp
index 844b0752b..06c1c533a 100644
--- a/host/lib/usrp/dboard/rhodium/rhodium_radio_control_init.cpp
+++ b/host/lib/usrp/dboard/rhodium/rhodium_radio_control_init.cpp
@@ -196,9 +196,10 @@ void rhodium_radio_control_impl::_init_peripherals()
     RFNOC_LOG_TRACE("Initializing GPIOs...");
     // DB GPIOs
     _gpio = usrp::gpio_atr::gpio_atr_3000::make(_wb_iface,
-        n320_regs::SR_DB_GPIO,
-        n320_regs::RB_DB_GPIO,
-        n320_regs::PERIPH_REG_OFFSET);
+        gpio_atr::gpio_atr_offsets::make_default(
+            n320_regs::SR_DB_GPIO,
+            n320_regs::RB_DB_GPIO,
+            n320_regs::PERIPH_REG_OFFSET));
     _gpio->set_atr_mode(usrp::gpio_atr::MODE_ATR, // Enable ATR mode for Rhodium bits
         RHODIUM_GPIO_MASK);
     _gpio->set_atr_mode(usrp::gpio_atr::MODE_GPIO, // Disable ATR mode for unused bits
@@ -206,9 +207,10 @@ void rhodium_radio_control_impl::_init_peripherals()
     _gpio->set_gpio_ddr(usrp::gpio_atr::DDR_OUTPUT, // Make all GPIOs outputs
         usrp::gpio_atr::gpio_atr_3000::MASK_SET_ALL);
     _fp_gpio = gpio_atr::gpio_atr_3000::make(_wb_iface,
-        n320_regs::SR_FP_GPIO,
-        n320_regs::RB_FP_GPIO,
-        n320_regs::PERIPH_REG_OFFSET);
+        gpio_atr::gpio_atr_offsets::make_default(
+            n320_regs::SR_FP_GPIO,
+            n320_regs::RB_FP_GPIO,
+            n320_regs::PERIPH_REG_OFFSET));
 
     RFNOC_LOG_TRACE("Set initial ATR values...");
     _update_atr(RHODIUM_DEFAULT_TX_ANTENNA, TX_DIRECTION);
diff --git a/host/lib/usrp/x300/x300_radio_control.cpp b/host/lib/usrp/x300/x300_radio_control.cpp
index 949650906..ab56adb07 100644
--- a/host/lib/usrp/x300/x300_radio_control.cpp
+++ b/host/lib/usrp/x300/x300_radio_control.cpp
@@ -151,9 +151,10 @@ public:
         // FP-GPIO (the gpio_atr_3000 ctor will initialize default values)
         RFNOC_LOG_TRACE("Creating FP-GPIO interface...");
         _fp_gpio = gpio_atr::gpio_atr_3000::make(_wb_iface,
-            x300_regs::SR_FP_GPIO,
-            x300_regs::RB_FP_GPIO,
-            x300_regs::PERIPH_REG_OFFSET);
+            gpio_atr::gpio_atr_offsets::make_default(
+                x300_regs::SR_FP_GPIO,
+                x300_regs::RB_FP_GPIO,
+                x300_regs::PERIPH_REG_OFFSET));
         // Create the GPIO banks and attributes, and populate them with some default
         // values
         // TODO: Do we need this section? Since the _fp_gpio handles state now, we
@@ -173,8 +174,9 @@ public:
 
         // LEDs are technically valid for both RX and TX, but let's put them
         // here
-        _leds = gpio_atr::gpio_atr_3000::make_write_only(
-            _wb_iface, x300_regs::SR_LEDS, x300_regs::PERIPH_REG_OFFSET);
+        _leds = gpio_atr::gpio_atr_3000::make(_wb_iface, 
+            gpio_atr::gpio_atr_offsets::make_write_only(
+                x300_regs::SR_LEDS, x300_regs::PERIPH_REG_OFFSET));
         _leds->set_atr_mode(
             usrp::gpio_atr::MODE_ATR, usrp::gpio_atr::gpio_atr_3000::MASK_SET_ALL);
         // We always want to initialize at least one frontend core for both TX and RX
@@ -1520,9 +1522,10 @@ private:
         // create a new dboard interface
         x300_dboard_iface_config_t db_config;
         db_config.gpio           = gpio_atr::db_gpio_atr_3000::make(_wb_iface,
-            x300_regs::SR_DB_GPIO,
-            x300_regs::RB_DB_GPIO,
-            x300_regs::PERIPH_REG_OFFSET);
+            gpio_atr::gpio_atr_offsets::make_default(
+                x300_regs::SR_DB_GPIO,
+                x300_regs::RB_DB_GPIO,
+                x300_regs::PERIPH_REG_OFFSET));
         db_config.spi            = _spi;
         db_config.rx_spi_slaveno = DB_RX_SEN;
         db_config.tx_spi_slaveno = DB_TX_SEN;