From b0f9fd9c4c573bfe584651513ca73b941ee6d8c1 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Wed, 15 Jun 2022 12:40:21 +0000
Subject: [PATCH 01/26] implement main driver functions

---
 artiq/coredevice/phaser.py | 58 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index c9bf0c479..6207715de 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -40,6 +40,16 @@ PHASER_ADDR_DUC1_P = 0x26
 PHASER_ADDR_DAC1_DATA = 0x28
 PHASER_ADDR_DAC1_TEST = 0x2c
 
+# servo registers
+PHASER_ADDR_SERVO_CFG0 = 0x30
+PHASER_ADDR_SERVO_CFG1 = 0x31
+
+# 0x32 - 0x61 ab regs
+PHASER_ADDR_SERVO_AB_BASE = 0x32
+# 0x62 - 0x71 offset regs
+PHASER_ADDR_SERVO_OFFSET_BASE = 0x62
+
+
 PHASER_SEL_DAC = 1 << 0
 PHASER_SEL_TRF0 = 1 << 1
 PHASER_SEL_TRF1 = 1 << 2
@@ -382,6 +392,14 @@ class Phaser:
         response = rtio_input_data(self.channel_base)
         return response >> self.miso_delay
 
+    @kernel
+    def write16(self, addr, data: TInt32):
+        """Write 16 bit to a sequence of FPGA registers."""
+        for offset in range(2):
+            byte = data >> 8
+            self.write8(addr + offset, byte)
+            data <<= 8
+
     @kernel
     def write32(self, addr, data: TInt32):
         """Write 32 bit to a sequence of FPGA registers."""
@@ -1039,6 +1057,46 @@ class PhaserChannel:
             data = data ^ ((1 << 12) | (1 << 13))
         self.trf_write(data)
 
+    @kernel
+    def set_servo_enable(self, en=1):
+        """Set the servo enable to True or False.
+
+        :param en: 1 to enable servo, 0 to disable
+        """
+        addr = PHASER_ADDR_SERVO_CFG1 if self.index == 1 else PHASER_ADDR_SERVO_CFG0
+        content = self.phaser.read8(addr)
+        delay(.1*ms)
+        content = (content | 1) & en
+        self.phaser.write8(addr, content)
+
+    @kernel
+    def set_servo_profile(self, profile):
+        """Set the servo profile.
+
+        :param profile: [0:3] profile index to select for channel
+        """
+        if profile not in range(4):
+            raise ValueError("invalid profile index")
+        addr = PHASER_ADDR_SERVO_CFG1 if self.index == 1 else PHASER_ADDR_SERVO_CFG0
+        content = self.phaser.read8(addr)
+        delay(.1*ms)
+        # shift one left and leave en bit
+        content = (profile << 1) | (content & 1)
+        self.phaser.write8(addr, content)
+
+    @kernel
+    def load_servo_profile(self, profile, ab, offset):
+        """Set the servo enable to True or False.
+        """
+        if profile not in range(4):
+            raise ValueError("invalid profile index")
+        # Should I check here if the profile I want to load is selected? What do I do if it is?
+        addr = PHASER_ADDR_SERVO_AB_BASE + (6 * profile) + (self.index * 24)
+        for coef in ab:
+            self.phaser.write16(addr, coef)
+            addr +=2
+        addr = PHASER_ADDR_SERVO_OFFSET_BASE + (2 * profile) + (self.index * 8)
+        self.phaser.write16(addr, offset)
 
 class PhaserOscillator:
     """Phaser IQ channel oscillator (NCO/DDS).

From 1bddadc6e24508ee191087e1b22e5826c2144091 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Wed, 15 Jun 2022 17:32:11 +0000
Subject: [PATCH 02/26] cleanup and comments

---
 artiq/coredevice/phaser.py | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 6207715de..14da24228 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1073,7 +1073,7 @@ class PhaserChannel:
     def set_servo_profile(self, profile):
         """Set the servo profile.
 
-        :param profile: [0:3] profile index to select for channel
+        :param profile: profile index to select for channel (0 to 3)
         """
         if profile not in range(4):
             raise ValueError("invalid profile index")
@@ -1086,11 +1086,17 @@ class PhaserChannel:
 
     @kernel
     def load_servo_profile(self, profile, ab, offset):
-        """Set the servo enable to True or False.
+        """Load a servo profile consiting of the three filter coefficients and an output offset.
+
+        :param profile: profile to load (0 to 3)
+        :param ab: 3 entry coefficient vector (16 bit)
+        :param offset: output offset (16 bit)
         """
         if profile not in range(4):
             raise ValueError("invalid profile index")
-        # Should I check here if the profile I want to load is selected? What do I do if it is?
+        if len(ab) != 3:
+            raise ValueError("invalid number of coefficients")
+        # Should I check here if the profile I want to load is selected? Aka read the register. What do I do if it is?
         addr = PHASER_ADDR_SERVO_AB_BASE + (6 * profile) + (self.index * 24)
         for coef in ab:
             self.phaser.write16(addr, coef)
@@ -1098,6 +1104,7 @@ class PhaserChannel:
         addr = PHASER_ADDR_SERVO_OFFSET_BASE + (2 * profile) + (self.index * 8)
         self.phaser.write16(addr, offset)
 
