Re: [PATCH v6 09/16] iio: frequency: ad9910: initial driver implementation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware sequencing violation: reference clock and GPIO powerdown action are initialized before device regulators, causing backpowering.
--

commit 71fb9226243ea37baeab1cd6841703ff6cde346d
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
Subject: iio: frequency: ad9910: initial driver implementation

This commit adds the core AD9910 DDS driver infrastructure with single tone
mode support, including SPI register access, profile management via GPIO pins,
and PLL/DAC configuration from firmware properties.

> diff --git a/drivers/iio/frequency/ad9910.c b/drivers/iio/frequency/ad9910.c
> new file mode 100644
> index 0000000000000..95b01295e4a0a
> --- /dev/null
> +++ b/drivers/iio/frequency/ad9910.c
[ ... ]
> +static int ad9910_probe(struct spi_device *spi)
> +{
[ ... ]
> +	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");
[ ... ]
> +	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");
> +
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get regulators\n");

[Severity: High]
Could this initialization order cause the device to be backpowered?

In ad9910_probe(), the reference clock is enabled and the hardware powerdown
cleanup action is registered before the device's regulators are enabled. This
might drive an active clock signal into the unpowered IC during the probe
sequence.

Additionally, because devres executes cleanup actions in reverse order (LIFO),
if a probe failure occurs later or the driver unbinds, the regulators will be
turned off first. The powerdown GPIO would then be driven high and the
reference clock would continue to toggle into an unpowered chip until they
are subsequently disabled.

Driving active digital signals into an unpowered CMOS device forward-biases
its internal ESD protection diodes, which can lead to latch-up or permanent
hardware damage. Should the regulators be enabled prior to applying any
clock or active GPIO signals?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-ad9910-iio-driver-v6-0-79125ffbe430@analog.com?part=9