x300: Fix LED configuration for TwinRX

The problem was that TwinRX has a special LED configuration (unlike most
other daughterboards): Since it has two channels, it is possible to
stream RX from both SMA ports. In that case, we would light up both LEDs
in green (which was not happening, only one LED would light up,
depending on which antenna was set last).

This fixes the problem and turns on both LEDs when both channels are
used, and both SMA ports are selected.

Note that the reason for this issue was an incorrect porting of this
code from UHD 3. There, we had separate LED ATR objects per channel.

1 files changed, 53 insertions, 13 deletions

diff --git a/host/lib/usrp/x300/x300_radio_control.cpp b/host/lib/usrp/x300/x300_radio_control.cpp
index 3bbadf7d6..6a851bc8f 100644
--- a/host/lib/usrp/x300/x300_radio_control.cpp
+++ b/host/lib/usrp/x300/x300_radio_control.cpp
@@ -94,6 +94,14 @@ static constexpr uint32_t SR_LEDS      = PERIPH_BASE + 176 * PERIPH_REG_OFFSET;
 static constexpr uint32_t SR_FP_GPIO   = PERIPH_BASE + 184 * PERIPH_REG_OFFSET;
 static constexpr uint32_t SR_DB_GPIO   = PERIPH_BASE + 192 * PERIPH_REG_OFFSET;
 
+// LED bit positions
+// Green LED on TX/RX port (left SMA)
+static constexpr int SR_LED_TXRX_RX = (1 << 0);
+// Red LED on TX/RX port (left SMA)
+static constexpr int SR_LED_TXRX_TX = (1 << 1);
+// Green LED on RX2 port (right SMA)
+static constexpr int SR_LED_RX2_RX = (1 << 2);
+
 static constexpr uint32_t RB_MISC_IO = PERIPH_BASE + 16 * PERIPH_REG_OFFSET;
 static constexpr uint32_t RB_SPI     = PERIPH_BASE + 17 * PERIPH_REG_OFFSET;
 // static constexpr uint32_t RB_LEDS    = PERIPH_BASE + 18 * PERIPH_REG_OFFSET;
@@ -178,6 +186,17 @@ public:
                 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);
+        // Set LEDs to default setting: RX2 LED on RX, TX/RX red LED on TX. The
+        // actual setting depends on the antenna choice and is handled in
+        // _update_atr_leds().
+        _leds->set_atr_reg(gpio_atr::ATR_REG_IDLE, 0);
+        _leds->set_atr_reg(gpio_atr::ATR_REG_RX_ONLY, x300_regs::SR_LED_RX2_RX);
+        _leds->set_atr_reg(gpio_atr::ATR_REG_TX_ONLY, x300_regs::SR_LED_TXRX_TX);
+        // We choose to light both LEDs on full duplex, regardless of the
+        // antenna selection, because the single multi-color LED on the TX/RX
+        // side does not provide useful visual feedback by itself.
+        _leds->set_atr_reg(gpio_atr::ATR_REG_FULL_DUPLEX,
+            x300_regs::SR_LED_RX2_RX | x300_regs::SR_LED_TXRX_TX);
         // We always want to initialize at least one frontend core for both TX and RX
         // RX periphs
         for (size_t i = 0; i < std::max<size_t>(get_num_output_ports(), 1); i++) {
@@ -1768,21 +1787,36 @@ private:
         _db_eeproms[addr] = db_eeprom;
     }
 
+    // A note on updating the LED ATR register: There is a single ATR register
+    // for the radio block, despite there being 2 channels. For most (1-channel)
+    // daughterboards, the rules are simple: When transmitting, the red LED turns
+    // on. When receiving, either the green LED under the RX2 or on the TX/RX
+    // port turn, depending on if the user has selected a TX/RX antenna or not.
+    // For TwinRX, we have additional rules. The board has two channels, but
+    // either of them can used with any SMA port. We therefore have to check
+    // which channels are active (0, 1, or both) and on the active channels,
+    // which set of antenna ports is used (RX1 aka TX/RX, RX2). All active RX
+    // ports shall then be added the the RX ATR register.
     void _update_atr_leds(const std::string& rx_ant, const size_t /*chan*/)
     {
-        // The "RX1" port is used by TwinRX and the "TX/RX" port is used by all
-        // other full-duplex dboards. We need to handle both here.
-        const bool is_txrx = (rx_ant == "TX/RX" or rx_ant == "RX1");
-        // Green LED on TX/RX port (left SMA)
-        constexpr int TXRX_RX = (1 << 0);
-        // Red LED on TX/RX port (left SMA)
-        constexpr int TXRX_TX = (1 << 1);
-        // Green LED on RX2 port (right SMA)
-        constexpr int RX2_RX = (1 << 2);
-        _leds->set_atr_reg(gpio_atr::ATR_REG_IDLE, 0);
-        _leds->set_atr_reg(gpio_atr::ATR_REG_RX_ONLY, is_txrx ? TXRX_RX : RX2_RX);
-        _leds->set_atr_reg(gpio_atr::ATR_REG_TX_ONLY, TXRX_TX);
-        _leds->set_atr_reg(gpio_atr::ATR_REG_FULL_DUPLEX, RX2_RX | TXRX_TX);
+        uint32_t rx_led_atr_state = 0;
+        if (_twinrx) {
+            for (size_t chan = 0; chan < get_num_output_ports(); chan++) {
+                const auto fe_enable_path = get_db_path("rx", chan) / "enabled";
+                if (get_tree()->access<bool>(fe_enable_path).get()) {
+                    if (get_rx_antenna(chan) == "RX1") {
+                        rx_led_atr_state |= x300_regs::SR_LED_TXRX_RX;
+                    }
+                    if (get_rx_antenna(chan) == "RX2") {
+                        rx_led_atr_state |= x300_regs::SR_LED_RX2_RX;
+                    }
+                }
+            }
+        } else {
+            rx_led_atr_state = rx_ant == "TX/RX" ? x300_regs::SR_LED_TXRX_RX
+                                                 : x300_regs::SR_LED_RX2_RX;
+        }
+        _leds->set_atr_reg(gpio_atr::ATR_REG_RX_ONLY, rx_led_atr_state);
     }
 
     void _set_rx_fe(const std::string& fe, const size_t chan)
@@ -1888,6 +1922,12 @@ private:
                     "Enabling RX chan " << chan << ": " << (chan_active ? "Yes" : "No"));
                 get_tree()->access<bool>(fe_enable_path).set(chan_active);
             }
+            // Modifying the number of active channels can affect how the
+            // front-panel LEDs get configured for TwinRX boards. Worst case,
+            // this is a no-op since we call it with the same argument as it was
+            // called before. Note this must be called after the 'enable'
+            // property is set.
+            _update_atr_leds(get_rx_antenna(chan), chan);
         }
 
         return true;