+
 class PhaserOscillator:
     """Phaser IQ channel oscillator (NCO/DDS).
 

From ae3f1c1c7174cce85e91ba29b486b4520a4a2600 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Fri, 17 Jun 2022 11:47:45 +0000
Subject: [PATCH 03/26] adapt servo functions. Todo: docu

---
 artiq/coredevice/phaser.py | 35 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 14da24228..0bb468e23 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1058,41 +1058,32 @@ class PhaserChannel:
         self.trf_write(data)
 
     @kernel
-    def set_servo_enable(self, en=1):
-        """Set the servo enable to True or False.
-
-        :param en: 1 to enable servo, 0 to disable
-        """
-        addr = PHASER_ADDR_SERVO_CFG1 if self.index == 1 else PHASER_ADDR_SERVO_CFG0
-        content = self.phaser.read8(addr)
-        delay(.1*ms)
-        content = (content | 1) & en
-        self.phaser.write8(addr, content)
-
-    @kernel
-    def set_servo_profile(self, profile):
-        """Set the servo profile.
+    def set_servo(self, bypass=1, hold=0, profile=0):
+        """Set the servo configuration.
 
+        :param bypass: 1 to enable bypass (default), 0 to engage servo
+        :param hold: 1 to hold the servo IIR filter output constant, 0 for normal operation
         :param profile: profile index to select for channel (0 to 3)
         """
-        if profile not in range(4):
+        if (profile < 0) | (profile > 3):
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG1 if self.index == 1 else PHASER_ADDR_SERVO_CFG0
-        content = self.phaser.read8(addr)
-        delay(.1*ms)
-        # shift one left and leave en bit
-        content = (profile << 1) | (content & 1)
-        self.phaser.write8(addr, content)
+        if bypass == 0:
+            data = 1
+        if hold == 1:
+            data = data | (1 << 1)
+        data = data | (profile << 2)
+        self.phaser.write8(addr, data)
 
     @kernel
-    def load_servo_profile(self, profile, ab, offset):
+    def set_iir_mu(self, profile, ab, offset):
         """Load a servo profile consiting of the three filter coefficients and an output offset.
 
         :param profile: profile to load (0 to 3)
         :param ab: 3 entry coefficient vector (16 bit)
         :param offset: output offset (16 bit)
         """
-        if profile not in range(4):
+        if (profile < 0) | (profile > 3):
             raise ValueError("invalid profile index")
         if len(ab) != 3:
             raise ValueError("invalid number of coefficients")

From 2044dc3ae59400b3e717d4ab261e1aaf748fe178 Mon Sep 17 00:00:00 2001
From: Norman Krackow <Norman_Krackow@gmx.de>
Date: Fri, 17 Jun 2022 14:39:37 +0200
Subject: [PATCH 04/26] Update artiq/coredevice/phaser.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Robert Jördens <rj@quartiq.de>
---
 artiq/coredevice/phaser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 0bb468e23..3f5e4262c 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1067,7 +1067,7 @@ class PhaserChannel:
         """
         if (profile < 0) | (profile > 3):
             raise ValueError("invalid profile index")
-        addr = PHASER_ADDR_SERVO_CFG1 if self.index == 1 else PHASER_ADDR_SERVO_CFG0
+        addr = PHASER_ADDR_SERVO_CFG0 + self.index
         if bypass == 0:
             data = 1
         if hold == 1:

From dc49372d57d0e1a3d6e025a230f53af49c7db422 Mon Sep 17 00:00:00 2001
From: Norman Krackow <Norman_Krackow@gmx.de>
Date: Fri, 17 Jun 2022 14:40:07 +0200
Subject: [PATCH 05/26] Update artiq/coredevice/phaser.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Robert Jördens <rj@quartiq.de>
---
 artiq/coredevice/phaser.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 3f5e4262c..e696fa227 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1072,7 +1072,9 @@ class PhaserChannel:
             data = 1
         if hold == 1:
             data = data | (1 << 1)
-        data = data | (profile << 2)
+        if bypass:
+            hold = 1
+        data = (profile << 2) | (hold << 1) | (bypass << 0)
         self.phaser.write8(addr, data)
 
     @kernel

From d09153411fd8fa38cb4d569e54cbd4a6eb8fd6e7 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Fri, 17 Jun 2022 13:03:21 +0000
Subject: [PATCH 06/26] adress some review comments

---
 artiq/coredevice/phaser.py | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index e696fa227..7d0f87315 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -395,10 +395,10 @@ class Phaser:
     @kernel
     def write16(self, addr, data: TInt32):
         """Write 16 bit to a sequence of FPGA registers."""
-        for offset in range(2):
-            byte = data >> 8
-            self.write8(addr + offset, byte)
-            data <<= 8
+        byte = data >> 8
+        self.write8(addr, byte)
+        byte = data & 0xFF
+        self.write8(addr + 1, byte)
 
     @kernel
     def write32(self, addr, data: TInt32):
@@ -1065,35 +1065,34 @@ class PhaserChannel:
         :param hold: 1 to hold the servo IIR filter output constant, 0 for normal operation
         :param profile: profile index to select for channel (0 to 3)
         """
-        if (profile < 0) | (profile > 3):
+        if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG0 + self.index
         if bypass == 0:
             data = 1
         if hold == 1:
-            data = data | (1 << 1)
+            data = data or (1 << 1)
         if bypass:
             hold = 1
