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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Mar 18, 2026 at 05:56:04PM +0000, Rodrigo Alencar via B4 Relay wrote:

> Add DRG channels with destination selection (frequency, phase, or
> amplitude), operating mode control, configurable upper/lower limits,
> increment/decrement step sizes, and step rate settings for the digital
> ramp generator.

...

> +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) {
> +	case AD9910_DRG_FREQ_STEP:
> +		type = IIO_VAL_INT_PLUS_MICRO;
> +		tmp64 *= st->data.sysclk_freq_hz;

> +		vals[0] = upper_32_bits(tmp64);
> +		vals[1] = upper_32_bits((u64)lower_32_bits(tmp64) * MICRO);

Not sure if wordparts.h fits here, esp. taking into account...

> +		break;
> +	case AD9910_DRG_PHASE_STEP:
> +		type = IIO_VAL_INT_PLUS_NANO;
> +		tmp64 *= AD9910_PI_NANORAD;
> +		tmp64 >>= 31;
> +		vals[0] = div_u64_rem(tmp64, NANO, &vals[1]);
> +		break;
> +	case AD9910_DRG_AMP_STEP:
> +		type = IIO_VAL_INT_PLUS_NANO;
> +		vals[0] = 0;
> +		vals[1] = tmp64 * NANO >> 32;

...open coded approach here. I think the open coded calculations.

> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return iio_format_value(buf, type, ARRAY_SIZE(vals), vals);
> +}


-- 
With Best Regards,
Andy Shevchenko