Re: [PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support

2026-04-26 Thread Jonathan Cameron
On Fri, 17 Apr 2026 09:17:33 +0100
Rodrigo Alencar via B4 Relay  
wrote:

> From: Rodrigo Alencar 
> 
> Add DRG channels with destination selection (frequency, phase, or

I'd spell out Digital Ramp Generator here as well rather than
just in the patch title.
 
I'll come back to the comment below (probably) when looking at the
ABI documentation.

> amplitude) based on attribute writes, dwell mode control,
> configurable upper/lower limits, increment/decrement step sizes, and
> step rate settings.
> 
> Signed-off-by: Rodrigo Alencar 
> ---
>  drivers/iio/frequency/ad9910.c | 425 
> -
>  1 file changed, 423 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
> index 5b4076028a29..c9ec677cd63a 100644
> --- a/drivers/iio/frequency/ad9910.c
> +++ b/drivers/iio/frequency/ad9910.c

>  
> +static const struct iio_chan_spec_ext_info ad9910_drg_ramp_ext_info[] = {
> + AD9910_DRG_EXT_INFO("frequency_step", AD9910_DRG_FREQ_STEP),
> + AD9910_DRG_EXT_INFO("phase_step", AD9910_DRG_PHASE_STEP),
> + AD9910_DRG_EXT_INFO("scale_step", AD9910_DRG_AMP_STEP),

For things that are tidied to a clock rate we normally try to express
then in terms of time rather than ticks / steps as that's what the
user normally cares about.  Anyhow I'll come back to this with the
documentation patch review.

> + { }
> +};



[PATCH RFC v3 4/9] iio: frequency: ad9910: add digital ramp generator support

2026-04-17 Thread Rodrigo Alencar via B4 Relay
From: Rodrigo Alencar 

Add DRG channels with destination selection (frequency, phase, or
amplitude) based on attribute writes, dwell mode control,
configurable upper/lower limits, increment/decrement step sizes, and
step rate settings.

Signed-off-by: Rodrigo Alencar 
---
 drivers/iio/frequency/ad9910.c | 425 -
 1 file changed, 423 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
index 5b4076028a29..c9ec677cd63a 100644
--- a/drivers/iio/frequency/ad9910.c
+++ b/drivers/iio/frequency/ad9910.c
@@ -132,6 +132,18 @@
 #define AD9910_MC_SYNC_OUTPUT_DELAY_MSKGENMASK(15, 11)
 #define AD9910_MC_SYNC_INPUT_DELAY_MSK GENMASK(7, 3)
 
+/* Digital Ramp Limit Register */
+#define AD9910_DRG_LIMIT_UPPER_MSK GENMASK_ULL(63, 32)
+#define AD9910_DRG_LIMIT_LOWER_MSK GENMASK_ULL(31, 0)
+
+/* Digital Ramp Step Register */
+#define AD9910_DRG_STEP_DEC_MSKGENMASK_ULL(63, 32)
+#define AD9910_DRG_STEP_INC_MSKGENMASK_ULL(31, 0)
+
+/* Digital Ramp Rate Register */
+#define AD9910_DRG_RATE_DEC_MSKGENMASK(31, 16)
+#define AD9910_DRG_RATE_INC_MSKGENMASK(15, 0)
+
 /* Profile Register Format (Single Tone Mode) */
 #define AD9910_PROFILE_ST_ASF_MSK  GENMASK_ULL(61, 48)
 #define AD9910_PROFILE_ST_POW_MSK  GENMASK_ULL(47, 32)
@@ -147,6 +159,7 @@
 #define AD9910_ASF_PP_LSB_MAX  GENMASK(5, 0)
 #define AD9910_POW_MAX GENMASK(15, 0)
 #define AD9910_POW_PP_LSB_MAX  GENMASK(7, 0)
+#define AD9910_STEP_RATE_MAX   GENMASK(15, 0)
 #define AD9910_NUM_PROFILES8
 
 /* PLL constants */
@@ -198,6 +211,9 @@
  * @AD9910_CHANNEL_PROFILE_6: Profile 6 output channel
  * @AD9910_CHANNEL_PROFILE_7: Profile 7 output channel
  * @AD9910_CHANNEL_PARALLEL_PORT: Parallel port output channel
+ * @AD9910_CHANNEL_DRG: Digital Ramp Generator output channel
+ * @AD9910_CHANNEL_DRG_RAMP_UP: DRG ramp up channel
+ * @AD9910_CHANNEL_DRG_RAMP_DOWN: DRG ramp down channel
  */
 enum ad9910_channel {
AD9910_CHANNEL_PHY = 100,
@@ -210,6 +226,24 @@ enum ad9910_channel {
AD9910_CHANNEL_PROFILE_6 = 107,
AD9910_CHANNEL_PROFILE_7 = 108,
AD9910_CHANNEL_PARALLEL_PORT = 110,
+   AD9910_CHANNEL_DRG = 120,
+   AD9910_CHANNEL_DRG_RAMP_UP = 121,
+   AD9910_CHANNEL_DRG_RAMP_DOWN = 122,
+};
+
+/**
+ * enum ad9910_destination - AD9910 DDS core parameter destination
+ *
+ * @AD9910_DEST_FREQUENCY: Frequency destination
+ * @AD9910_DEST_PHASE: Phase destination
+ * @AD9910_DEST_AMPLITUDE: Amplitude destination
+ * @AD9910_DEST_POLAR: Polar destination
+ */
+enum ad9910_destination {
+   AD9910_DEST_FREQUENCY,
+   AD9910_DEST_PHASE,
+   AD9910_DEST_AMPLITUDE,
+   AD9910_DEST_POLAR,
 };
 
 enum {
@@ -236,6 +270,9 @@ enum {
AD9910_PP_FREQ_OFFSET,
AD9910_PP_PHASE_OFFSET,
AD9910_PP_AMP_OFFSET,
+   AD9910_DRG_FREQ_STEP,
+   AD9910_DRG_PHASE_STEP,
+   AD9910_DRG_AMP_STEP,
 };
 
 struct ad9910_data {
@@ -475,6 +512,16 @@ static int ad9910_powerdown_set(struct ad9910_state *st, 
bool enable)
return gpiod_set_value_cansleep(st->gpio_pwdown, enable);
 }
 
+static inline int ad9910_drg_destination_set(struct ad9910_state *st,
+enum ad9910_destination dest,
+bool update)
+{
+   return ad9910_reg32_update(st, AD9910_REG_CFR2,
+  AD9910_CFR2_DRG_DEST_MSK,
+  FIELD_PREP(AD9910_CFR2_DRG_DEST_MSK, dest),
+  update);
+}
+
 static ssize_t ad9910_ext_info_read(struct iio_dev *indio_dev,
uintptr_t private,
const struct iio_chan_spec *chan,
@@ -638,6 +685,140 @@ static ssize_t ad9910_pp_attrs_write(struct iio_dev 
*indio_dev,
return ret ?: len;
 }
 
+static ssize_t ad9910_drg_attrs_read(struct iio_dev *indio_dev,
+uintptr_t private,
+const struct iio_chan_spec *chan,
+char *buf)
+{
+   struct ad9910_state *st = iio_priv(indio_dev);
+   unsigned int type;
+   int vals[2];
+   u64 tmp64;
+
+   guard(mutex)(&st->lock);
+
+   switch (chan->channel) {
+   case AD9910_CHANNEL_DRG_RAMP_UP:
+   tmp64 = FIELD_GET(AD9910_DRG_STEP_INC_MSK,
+ st->reg[AD9910_REG_DRG_STEP].val64);
+   break;
+   case AD9910_CHANNEL_DRG_RAMP_DOWN:
+   tmp64 = FIELD_GET(AD9910_DRG_STEP_DEC_MSK,
+ st->reg[AD9910_REG_DRG_STEP].val64);
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   switch (private)