-        data = (profile << 2) | (hold << 1) | (bypass << 0)
+        data = (profile << 2) or (hold << 1) or (bypass << 0)
         self.phaser.write8(addr, data)
 
     @kernel
-    def set_iir_mu(self, profile, ab, offset):
+    def set_iir_mu(self, profile, b0, b1, a1, offset):
         """Load a servo profile consiting of the three filter coefficients and an output offset.
 
         :param profile: profile to load (0 to 3)
         :param ab: 3 entry coefficient vector (16 bit)
         :param offset: output offset (16 bit)
         """
-        if (profile < 0) | (profile > 3):
+        if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
-        if len(ab) != 3:
-            raise ValueError("invalid number of coefficients")
-        # Should I check here if the profile I want to load is selected? Aka read the register. What do I do if it is?
+        # 24 byte-sized ab registers per channel and 6 (2 bytes * 3 coefficients) registers per profile
         addr = PHASER_ADDR_SERVO_AB_BASE + (6 * profile) + (self.index * 24)
-        for coef in ab:
+        for coef in [b0, b1, a1]:
             self.phaser.write16(addr, coef)
-            addr +=2
+            addr += 2
+        # 8 offset registers per channel and 2 registers per offset
         addr = PHASER_ADDR_SERVO_OFFSET_BASE + (2 * profile) + (self.index * 8)
         self.phaser.write16(addr, offset)
 

From 5df766e6da75c37fd29217e75b7c933e40f87cfc Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Tue, 21 Jun 2022 07:36:59 +0000
Subject: [PATCH 07/26] fix ors

---
 artiq/coredevice/phaser.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 7d0f87315..b620e653e 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1058,7 +1058,7 @@ class PhaserChannel:
         self.trf_write(data)
 
     @kernel
-    def set_servo(self, bypass=1, hold=0, profile=0):
+    def set_servo(self, profile=0, bypass=1, hold=0):
         """Set the servo configuration.
 
         :param bypass: 1 to enable bypass (default), 0 to engage servo
@@ -1068,13 +1068,14 @@ class PhaserChannel:
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG0 + self.index
+        data = 0
         if bypass == 0:
             data = 1
         if hold == 1:
-            data = data or (1 << 1)
+            data = data | (1 << 1)
         if bypass:
             hold = 1
-        data = (profile << 2) or (hold << 1) or (bypass << 0)
+        data = (profile << 2) | (hold << 1) | (bypass << 0)
         self.phaser.write8(addr, data)
 
     @kernel

From 751af3144e83707a2345e132ab47cf13565303bb Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Tue, 21 Jun 2022 07:43:28 +0000
Subject: [PATCH 08/26] fix old line that I forgot

---
 artiq/coredevice/phaser.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index b620e653e..c418f990d 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1073,9 +1073,7 @@ class PhaserChannel:
             data = 1
         if hold == 1:
             data = data | (1 << 1)
-        if bypass:
-            hold = 1
-        data = (profile << 2) | (hold << 1) | (bypass << 0)
+        data = data | (profile << 2)
         self.phaser.write8(addr, data)
 
     @kernel

From 0388161754b0a2f3ed4e31017b9d3ad0fe493b2c Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Tue, 21 Jun 2022 07:49:29 +0000
Subject: [PATCH 09/26] disable servo in init

---
 artiq/coredevice/phaser.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index c418f990d..417bcf32f 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -320,6 +320,9 @@ class Phaser:
             delay(.1*ms)
             channel.set_att_mu(0x00)  # minimum attenuation
 
+            # disable servo, set iir profile to 0 and disable iir hold 
+            channel.set_servo(0, 1, 0)
+
             # test oscillators and DUC
             for i in range(len(channel.oscillator)):
                 oscillator = channel.oscillator[i]

From 8bea821f9346d5d66a090ce7e30b76e152750d58 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Tue, 21 Jun 2022 08:43:55 +0000
Subject: [PATCH 10/26] just &1 to stay in field

---
 artiq/coredevice/phaser.py | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 417bcf32f..9d1d2a63c 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1071,12 +1071,7 @@ class PhaserChannel:
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG0 + self.index
-        data = 0
-        if bypass == 0:
-            data = 1
-        if hold == 1:
-            data = data | (1 << 1)
-        data = data | (profile << 2)
+        data = (profile << 2) | ((hold & 1) << 1) | (~bypass & 1)
         self.phaser.write8(addr, data)
 
     @kernel

From 57176fedb29ae64e72b562bd6dfa58df7dfbca47 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Tue, 21 Jun 2022 09:29:42 +0000
Subject: [PATCH 11/26] add servo docu

---
 artiq/coredevice/phaser.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 9d1d2a63c..6826bef7f 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -122,6 +122,26 @@ class Phaser:
     configured through a shared SPI bus that is accessed and controlled via
     FPGA registers.
 
+    Each phaser output channel features a servo to control the RF output amplitude
+    using feedback from an ADC. The servo consists of a first order IIR (infinite
+    impulse response) filter fed by the ADC and a multiplier that scales the I
+    and Q datastreams from the DUC by the IIR output.
+
+    Each channels IIR features 4 profiles, each consisting of the [b0, b1, a1] filter
+    coefficients as well as an output offset. The coefficients and offset can be
+    set for each profile individually and the profiles each have their own filter
+    state. To avoid transient effects, care should be taken to not update the
+    coefficents in the currently selected profile.
+
+    The IIR output can be put on hold for each channel. In hold mode, the filter
+    still ingests samples and updates its input x0 and x1 registers, but does not
+    update the y0, y1 output registers. The servo can also be bypassed.
+
+    After power-up the servo is bypassed, in profile 0, with coefficients [0, 0, 0]
+    and hold is disabled. If older gateware without ther servo is loaded onto the
+    Phaser FPGA, the device simply behaves as if the servo is bypassed and none of
+    the servo functions have any effect.
+
     .. note:: Various register settings of the DAC and the quadrature
         upconverters are available to be modified through the `dac`, `trf0`,
         `trf1` dictionaries. These can be set through the device database

From b67a70392d8e60d87bb663a0b5f07c6acf712b20 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Tue, 21 Jun 2022 09:59:40 +0000
Subject: [PATCH 12/26] rename to coeff base and shorter write16

---
 artiq/coredevice/phaser.py | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 6826bef7f..99f3f098c 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -45,7 +45,7 @@ PHASER_ADDR_SERVO_CFG0 = 0x30
 PHASER_ADDR_SERVO_CFG1 = 0x31
 
 # 0x32 - 0x61 ab regs
-PHASER_ADDR_SERVO_AB_BASE = 0x32
+PHASER_ADDR_SERVO_COEFFICIENTS_BASE = 0x32
 # 0x62 - 0x71 offset regs
 PHASER_ADDR_SERVO_OFFSET_BASE = 0x62
 
@@ -418,10 +418,8 @@ class Phaser:
     @kernel
     def write16(self, addr, data: TInt32):
         """Write 16 bit to a sequence of FPGA registers."""
-        byte = data >> 8
-        self.write8(addr, byte)
-        byte = data & 0xFF
-        self.write8(addr + 1, byte)
+        self.write8(addr, data >> 8)
+        self.write8(addr + 1, data)
 
     @kernel
     def write32(self, addr, data: TInt32):

From ce4055db3beae3c0ec9b7629179ecdb9e14a429c Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Tue, 21 Jun 2022 10:11:49 +0000
Subject: [PATCH 13/26] force hold on bypass and use names in set_servo() in
 init

---
 artiq/coredevice/phaser.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 99f3f098c..e4c5197a9 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -138,7 +138,7 @@ class Phaser:
     update the y0, y1 output registers. The servo can also be bypassed.
 
     After power-up the servo is bypassed, in profile 0, with coefficients [0, 0, 0]
-    and hold is disabled. If older gateware without ther servo is loaded onto the
+    and hold is enabled. If older gateware without ther servo is loaded onto the
     Phaser FPGA, the device simply behaves as if the servo is bypassed and none of
     the servo functions have any effect.
 
@@ -340,8 +340,7 @@ class Phaser:
             delay(.1*ms)
             channel.set_att_mu(0x00)  # minimum attenuation
 
-            # disable servo, set iir profile to 0 and disable iir hold 
-            channel.set_servo(0, 1, 0)
+            channel.set_servo(profile=0, bypass=1, hold=1)
 
             # test oscillators and DUC
             for i in range(len(channel.oscillator)):
@@ -1082,14 +1081,16 @@ class PhaserChannel:
     def set_servo(self, profile=0, bypass=1, hold=0):
         """Set the servo configuration.
 
