On Wed, 22 Jul 2026 16:50:22 +0100 Rodrigo Alencar via B4 Relay <[email protected]> wrote:
> From: Rodrigo Alencar <[email protected]> > > Add RAM control channel, which includes: > - RAM data loading via firmware upload interface; > - Per-profile configuration and DDS core parameter destination as firmware > metadata; > - Profile switching relying on profile channels; > - Sampling frequency control of the active profile; > - ram-enable-aware read/write paths that redirect single tone > frequency/phase/amplitude access through reg_profile cache when RAM is > active; > > When RAM is enabled, the DDS profile parameters (frequency, phase, > amplitude) for the single tone mode are sourced from a shadow register > cache (reg_profile[]) since the profile registers are repurposed for RAM > control. > > Signed-off-by: Rodrigo Alencar <[email protected]> As mentioned in reply to an earlier patch, I haven't looked in detail at the firmware cancel path stuff sashiko is unhappy with. Whilst it looks like the sort of esoteric path where maybe it is fine to fail good to take one more look. One other thing Sashiko commented on inline. I think that is either right or a bit more detail is needed in the comment. Thanks, Jonathan > diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c > index 6c794e1b4b1c..844cc0cc8f3e 100644 > --- a/drivers/iio/frequency/ad9910.c > +++ b/drivers/iio/frequency/ad9910.c ... > @@ -1119,7 +1220,7 @@ static int ad9910_write_raw(struct iio_dev *indio_dev, > struct ad9910_state *st = iio_priv(indio_dev); > u64 tmp64; > u32 tmp32; > - int ret; > + int ret, i; > > guard(mutex)(&st->lock); > > @@ -1156,6 +1257,41 @@ static int ad9910_write_raw(struct iio_dev *indio_dev, > AD9910_CFR2_DRG_DEST_MSK | > AD9910_CFR2_DRG_ENABLE_MSK, > tmp32, true); > + case AD9910_CHANNEL_RAM: > + if (AD9910_RAM_ENABLED(st) == !!val) > + return 0; > + > + /* swap profile configs */ > + for (i = 0; i < AD9910_NUM_PROFILES; i++) { > + tmp64 = st->reg[AD9910_REG_PROFILE(i)].val64; > + ret = ad9910_reg64_write(st, > + AD9910_REG_PROFILE(i), > + st->reg_profile[i], > + false); > + if (ret) > + break; > + st->reg_profile[i] = tmp64; > + } > + > + if (ret) { > + /* > + * After the write failure, profiles 0..i-1 were > + * already swapped in SW, but Hw registers are > + * still pending an IO update, so swap them back > + * in SW to keep the state consistent. Sashiko's follow up question about whether a subsequent use of IO update might end up with these stale values seems like a reasonable one. Perhaps a little more detail on why that doesn't matter is needed here? > + */ > + while (i--) { > + tmp64 = > st->reg[AD9910_REG_PROFILE(i)].val64; > + st->reg[AD9910_REG_PROFILE(i)].val64 = > st->reg_profile[i]; > + st->reg_profile[i] = tmp64; > + } > + return ret; > + } > + > + tmp32 = FIELD_PREP(AD9910_CFR1_RAM_ENABLE_MSK, !!val); > + return ad9910_reg32_update(st, AD9910_REG_CFR1, > + AD9910_CFR1_RAM_ENABLE_MSK, > + tmp32, true); > default: > return -EINVAL; > }
