From: "Nuno Sá" <noname.nuno@gmail.com>
To: rodrigo.alencar@analog.com
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-hardening@vger.kernel.org,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>,
Jonathan Cameron <jic23@kernel.org>,
David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Philipp Zabel <p.zabel@pengutronix.de>,
Jonathan Corbet <corbet@lwn.net>,
Shuah Khan <skhan@linuxfoundation.org>,
Kees Cook <kees@kernel.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>
Subject: Re: [PATCH v10 10/17] iio: frequency: ad9910: initial driver implementation
Date: Tue, 4 Aug 2026 16:12:07 +0100 [thread overview]
Message-ID: <anIA101fhQ4BIdmp@nsa> (raw)
In-Reply-To: <20260729-ad9910-iio-driver-v10-10-aa1f6167729e@analog.com>
Hi Rodrigo,
Just a late small nit...
On Wed, Jul 29, 2026 at 09:25:37AM +0100, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Add the core AD9910 DDS driver infrastructure with single tone mode
> support. This includes SPI register access, profile management via GPIO
> pins, PLL/DAC configuration from firmware properties, and single tone
> frequency/phase/amplitude control through IIO attributes.
>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> +
> +static inline int ad9910_spi_read(struct ad9910_state *st, u8 reg, void *data,
> + size_t len)
> +{
> + u8 inst = AD9910_SPI_READ_MSK | FIELD_PREP(AD9910_SPI_ADDR_MSK, reg);
> +
> + return spi_write_then_read(st->spi, &inst, sizeof(inst), data, len);
> +}
> +
> +static inline int ad9910_spi_write(struct ad9910_state *st, u8 reg, size_t len,
> + bool update)
> +{
> + int ret;
> +
> + st->tx_buf[0] = FIELD_PREP(AD9910_SPI_ADDR_MSK, reg);
> + ret = spi_write(st->spi, st->tx_buf, AD9910_SPI_DATA_IDX + len);
> + if (ret)
> + return ret;
> +
> + if (update)
> + return ad9910_io_update(st);
> +
> + return 0;
> +}
I would really drop the inline here. It adds no value. Basically just an
hint and the compiler is smart enough to decide whether it should inline
the above or not.
- Nuno Sá
> +
> +#define AD9910_REG_READ_FN(nb) \
> +static int ad9910_reg##nb##_read(struct ad9910_state *st, u8 reg, \
> + u##nb * data) \
> +{ \
> + __be##nb be_data; \
> + int ret; \
> + \
> + ret = ad9910_spi_read(st, reg, &be_data, sizeof(be_data)); \
> + if (ret) \
> + return ret; \
> + \
> + *data = be##nb##_to_cpu(be_data); \
> + return ret; \
> +}
> +
> +AD9910_REG_READ_FN(16)
> +AD9910_REG_READ_FN(32)
> +AD9910_REG_READ_FN(64)
> +
> +#define AD9910_REG_WRITE_FN(nb) \
> +static int ad9910_reg##nb##_write(struct ad9910_state *st, u8 reg, \
> + u##nb data, bool update) \
> +{ \
> + int ret; \
> + \
> + put_unaligned_be##nb(data, &st->tx_buf[AD9910_SPI_DATA_IDX]); \
> + ret = ad9910_spi_write(st, reg, sizeof(data), update); \
> + if (ret) \
> + return ret; \
> + \
> + st->reg[reg].val##nb = data; \
> + return ret; \
> +}
> +
> +AD9910_REG_WRITE_FN(16)
> +AD9910_REG_WRITE_FN(32)
> +AD9910_REG_WRITE_FN(64)
> +
> +#define AD9910_REG_UPDATE_FN(nb) \
> +static int ad9910_reg##nb##_update(struct ad9910_state *st, \
> + u8 reg, u##nb mask, \
> + u##nb data, bool update) \
> +{ \
> + u##nb reg_val = (st->reg[reg].val##nb & ~mask) | (data & mask); \
> + \
> + if (reg_val == st->reg[reg].val##nb && !update) \
> + return 0; \
> + \
> + return ad9910_reg##nb##_write(st, reg, reg_val, update); \
> +}
> +
> +AD9910_REG_UPDATE_FN(16)
> +AD9910_REG_UPDATE_FN(32)
> +AD9910_REG_UPDATE_FN(64)
> +
> +static int ad9910_set_dac_current(struct ad9910_state *st, u32 val_uA,
> + bool update)
> +{
> + u32 code;
> +
> + /* FSC = (86.4 / Rset) * (1 + CODE/96) where Rset = 10k ohms */
> + val_uA = clamp(val_uA, AD9910_DAC_IOUT_MIN_uA, AD9910_DAC_IOUT_MAX_uA);
> + code = DIV_ROUND_CLOSEST(val_uA - AD9910_DAC_IOUT_MIN_uA, 90);
> + st->data.output_current_uA = AD9910_DAC_IOUT_MIN_uA + code * 90;
> +
> + return ad9910_reg32_write(st, AD9910_REG_AUX_DAC, code, update);
> +}
> +
> +static int ad9910_set_sysclk_freq(struct ad9910_state *st, u32 freq_hz,
> + bool update)
> +{
> + struct device *dev = &st->spi->dev;
> + unsigned long refclk_freq_hz;
> + u32 sysclk_freq_hz;
> + u32 tmp32, vco_sel;
> + int ret;
> +
> + if (!freq_hz || freq_hz > AD9910_MAX_SYSCLK_HZ)
> + return -EINVAL;
> +
> + refclk_freq_hz = clk_get_rate(st->refclk);
> + if (st->data.pll_enabled) {
> + if (refclk_freq_hz < AD9910_PLL_IN_MIN_FREQ_HZ ||
> + refclk_freq_hz > AD9910_PLL_IN_MAX_FREQ_HZ) {
> + dev_err(dev,
> + "REF_CLK freq %lu Hz is out of PLL input range\n",
> + refclk_freq_hz);
> + return -ERANGE;
> + }
> +
> + tmp32 = DIV_ROUND_CLOSEST(freq_hz, refclk_freq_hz);
> + tmp32 = clamp(tmp32, DIV_ROUND_UP(AD9910_PLL_OUT_MIN_FREQ_HZ, refclk_freq_hz),
> + AD9910_PLL_OUT_MAX_FREQ_HZ / refclk_freq_hz);
> + tmp32 = clamp(tmp32, AD9910_PLL_MIN_N, AD9910_PLL_MAX_N);
> + sysclk_freq_hz = refclk_freq_hz * tmp32;
> +
> + if (sysclk_freq_hz <= AD9910_VCO0_RANGE_AUTO_MAX_HZ)
> + vco_sel = 0;
> + else if (sysclk_freq_hz <= AD9910_VCO1_RANGE_AUTO_MAX_HZ)
> + vco_sel = 1;
> + else if (sysclk_freq_hz <= AD9910_VCO2_RANGE_AUTO_MAX_HZ)
> + vco_sel = 2;
> + else if (sysclk_freq_hz <= AD9910_VCO3_RANGE_AUTO_MAX_HZ)
> + vco_sel = 3;
> + else if (sysclk_freq_hz <= AD9910_VCO4_RANGE_AUTO_MAX_HZ)
> + vco_sel = 4;
> + else
> + vco_sel = 5;
> +
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR3,
> + AD9910_CFR3_N_MSK | AD9910_CFR3_VCO_SEL_MSK,
> + FIELD_PREP(AD9910_CFR3_N_MSK, tmp32) |
> + FIELD_PREP(AD9910_CFR3_VCO_SEL_MSK, vco_sel),
> + update);
> + if (ret)
> + return ret;
> + } else {
> + if (refclk_freq_hz < AD9910_REFDIV2_MIN_FREQ_HZ ||
> + refclk_freq_hz > AD9910_REFDIV2_MAX_FREQ_HZ) {
> + dev_err(dev,
> + "REF_CLK freq %lu Hz is out of divider range\n",
> + refclk_freq_hz);
> + return -ERANGE;
> + }
> +
> + tmp32 = DIV_ROUND_CLOSEST(refclk_freq_hz, freq_hz);
> + tmp32 = clamp(tmp32, 1U, 2U);
> + sysclk_freq_hz = refclk_freq_hz / tmp32;
> + tmp32 = AD9910_CFR3_VCO_SEL_MSK |
> + FIELD_PREP(AD9910_CFR3_REFCLK_DIV_BYPASS_MSK, tmp32 % 2);
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR3,
> + AD9910_CFR3_VCO_SEL_MSK |
> + AD9910_CFR3_REFCLK_DIV_BYPASS_MSK,
> + tmp32, update);
> + if (ret)
> + return ret;
> + }
> +
> + st->data.sysclk_freq_hz = sysclk_freq_hz;
> +
> + return 0;
> +}
> +
> +static int ad9910_profile_set(struct ad9910_state *st, u8 profile)
> +{
> + DECLARE_BITMAP(values, BITS_PER_TYPE(profile));
> +
> + st->profile = profile;
> + values[0] = profile;
> + gpiod_multi_set_value_cansleep(st->gpio_profile, values);
> +
> + return 0;
> +}
> +
> +static inline bool ad9910_sw_powerdown_get(struct ad9910_state *st)
> +{
> + return FIELD_GET(AD9910_CFR1_SW_POWER_DOWN_MSK,
> + st->reg[AD9910_REG_CFR1].val32) ? true : false;
> +}
> +
> +static int ad9910_sw_powerdown_set(struct ad9910_state *st, bool enable)
> +{
> + int ret;
> +
> + if (ad9910_sw_powerdown_get(st) == enable)
> + return 0;
> +
> + /*
> + * When powering down, the DAC and AUX_DAC (SW1) must be powered down
> + * first, as they require an I/O update to take effect. The opposite is
> + * true when powering up, the DAC and AUX_DAC must be powered up last,
> + * i.e., after the DIGITAL and REFCLK_INPUT (SW0) power up.
> + */
> + if (enable) {
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR1,
> + AD9910_CFR1_SW1_POWER_DOWN_MSK,
> + AD9910_CFR1_SW1_POWER_DOWN_MSK,
> + true);
> + if (ret)
> + return ret;
> +
> + return ad9910_reg32_update(st, AD9910_REG_CFR1,
> + AD9910_CFR1_SW0_POWER_DOWN_MSK,
> + AD9910_CFR1_SW0_POWER_DOWN_MSK,
> + false);
> + }
> +
> + ret = ad9910_reg32_update(st, AD9910_REG_CFR1,
> + AD9910_CFR1_SW0_POWER_DOWN_MSK, 0, false);
> + if (ret)
> + return ret;
> +
> + return ad9910_reg32_update(st, AD9910_REG_CFR1,
> + AD9910_CFR1_SW1_POWER_DOWN_MSK, 0, true);
> +}
> +
> +static ssize_t ad9910_ext_info_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);
> + int val;
> +
> + guard(mutex)(&st->lock);
> +
> + switch (private) {
> + case AD9910_POWERDOWN:
> + val = ad9910_sw_powerdown_get(st);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +}
> +
> +static ssize_t ad9910_ext_info_write(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> + u32 val32;
> + int ret;
> +
> + ret = kstrtou32(buf, 10, &val32);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&st->lock);
> +
> + switch (private) {
> + case AD9910_POWERDOWN:
> + ret = ad9910_sw_powerdown_set(st, val32 ? true : false);
> + if (ret)
> + return ret;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return len;
> +}
> +
> +static const struct iio_chan_spec_ext_info ad9910_phy_ext_info[] = {
> + {
> + .name = "powerdown",
> + .read = ad9910_ext_info_read,
> + .write = ad9910_ext_info_write,
> + .private = AD9910_POWERDOWN,
> + .shared = IIO_SEPARATE,
> + },
> + { }
> +};
> +
> +#define AD9910_PROFILE_CHAN(idx) { \
> + .type = IIO_ALTCURRENT, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = AD9910_CHANNEL_PROFILE_ ## idx, \
> + .address = AD9910_CHAN_IDX_PROFILE_ ## idx, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_ENABLE) | \
> + BIT(IIO_CHAN_INFO_FREQUENCY) | \
> + BIT(IIO_CHAN_INFO_PHASE) | \
> + BIT(IIO_CHAN_INFO_RAW), \
> + .parent = &ad9910_channels[AD9910_CHAN_IDX_PHY], \
> +}
> +
> +static const struct iio_chan_spec ad9910_channels[] = {
> + [AD9910_CHAN_IDX_PHY] = {
> + .type = IIO_ALTCURRENT,
> + .indexed = 1,
> + .output = 1,
> + .channel = AD9910_CHANNEL_PHY,
> + .address = AD9910_CHAN_IDX_PHY,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> + .ext_info = ad9910_phy_ext_info,
> + },
> + [AD9910_CHAN_IDX_PROFILE_0] = AD9910_PROFILE_CHAN(0),
> + [AD9910_CHAN_IDX_PROFILE_1] = AD9910_PROFILE_CHAN(1),
> + [AD9910_CHAN_IDX_PROFILE_2] = AD9910_PROFILE_CHAN(2),
> + [AD9910_CHAN_IDX_PROFILE_3] = AD9910_PROFILE_CHAN(3),
> + [AD9910_CHAN_IDX_PROFILE_4] = AD9910_PROFILE_CHAN(4),
> + [AD9910_CHAN_IDX_PROFILE_5] = AD9910_PROFILE_CHAN(5),
> + [AD9910_CHAN_IDX_PROFILE_6] = AD9910_PROFILE_CHAN(6),
> + [AD9910_CHAN_IDX_PROFILE_7] = AD9910_PROFILE_CHAN(7),
> +};
> +
> +static int ad9910_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long info)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> + u64 tmp64;
> + u32 tmp32;
> +
> + guard(mutex)(&st->lock);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_ENABLE:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + if (ad9910_sw_powerdown_get(st)) {
> + *val = 0;
> + } else {
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + *val = (tmp32 == st->profile);
> + }
> + break;
> + default:
> + return -EINVAL;
> + }
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_FREQUENCY:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp64 = FIELD_GET(AD9910_PROFILE_ST_FTW_MSK,
> + st->reg[AD9910_REG_PROFILE(tmp32)].val64);
> + break;
> + default:
> + return -EINVAL;
> + }
> + tmp64 *= st->data.sysclk_freq_hz;
> + *val = tmp64 >> 32;
> + *val2 = ((tmp64 & GENMASK_ULL(31, 0)) * MICRO) >> 32;
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_PHASE:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp64 = FIELD_GET(AD9910_PROFILE_ST_POW_MSK,
> + st->reg[AD9910_REG_PROFILE(tmp32)].val64);
> + break;
> + default:
> + return -EINVAL;
> + }
> + tmp32 = (tmp64 * AD9910_MAX_PHASE_MICRORAD) >> 16;
> + *val = tmp32 / MICRO;
> + *val2 = tmp32 % MICRO;
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + *val = FIELD_GET(AD9910_PROFILE_ST_ASF_MSK,
> + st->reg[AD9910_REG_PROFILE(tmp32)].val64);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PHY:
> + *val = st->data.sysclk_freq_hz;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->address) {
> + case AD9910_CHAN_IDX_PHY:
> + tmp64 = (u64)st->data.output_current_uA *
> + AD9910_NANO_MILLIAMP_PER_MICROAMP;
> + *val = 0;
> + *val2 = tmp64 >> 14;
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad9910_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long info)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> + u64 tmp64;
> + u32 tmp32;
> + int ret;
> +
> + guard(mutex)(&st->lock);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_ENABLE:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + if (!val) {
> + if (tmp32 != st->profile)
> + return 0; /* nothing to do */
> +
> + return ad9910_sw_powerdown_set(st, true);
> + }
> +
> + ret = ad9910_sw_powerdown_set(st, false);
> + if (ret)
> + return ret;
> +
> + return ad9910_profile_set(st, tmp32);
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_FREQUENCY:
> + if (val < 0 || val2 < 0 || val >= st->data.sysclk_freq_hz / 2)
> + return -EINVAL;
> +
> + tmp64 = ad9910_rational_scale((u64)val * MICRO + val2, BIT_ULL(32),
> + (u64)MICRO * st->data.sysclk_freq_hz);
> + tmp64 = min_t(u64, tmp64, U32_MAX);
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp64 = FIELD_PREP(AD9910_PROFILE_ST_FTW_MSK, tmp64);
> + return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
> + AD9910_PROFILE_ST_FTW_MSK,
> + tmp64, true);
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_PHASE:
> + if (val < 0 || val2 < 0)
> + return -EINVAL;
> +
> + tmp64 = (u64)val * MICRO + val2;
> + if (tmp64 >= AD9910_MAX_PHASE_MICRORAD)
> + return -EINVAL;
> +
> + tmp64 = DIV_U64_ROUND_CLOSEST(tmp64 << 16, AD9910_MAX_PHASE_MICRORAD);
> + tmp64 = min(tmp64, AD9910_POW_MAX);
> +
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp64 = FIELD_PREP(AD9910_PROFILE_ST_POW_MSK, tmp64);
> + return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
> + AD9910_PROFILE_ST_POW_MSK,
> + tmp64, true);
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + if (val < 0)
> + return -EINVAL;
> +
> + tmp32 = chan->channel - AD9910_CHANNEL_PROFILE_0;
> + tmp64 = FIELD_PREP(AD9910_PROFILE_ST_ASF_MSK,
> + min_t(u64, val, AD9910_ASF_MAX));
> + return ad9910_reg64_update(st, AD9910_REG_PROFILE(tmp32),
> + AD9910_PROFILE_ST_ASF_MSK,
> + tmp64, true);
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return ad9910_set_sysclk_freq(st, val, true);
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->address) {
> + case AD9910_CHAN_IDX_PHY:
> + if (val != 0 || val2 < 0)
> + return -EINVAL;
> +
> + tmp32 = DIV_U64_ROUND_CLOSEST((u64)val2 << 14,
> + AD9910_NANO_MILLIAMP_PER_MICROAMP);
> + return ad9910_set_dac_current(st, tmp32, true);
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad9910_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_ENABLE:
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_FREQUENCY:
> + case IIO_CHAN_INFO_PHASE:
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->channel) {
> + case AD9910_CHANNEL_PROFILE_0 ... AD9910_CHANNEL_PROFILE_7:
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad9910_debugfs_reg_access(struct iio_dev *indio_dev,
> + unsigned int reg, u64 writeval,
> + u64 *readval)
> +{
> + struct ad9910_state *st = iio_priv(indio_dev);
> + union ad9910_reg tmp;
> + int ret;
> +
> + if (reg >= AD9910_REG_RAM)
> + return -EINVAL;
> +
> + guard(mutex)(&st->lock);
> +
> + switch (reg) {
> + case AD9910_REG_DRG_LIMIT:
> + case AD9910_REG_DRG_STEP:
> + case AD9910_REG_PROFILE0 ... AD9910_REG_PROFILE7:
> + if (!readval)
> + return ad9910_reg64_write(st, reg, writeval, true);
> +
> + ret = ad9910_reg64_read(st, reg, &tmp.val64);
> + if (ret)
> + return ret;
> + *readval = tmp.val64;
> + return 0;
> + case AD9910_REG_POW:
> + if (!readval)
> + return ad9910_reg16_write(st, reg, writeval, true);
> +
> + ret = ad9910_reg16_read(st, reg, &tmp.val16);
> + if (ret)
> + return ret;
> + *readval = tmp.val16;
> + return 0;
> + default:
> + if (!readval)
> + return ad9910_reg32_write(st, reg, writeval, true);
> +
> + ret = ad9910_reg32_read(st, reg, &tmp.val32);
> + if (ret)
> + return ret;
> + *readval = tmp.val32;
> + return 0;
> + }
> +}
> +
> +static const char * const ad9910_channel_str[] = {
> + [AD9910_CHAN_IDX_PHY] = "phy",
> + [AD9910_CHAN_IDX_PROFILE_0] = "profile0",
> + [AD9910_CHAN_IDX_PROFILE_1] = "profile1",
> + [AD9910_CHAN_IDX_PROFILE_2] = "profile2",
> + [AD9910_CHAN_IDX_PROFILE_3] = "profile3",
> + [AD9910_CHAN_IDX_PROFILE_4] = "profile4",
> + [AD9910_CHAN_IDX_PROFILE_5] = "profile5",
> + [AD9910_CHAN_IDX_PROFILE_6] = "profile6",
> + [AD9910_CHAN_IDX_PROFILE_7] = "profile7",
> +};
> +
> +static int ad9910_read_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + char *label)
> +{
> + return sysfs_emit(label, "%s\n", ad9910_channel_str[chan->address]);
> +}
> +
> +static const struct iio_info ad9910_info = {
> + .read_raw = ad9910_read_raw,
> + .write_raw = ad9910_write_raw,
> + .write_raw_get_fmt = ad9910_write_raw_get_fmt,
> + .read_label = ad9910_read_label,
> + .debugfs_reg64_access = &ad9910_debugfs_reg_access,
> +};
> +
> +static int ad9910_cfg_sysclk(struct ad9910_state *st, bool update)
> +{
> + u32 cfr3 = AD9910_CFR3_OPEN_MSK;
> + u32 tmp32;
> +
> + cfr3 |= FIELD_PREP(AD9910_CFR3_DRV0_MSK, st->data.refclk_out_drv);
> +
> + if (st->data.pll_enabled) {
> + tmp32 = st->data.pll_charge_pump_current - AD9910_ICP_MIN_uA;
> + tmp32 = DIV_ROUND_CLOSEST(tmp32, AD9910_ICP_STEP_uA);
> + cfr3 |= FIELD_PREP(AD9910_CFR3_ICP_MSK, tmp32) |
> + AD9910_CFR3_PLL_EN_MSK;
> + } else {
> + cfr3 |= AD9910_CFR3_ICP_MSK |
> + AD9910_CFR3_REFCLK_DIV_RESETB_MSK |
> + AD9910_CFR3_PFD_RESET_MSK;
> + }
> + st->reg[AD9910_REG_CFR3].val32 = cfr3;
> +
> + return ad9910_set_sysclk_freq(st, AD9910_MAX_SYSCLK_HZ, update);
> +}
> +
> +static int ad9910_parse_fw(struct ad9910_state *st)
> +{
> + static const char * const refclk_out_drv0[] = {
> + "disabled", "low", "medium", "high",
> + };
> + struct device *dev = &st->spi->dev;
> + const char *prop;
> + u32 tmp;
> + int ret;
> +
> + st->data.pll_enabled = device_property_read_bool(dev, "adi,pll-enable");
> + if (st->data.pll_enabled) {
> + prop = "adi,charge-pump-current-microamp";
> + if (device_property_present(dev, prop)) {
> + ret = device_property_read_u32(dev, prop, &tmp);
> + if (ret)
> + return dev_err_probe(dev, ret, "property read: %s\n", prop);
> +
> + if (tmp < AD9910_ICP_MIN_uA || tmp > AD9910_ICP_MAX_uA)
> + return dev_err_probe(dev, -ERANGE,
> + "invalid charge pump current %u\n", tmp);
> + } else {
> + tmp = AD9910_ICP_MIN_uA;
> + }
> + st->data.pll_charge_pump_current = tmp;
> +
> + prop = "adi,refclk-out-drive-strength";
> + if (device_property_present(dev, prop)) {
> + ret = device_property_match_property_string(dev, prop,
> + refclk_out_drv0,
> + ARRAY_SIZE(refclk_out_drv0));
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "property read: %s\n", prop);
> +
> + st->data.refclk_out_drv = ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void ad9910_sw_powerdown_action(void *data)
> +{
> + ad9910_sw_powerdown_set(data, true);
> +}
> +
> +static void ad9910_hw_powerdown_action(void *data)
> +{
> + struct ad9910_state *st = data;
> +
> + gpiod_set_value_cansleep(st->gpio_pwdown, 1);
> +}
> +
> +static int ad9910_setup(struct device *dev, struct ad9910_state *st,
> + struct reset_control *dev_rst)
> +{
> + int ret;
> +
> + ret = reset_control_assert(dev_rst);
> + if (ret)
> + return ret;
> +
> + fsleep(AD9910_RESET_DELAY_us);
> +
> + ret = reset_control_deassert(dev_rst);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_reg32_write(st, AD9910_REG_CFR1,
> + (st->spi->mode & SPI_3WIRE ? 0 :
> + AD9910_CFR1_SDIO_INPUT_ONLY_MSK), false);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev, ad9910_sw_powerdown_action, st);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_reg32_write(st, AD9910_REG_CFR2,
> + AD9910_CFR2_AMP_SCALE_SINGLE_TONE_MSK |
> + AD9910_CFR2_SYNC_TIMING_VAL_DISABLE_MSK |
> + AD9910_CFR2_DRG_NO_DWELL_MSK |
> + AD9910_CFR2_DATA_ASM_HOLD_LAST_MSK |
> + AD9910_CFR2_SYNC_CLK_EN_MSK |
> + AD9910_CFR2_PDCLK_ENABLE_MSK, false);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_cfg_sysclk(st, false);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_set_dac_current(st, AD9910_DAC_IOUT_DEFAULT_uA, false);
> + if (ret)
> + return ret;
> +
> + return ad9910_io_update(st);
> +}
> +
> +static int ad9910_probe(struct spi_device *spi)
> +{
> + static const char * const supplies[] = {
> + "dvdd-io33", "avdd33", "dvdd18", "avdd18",
> + };
> + struct device *dev = &spi->dev;
> + struct reset_control *dev_rst;
> + struct gpio_desc *io_rst_gpio;
> + struct iio_dev *indio_dev;
> + struct ad9910_state *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->spi = spi;
> +
> + indio_dev->name = "ad9910";
> + indio_dev->info = &ad9910_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = ad9910_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ad9910_channels);
> +
> + ret = devm_mutex_init(dev, &st->lock);
> + if (ret)
> + return ret;
> +
> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get regulators\n");
> +
> + st->refclk = devm_clk_get_enabled(dev, "ref_clk");
> + if (IS_ERR(st->refclk))
> + return dev_err_probe(dev, PTR_ERR(st->refclk),
> + "Failed to get reference clock\n");
> +
> + dev_rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> + if (IS_ERR(dev_rst))
> + return dev_err_probe(dev, PTR_ERR(dev_rst),
> + "failed to get device reset control\n");
> +
> + /*
> + * The IO RESET pin is not used in this driver, as we assume that all
> + * SPI transfers are complete, but if it is wired up, we need to make
> + * sure it is not floating. We can use either a reset controller or a
> + * GPIO for this.
> + */
> + io_rst_gpio = devm_gpiod_get_optional(dev, "io-reset", GPIOD_OUT_LOW);
> + if (IS_ERR(io_rst_gpio))
> + return dev_err_probe(dev, PTR_ERR(io_rst_gpio),
> + "failed to get io reset gpio\n");
> +
> + st->gpio_update = devm_gpiod_get_optional(dev, "update", GPIOD_OUT_LOW);
> + if (IS_ERR(st->gpio_update))
> + return dev_err_probe(dev, PTR_ERR(st->gpio_update),
> + "failed to get update gpio\n");
> +
> + st->gpio_profile = devm_gpiod_get_array_optional(dev, "profile",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(st->gpio_profile))
> + return dev_err_probe(dev, PTR_ERR(st->gpio_profile),
> + "failed to get profile gpios\n");
> +
> + if (st->gpio_profile && st->gpio_profile->ndescs != 3)
> + return dev_err_probe(dev, -EINVAL,
> + "invalid number of profile gpios\n");
> +
> + st->gpio_pwdown = devm_gpiod_get_optional(dev, "powerdown",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(st->gpio_pwdown))
> + return dev_err_probe(dev, PTR_ERR(st->gpio_pwdown),
> + "failed to get powerdown gpio\n");
> +
> + ret = devm_add_action_or_reset(dev, ad9910_hw_powerdown_action, st);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to add hw powerdown action\n");
> +
> + fsleep(AD9910_WAKEUP_DELAY_us);
> +
> + ret = ad9910_parse_fw(st);
> + if (ret)
> + return ret;
> +
> + ret = ad9910_setup(dev, st, dev_rst);
> + if (ret)
> + return dev_err_probe(dev, ret, "device setup failed\n");
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static const struct spi_device_id ad9910_id[] = {
> + { .name = "ad9910" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, ad9910_id);
> +
> +static const struct of_device_id ad9910_of_match[] = {
> + { .compatible = "adi,ad9910" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad9910_of_match);
> +
> +static struct spi_driver ad9910_driver = {
> + .driver = {
> + .name = "ad9910",
> + .of_match_table = ad9910_of_match,
> + },
> + .probe = ad9910_probe,
> + .id_table = ad9910_id,
> +};
> +module_spi_driver(ad9910_driver);
> +
> +MODULE_AUTHOR("Rodrigo Alencar <rodrigo.alencar@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD9910 DDS driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.43.0
>
>
next prev parent reply other threads:[~2026-08-04 15:11 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-29 8:25 [PATCH v10 00/17] AD9910 Direct Digital Synthesizer Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 01/17] iio: ABI: add attributes for altcurrent channels Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 02/17] iio: ABI: raw, scale and offset for frequency/phase channels Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 03/17] iio: ABI: add parent entry for iio channels Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 04/17] iio: add IIO_FREQUENCY channel type Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 05/17] iio: core: support 64-bit register through debugfs Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 06/17] iio: core: create local __iio_chan_prefix_emit() for reuse Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 07/17] iio: test: add kunit tests for channel prefix naming generation Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:38 ` sashiko-bot
2026-08-01 23:55 ` Jonathan Cameron
2026-07-29 8:25 ` [PATCH v10 08/17] iio: core: add hierarchical channel relationships Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 09/17] dt-bindings: iio: frequency: add ad9910 Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 10/17] iio: frequency: ad9910: initial driver implementation Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:47 ` sashiko-bot
2026-08-04 15:12 ` Nuno Sá [this message]
2026-07-29 8:25 ` [PATCH v10 11/17] iio: frequency: ad9910: add basic parallel port support Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-08-02 0:56 ` Jonathan Cameron
2026-08-02 9:04 ` David Laight
2026-08-04 23:07 ` Jonathan Cameron
2026-07-29 8:25 ` [PATCH v10 12/17] iio: frequency: ad9910: add digital ramp generator support Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 13/17] iio: frequency: ad9910: add RAM mode support Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:45 ` sashiko-bot
2026-07-29 8:25 ` [PATCH v10 14/17] iio: frequency: ad9910: add output shift keying support Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 15/17] iio: frequency: ad9910: show channel priority in debugfs Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 16/17] iio: ABI: add docs for ad9910 sysfs and debugfs entries Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-07-29 8:25 ` [PATCH v10 17/17] docs: iio: add documentation for ad9910 driver Rodrigo Alencar via B4 Relay
2026-07-29 8:25 ` Rodrigo Alencar
2026-08-02 0:09 ` [PATCH v10 00/17] AD9910 Direct Digital Synthesizer Jonathan Cameron
2026-08-02 0:58 ` Jonathan Cameron
2026-08-04 10:56 ` Rodrigo Alencar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=anIA101fhQ4BIdmp@nsa \
--to=noname.nuno@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=corbet@lwn.net \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=gustavoars@kernel.org \
--cc=jic23@kernel.org \
--cc=kees@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-doc@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=robh@kernel.org \
--cc=rodrigo.alencar@analog.com \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.