-        :param bypass: 1 to enable bypass (default), 0 to engage servo
+        :param bypass: 1 to enable bypass (default), 0 to engage servo. If bypassed, hold
+            is forced since the control loop is broken.
         :param hold: 1 to hold the servo IIR filter output constant, 0 for normal operation
         :param profile: profile index to select for channel (0 to 3)
         """
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG0 + self.index
-        data = (profile << 2) | ((hold & 1) << 1) | (~bypass & 1)
+        # enforce hold if the servo is bypassed
+        data = (profile << 2) | (((hold | bypass) & 1) << 1) | (~bypass & 1)
         self.phaser.write8(addr, data)
 
     @kernel

From 43c94577ce5bf653cf58905929d611f2b32cab54 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Wed, 22 Jun 2022 15:35:49 +0000
Subject: [PATCH 14/26] impl set_iir. untested

---
 artiq/coredevice/phaser.py | 44 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index e4c5197a9..1e93db118 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -68,6 +68,10 @@ PHASER_DAC_SEL_TEST = 1
 
 PHASER_HW_REV_VARIANT = 1 << 4
 
+SERVO_COEFF_WIDTH = 16
+SERVO_COEFF_SHIFT = 14
+SERVO_T_CYCLE = (32+12+192+24+4)*ns  # Must match gateware ADC parameters
+
 
 class Phaser:
     """Phaser 4-channel, 16-bit, 1 GS/s DAC coredevice driver.
@@ -127,7 +131,7 @@ class Phaser:
     impulse response) filter fed by the ADC and a multiplier that scales the I
     and Q datastreams from the DUC by the IIR output.
 
-    Each channels IIR features 4 profiles, each consisting of the [b0, b1, a1] filter
+    Each channel IIR features 4 profiles, each consisting of the [b0, b1, a1] filter
     coefficients as well as an output offset. The coefficients and offset can be
     set for each profile individually and the profiles each have their own filter
     state. To avoid transient effects, care should be taken to not update the
