Re: [PATCH v9 10/17] iio: frequency: ad9910: initial driver implementation

On Mon, 27 Jul 2026 10:37:26 +0100
Rodrigo Alencar <[email protected]> wrote:

> On 25/07/26 23:33, Jonathan Cameron wrote:
> > On Wed, 22 Jul 2026 16:50:19 +0100
> > Rodrigo Alencar via B4 Relay <[email protected]> wrote:
> >   
> > > From: Rodrigo Alencar <[email protected]>
> > > 
> > > 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 <[email protected]>  
> > 
> > ...
> >   
> > > diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
> > > new file mode 100644
> > > index 000000000000..b41b011af281
> > > --- /dev/null
> > > +++ b/drivers/iio/frequency/ad9910.c  
> > 
> > ...
> >   
> > > +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";  
> > 
> > Sashiko did have a question about this.  I didn't care enough about the particular
> > combination restrictions on the clock being output, but please have a quick check.
> > https://sashiko.dev/#/patchset/20260722-ad9910-iio-driver-v9-0-459d1df5ac56%40analog.com
> > 
> > If it is possible but nonsensical, add a comment here.  
> 
> The datasheet states:
> 
> 	When the PLL is enabled, a buffered clock signal is available at 
> 	the REFCLK_OUT pin. This clock signal is the same frequency 
> 	as the REF_CLK input. This is especially useful when a crystal 
> 	is connected because it gives the user a replica of the crystal 
> 	clock for driving other external devices
> 
> REFCLK_OUT is basically the input to the PLL added some gain. So that is only
> applicable when PLL is enabled. By default, st->data.refclk_out_drv is zero
> so it is disabled when PLL is not being used.
> 
> This DT-property (adi,refclk-out-drive-strength) is also dependent on the
> adi,pll-enable property in the bindings.

Thanks for laying that out.  Maybe add a breadcrumb but not that important.

Jonathan


> 
> > > +		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;
> > > +}  
>