@@ -1104,13 +1108,49 @@ class PhaserChannel:
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
         # 24 byte-sized ab registers per channel and 6 (2 bytes * 3 coefficients) registers per profile
-        addr = PHASER_ADDR_SERVO_AB_BASE + (6 * profile) + (self.index * 24)
+        addr = PHASER_ADDR_SERVO_COEFFICIENTS_BASE + (6 * profile) + (self.index * 24)
         for coef in [b0, b1, a1]:
             self.phaser.write16(addr, coef)
             addr += 2
         # 8 offset registers per channel and 2 registers per offset
         addr = PHASER_ADDR_SERVO_OFFSET_BASE + (2 * profile) + (self.index * 8)
         self.phaser.write16(addr, offset)
+    
+    @kernel
+    def set_iir(self, profile, kp, ki=0., g=0., x_offset=0., y_offset=0.):
+
+        NORM = 1 << SERVO_COEFF_SHIFT
+        COEFF_MAX = 1 << SERVO_COEFF_WIDTH - 1
+
+        kp *= NORM
+        if ki == 0.:
+            # pure P
+            a1 = 0
+            b1 = 0
+            b0 = int(round(kp))
+        else:
+            # I or PI
+            ki *= NORM*SERVO_T_CYCLE/2.
+            if g == 0.:
+                c = 1.
+                a1 = NORM
+            else:
+                c = 1./(1. + ki/(g*NORM))
+                a1 = int(round((2.*c - 1.)*NORM))
+            b0 = int(round(kp + ki*c))
+            b1 = int(round(kp + (ki - 2.*kp)*c))
+            if b1 == -b0:
+                raise ValueError("low integrator gain and/or gain limit")
+
+        if (b0 >= COEFF_MAX or b0 < -COEFF_MAX or
+                b1 >= COEFF_MAX or b1 < -COEFF_MAX):
+            raise ValueError("high gains")
+
+        forward_gain = b0 + b1
+        effective_offset = y_offset + forward_gain * x_offset
+
+        self.set_iir_mu(profile, b0, b1, a1, effective_offset)
+
 
 
 class PhaserOscillator:

From c0581178d60ffb9cd2882ed9ef2d085692335ddf Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Wed, 22 Jun 2022 16:20:59 +0000
Subject: [PATCH 15/26] impl offsets. to be tested

---
 artiq/coredevice/phaser.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 1e93db118..c23fe48e6 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -69,6 +69,7 @@ PHASER_DAC_SEL_TEST = 1
 PHASER_HW_REV_VARIANT = 1 << 4
 
 SERVO_COEFF_WIDTH = 16
+SERVO_DATA_WIDTH = 16
 SERVO_COEFF_SHIFT = 14
 SERVO_T_CYCLE = (32+12+192+24+4)*ns  # Must match gateware ADC parameters
 
@@ -1121,6 +1122,7 @@ class PhaserChannel:
 
         NORM = 1 << SERVO_COEFF_SHIFT
         COEFF_MAX = 1 << SERVO_COEFF_WIDTH - 1
+        DATA_MAX = 1 << SERVO_DATA_WIDTH - 1
 
         kp *= NORM
         if ki == 0.:
@@ -1146,8 +1148,8 @@ class PhaserChannel:
                 b1 >= COEFF_MAX or b1 < -COEFF_MAX):
             raise ValueError("high gains")
 
-        forward_gain = b0 + b1
-        effective_offset = y_offset + forward_gain * x_offset
+        forward_gain = (b0 + b1) * (DATA_MAX - NORM)
+        effective_offset = int(round(DATA_MAX * y_offset + forward_gain * x_offset))
 
         self.set_iir_mu(profile, b0, b1, a1, effective_offset)
 

From 56c59e38f0f4a879be6b04748852e0ef97b5467a Mon Sep 17 00:00:00 2001
From: Norman Krackow <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 09:15:50 +0200
Subject: [PATCH 16/26] Update artiq/coredevice/phaser.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Robert Jördens <rj@quartiq.de>
---
 artiq/coredevice/phaser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index c23fe48e6..ba7a25dd9 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1148,7 +1148,7 @@ class PhaserChannel:
                 b1 >= COEFF_MAX or b1 < -COEFF_MAX):
             raise ValueError("high gains")
 
-        forward_gain = (b0 + b1) * (DATA_MAX - NORM)
+        forward_gain = (b0 + b1) * (1 << SERVO_DATA_WIDTH - 1 - SERVO_COEFF_SHIFT)
         effective_offset = int(round(DATA_MAX * y_offset + forward_gain * x_offset))
 
         self.set_iir_mu(profile, b0, b1, a1, effective_offset)

From 24b4ec46bd4b3fbad907d7dfaafbbf22226d1830 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 08:48:28 +0000
Subject: [PATCH 17/26] more documentation

---
 artiq/coredevice/phaser.py | 67 ++++++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 6 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index ba7a25dd9..2a10bf5b9 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -130,7 +130,8 @@ class Phaser:
     Each phaser output channel features a servo to control the RF output amplitude
     using feedback from an ADC. The servo consists of a first order IIR (infinite
     impulse response) filter fed by the ADC and a multiplier that scales the I
-    and Q datastreams from the DUC by the IIR output.
+    and Q datastreams from the DUC by the IIR output. The IIR state is updated at
+    the 3.788 MHz ADC sampling rate.
 
     Each channel IIR features 4 profiles, each consisting of the [b0, b1, a1] filter
     coefficients as well as an output offset. The coefficients and offset can be
@@ -1088,8 +1089,8 @@ class PhaserChannel:
 
         :param bypass: 1 to enable bypass (default), 0 to engage servo. If bypassed, hold
             is forced since the control loop is broken.
-        :param hold: 1 to hold the servo IIR filter output constant, 0 for normal operation
-        :param profile: profile index to select for channel (0 to 3)
+        :param hold: 1 to hold the servo IIR filter output constant, 0 for normal operation.
+        :param profile: Profile index to select for channel. (0 to 3)
         """
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
@@ -1102,9 +1103,32 @@ class PhaserChannel:
     def set_iir_mu(self, profile, b0, b1, a1, offset):
         """Load a servo profile consiting of the three filter coefficients and an output offset.
 
-        :param profile: profile to load (0 to 3)
-        :param ab: 3 entry coefficient vector (16 bit)
-        :param offset: output offset (16 bit)
+        Avoid setting the IIR parameters of the currently active profile.
+
+        The recurrence relation is (all data signed and MSB aligned):
+
+        .. math::
+            a_0 y_n = a_1 y_{n - 1} + b_0 x_n + b_1 x_{n - 1} + o
+
+        Where:
+
+            * :math:`y_n` and :math:`y_{n-1}` are the current and previous
+              filter outputs, clipped to :math:`[0, 1[`.
+            * :math:`x_n` and :math:`x_{n-1}` are the current and previous
+              filter inputs in :math:`[-1, 1[`.
+            * :math:`o` is the offset
+            * :math:`a_0` is the normalization factor :math:`2^{14}`
+            * :math:`a_1` is the feedback gain
+            * :math:`b_0` and :math:`b_1` are the feedforward gains for the two
+              delays
+
+        .. seealso:: :meth:`set_iir`
+
+        :param profile: Profile to set (0 to 3)
+        :param b0: b0 filter coefficient (16 bit signed)
+        :param b1: b1 filter coefficient (16 bit signed)
+        :param a1: a1 filter coefficient (16 bit signed)
+        :param offset: Output offset (16 bit signed)
         """
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
@@ -1119,7 +1143,38 @@ class PhaserChannel:
     
     @kernel
     def set_iir(self, profile, kp, ki=0., g=0., x_offset=0., y_offset=0.):
+        """Set servo profile IIR coefficients.
 
+        Avoid setting the IIR parameters of the currently active profile.
+
+        Gains are given in units of output full per scale per input full scale.
+
+        The transfer function is (up to time discretization and
+        coefficient quantization errors):
+
+        .. math::
+            H(s) = k_p + \\frac{k_i}{s + \\frac{k_i}{g}}
+
+        Where:
+            * :math:`s = \\sigma + i\\omega` is the complex frequency
+            * :math:`k_p` is the proportional gain
+            * :math:`k_i` is the integrator gain
+            * :math:`g` is the integrator gain limit
+
+        :param profile: Profile number (0-3)
+        :param kp: Proportional gain. This is usually negative (closed
+            loop, positive ADC voltage, positive setpoint). When 0, this
+            implements a pure I controller.
+        :param ki: Integrator gain (rad/s). Equivalent to the gain at 1 Hz.
+            When 0 (the default) this implements a pure P controller.
+            Same sign as ``kp``.
+        :param g: Integrator gain limit (1). When 0 (the default) the
+            integrator gain limit is infinite. Same sign as ``ki``.
+        :param x_offset: IIR input offset. Used as the negative
+            setpoint when stabilizing to a desired input setpoint. Will
+            be converted to an equivalent output offset and added to y_offset.
+        :param y_offset: IIR output offset.
+        """
         NORM = 1 << SERVO_COEFF_SHIFT
         COEFF_MAX = 1 << SERVO_COEFF_WIDTH - 1
         DATA_MAX = 1 << SERVO_DATA_WIDTH - 1

From ab097b8ef9f955b07fe23ec869dbc5e8a5585ee0 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 09:37:37 +0000
Subject: [PATCH 18/26] add offset to coefficients as data

---
 artiq/coredevice/phaser.py | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 2a10bf5b9..d7fc15f51 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -44,10 +44,8 @@ PHASER_ADDR_DAC1_TEST = 0x2c
 PHASER_ADDR_SERVO_CFG0 = 0x30
 PHASER_ADDR_SERVO_CFG1 = 0x31
 
-# 0x32 - 0x61 ab regs
-PHASER_ADDR_SERVO_COEFFICIENTS_BASE = 0x32
-# 0x62 - 0x71 offset regs
-PHASER_ADDR_SERVO_OFFSET_BASE = 0x62
+# 0x32 - 0x71 servo coefficients + offset data
+PHASER_ADDR_SERVO_DATA_BASE = 0x32
 
 
 PHASER_SEL_DAC = 1 << 0
@@ -1133,13 +1131,10 @@ class PhaserChannel:
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
         # 24 byte-sized ab registers per channel and 6 (2 bytes * 3 coefficients) registers per profile
-        addr = PHASER_ADDR_SERVO_COEFFICIENTS_BASE + (6 * profile) + (self.index * 24)
-        for coef in [b0, b1, a1]:
-            self.phaser.write16(addr, coef)
+        addr = PHASER_ADDR_SERVO_DATA_BASE + (8 * profile) + (self.index * 32)
+        for data in [b0, b1, a1, offset]:
+            self.phaser.write16(addr, data)
             addr += 2
-        # 8 offset registers per channel and 2 registers per offset
-        addr = PHASER_ADDR_SERVO_OFFSET_BASE + (2 * profile) + (self.index * 8)
-        self.phaser.write16(addr, offset)
     
     @kernel
     def set_iir(self, profile, kp, ki=0., g=0., x_offset=0., y_offset=0.):

From 3f8a221c764d7d66e81cbce5d2f21c56615fac3e Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 10:08:34 +0000
Subject: [PATCH 19/26] flip logic of enable bit to bypass bit and update some
 comments

---
 artiq/coredevice/phaser.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index d7fc15f51..76e9fec7e 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1094,7 +1094,7 @@ class PhaserChannel:
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG0 + self.index
         # enforce hold if the servo is bypassed
-        data = (profile << 2) | (((hold | bypass) & 1) << 1) | (~bypass & 1)
+        data = (profile << 2) | (((hold | bypass) & 1) << 1) | (bypass & 1)
         self.phaser.write8(addr, data)
 
     @kernel
@@ -1130,7 +1130,7 @@ class PhaserChannel:
         """
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
-        # 24 byte-sized ab registers per channel and 6 (2 bytes * 3 coefficients) registers per profile
+        # 32 byte-sized data registers per channel and 8 (2 bytes * (3 coefficients + 1 offset)) registers per profile
         addr = PHASER_ADDR_SERVO_DATA_BASE + (8 * profile) + (self.index * 32)
         for data in [b0, b1, a1, offset]:
             self.phaser.write16(addr, data)

From 2e834cf406426bf844435b653f2dd39775c75c32 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 10:20:38 +0000
Subject: [PATCH 20/26] unflip logic..

---
 artiq/coredevice/phaser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index 76e9fec7e..bb3cfd6df 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1094,7 +1094,7 @@ class PhaserChannel:
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG0 + self.index
         # enforce hold if the servo is bypassed
-        data = (profile << 2) | (((hold | bypass) & 1) << 1) | (bypass & 1)
+        data = (profile << 2) | (((hold | bypass) & 1) << 1) | (~bypass & 1)
         self.phaser.write8(addr, data)
 
     @kernel

From d8cfe22501e3e85654d20dab90bceb822de00c57 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 15:18:55 +0000
Subject: [PATCH 21/26] add note about setpoint resolution

---
 artiq/coredevice/phaser.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index bb3cfd6df..c6e614a01 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1144,6 +1144,10 @@ class PhaserChannel:
 
         Gains are given in units of output full per scale per input full scale.
 
+        .. note:: Due to inherent constraints of the fixed point datatypes and IIR
+        filters, the setpoint resolution depends on the selected gains. This is
+        especially the case for low ``ki`` gains.
+
         The transfer function is (up to time discretization and
         coefficient quantization errors):
 

From 689a2ef8ba82d92e6c68e17465efa0ff031559e8 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 15:23:00 +0000
Subject: [PATCH 22/26] refine note

---
 artiq/coredevice/phaser.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index c6e614a01..b69677500 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -1145,8 +1145,8 @@ class PhaserChannel:
         Gains are given in units of output full per scale per input full scale.
 
         .. note:: Due to inherent constraints of the fixed point datatypes and IIR
-        filters, the setpoint resolution depends on the selected gains. This is
-        especially the case for low ``ki`` gains.
+        filters, the ``x_offset`` (setpoint) resolution depends on the selected gains. 
+        Low ``ki`` gains will lead to a low ``x_offset`` resolution.
 
         The transfer function is (up to time discretization and
         coefficient quantization errors):

From 953dd899fd9f06088a05cf9650492c14377169ce Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 23 Jun 2022 15:46:15 +0000
Subject: [PATCH 23/26] refine docu

---
 artiq/coredevice/phaser.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index b69677500..eeadfc73e 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -133,13 +133,15 @@ class Phaser:
 
     Each channel IIR features 4 profiles, each consisting of the [b0, b1, a1] filter
     coefficients as well as an output offset. The coefficients and offset can be
-    set for each profile individually and the profiles each have their own filter
-    state. To avoid transient effects, care should be taken to not update the
-    coefficents in the currently selected profile.
+    set for each profile individually and the profiles each have their own ``y0``,
+    ``y1`` output registers (the ``x0``, ``x1`` inputs are shared). To avoid
+    transient effects, care should be taken to not update the coefficents in the
+    currently selected profile.
 
     The IIR output can be put on hold for each channel. In hold mode, the filter
-    still ingests samples and updates its input x0 and x1 registers, but does not
-    update the y0, y1 output registers. The servo can also be bypassed.
+    still ingests samples and updates its input ``x0`` and ``x1`` registers, but
+    does not update the ``y0``, ``y1`` output registers. The servo can also be
+    bypassed.
 
     After power-up the servo is bypassed, in profile 0, with coefficients [0, 0, 0]
     and hold is enabled. If older gateware without ther servo is loaded onto the

From 9c8ffa54b2c3ef7ccb5e0cdca5c3742a982a1d3a Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Wed, 6 Jul 2022 14:33:46 +0000
Subject: [PATCH 24/26] reverse to servo enable. hopefully adapted all comments
 etc.

---
 artiq/coredevice/phaser.py | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/artiq/coredevice/phaser.py b/artiq/coredevice/phaser.py
index eeadfc73e..9fad1d964 100644
--- a/artiq/coredevice/phaser.py
+++ b/artiq/coredevice/phaser.py
@@ -138,14 +138,16 @@ class Phaser:
     transient effects, care should be taken to not update the coefficents in the
     currently selected profile.
 
+    The servo can be en- or disabled for each channel. When disabled, the servo
+    output multiplier is simply bypassed and the datastream reaches the DAC unscaled.
+
     The IIR output can be put on hold for each channel. In hold mode, the filter
     still ingests samples and updates its input ``x0`` and ``x1`` registers, but
-    does not update the ``y0``, ``y1`` output registers. The servo can also be
-    bypassed.
+    does not update the ``y0``, ``y1`` output registers.
 
-    After power-up the servo is bypassed, in profile 0, with coefficients [0, 0, 0]
+    After power-up the servo is disabled, in profile 0, with coefficients [0, 0, 0]
     and hold is enabled. If older gateware without ther servo is loaded onto the
-    Phaser FPGA, the device simply behaves as if the servo is bypassed and none of
+    Phaser FPGA, the device simply behaves as if the servo is disabled and none of
     the servo functions have any effect.
 
     .. note:: Various register settings of the DAC and the quadrature
@@ -346,7 +348,7 @@ class Phaser:
             delay(.1*ms)
             channel.set_att_mu(0x00)  # minimum attenuation
 
-            channel.set_servo(profile=0, bypass=1, hold=1)
+            channel.set_servo(profile=0, enable=0, hold=1)
 
             # test oscillators and DUC
             for i in range(len(channel.oscillator)):
@@ -1084,19 +1086,19 @@ class PhaserChannel:
         self.trf_write(data)
 
     @kernel
-    def set_servo(self, profile=0, bypass=1, hold=0):
+    def set_servo(self, profile=0, enable=0, hold=0):
         """Set the servo configuration.
 
-        :param bypass: 1 to enable bypass (default), 0 to engage servo. If bypassed, hold
-            is forced since the control loop is broken.
+        :param enable: 1 to enable servo, 0 to disable servo (default). If disabled,
+            the servo is bypassed and hold is enforced since the control loop is broken.
         :param hold: 1 to hold the servo IIR filter output constant, 0 for normal operation.
         :param profile: Profile index to select for channel. (0 to 3)
         """
         if (profile < 0) or (profile > 3):
             raise ValueError("invalid profile index")
         addr = PHASER_ADDR_SERVO_CFG0 + self.index
-        # enforce hold if the servo is bypassed
-        data = (profile << 2) | (((hold | bypass) & 1) << 1) | (~bypass & 1)
+        # enforce hold if the servo is disabled
+        data = (profile << 2) | (((hold | ~enable) & 1) << 1) | (enable & 1)
         self.phaser.write8(addr, data)
 
     @kernel

From 57ac6ec00370d2c183f3e22666261532496d3852 Mon Sep 17 00:00:00 2001
From: SingularitySurfer <Norman_Krackow@gmx.de>
Date: Thu, 7 Jul 2022 10:19:47 +0000
Subject: [PATCH 25/26] add release note

---
 RELEASE_NOTES.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/RELEASE_NOTES.rst b/RELEASE_NOTES.rst
index 213f77598..0c1f0af74 100644
--- a/RELEASE_NOTES.rst
+++ b/RELEASE_NOTES.rst
@@ -21,6 +21,7 @@ Highlights:
    - Expose the DAC coarse mixer and ``sif_sync``
    - Exposes upconverter calibration and enabling/disabling of upconverter LO & RF outputs.
    - Add helpers to align Phaser updates to the RTIO timeline (``get_next_frame_mu()``)
+   - Implemented Phaser-servo.
 * Core device moninj is now proxied via the ``aqctl_moninj_proxy`` controller.
 * The configuration entry ``rtio_clock`` supports multiple clocking settings, deprecating the usage
   of compile-time options.

From 4ea11f46099d8d8045cbcac1cf2f02ee9763bafc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20J=C3=B6rdens?= <rj@quartiq.de>
Date: Thu, 7 Jul 2022 16:03:35 +0200
Subject: [PATCH 26/26] RELEASE_NOTES: update servo note

---
 RELEASE_NOTES.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/RELEASE_NOTES.rst b/RELEASE_NOTES.rst
index 0c1f0af74..6f8783795 100644
--- a/RELEASE_NOTES.rst
+++ b/RELEASE_NOTES.rst
@@ -21,7 +21,7 @@ Highlights:
    - Expose the DAC coarse mixer and ``sif_sync``
    - Exposes upconverter calibration and enabling/disabling of upconverter LO & RF outputs.
    - Add helpers to align Phaser updates to the RTIO timeline (``get_next_frame_mu()``)
-   - Implemented Phaser-servo.
+   - Implemented Phaser-servo. This requires recent gateware on Phaser.
 * Core device moninj is now proxied via the ``aqctl_moninj_proxy`` controller.
 * The configuration entry ``rtio_clock`` supports multiple clocking settings, deprecating the usage
   of compile-time options.