Re: [PATCH v3 07/10] iio: frequency: ad9910: support drctl control

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 03 Nov 2026 11:53:03 -0300
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> The DRCTL pin controls the slope direction of the AD9910 digital ramp
> generator (DRG). When dwell is enabled at a ramp limit, the DRG holds
> that limit until the next DRCTL transition, so the sweep pattern is
> defined by the waveform applied to that pin, which can be generated by
> a PWM.
> 
> Get the optional "drctl" PWM and expose the integration_time attribute
> in the DRG ramp up and ramp down channels, which sets the time interval
> assigned to each ramp direction while the sweep parameter is accumulating.
> 
> The DRCTL waveform is described with the PWM waveform API, using the duty
> offset when DRCTL needs to start low. The PWM is disabled when the
> resulting period is zero. The waveform is rounded by the PWM provider
> before being applied, so that integration_time reports the intervals
> achieved by it. Writing to integration_time fails with -EOPNOTSUPP when
> no DRCTL PWM is available. As the waveform API has no stubs, the driver
> now depends on PWM.
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>

The passing comment reflects discussion on v2.  Still seems worth
tidying up to me!  Maybe as a follow up though.

Jonathan


>  static void ad9910_hw_powerdown_action(void *data)
>  {
>  	struct ad9910_state *st = data;
> @@ -2249,6 +2365,26 @@ static int ad9910_probe(struct spi_device *spi)
>  
>  	fsleep(AD9910_WAKEUP_DELAY_us);
>  
> +	/*
> +	 * DRCTL is the only PWM used, so it is looked up by index. A named
> +	 * lookup would fail with -EINVAL when pwm-names is absent, preventing
> +	 * the optional handling below.
> +	 */
> +	st->drctl_pwm = devm_pwm_get(dev, NULL);

Maybe not something for this series, but it might be nice to have
a devm_pwm_get_optional() to avoid having to track specific error
values. Probably return NULL for nothing specified,
similar to your wrapper code here. 


> +	if (IS_ERR(st->drctl_pwm)) {
> +		ret = PTR_ERR(st->drctl_pwm);
> +		if (ret != -ENOENT && ret != -ENODEV)
> +			return dev_err_probe(dev, PTR_ERR(st->drctl_pwm),
> +					     "failed to get drctl pwm\n");
> +		st->drctl_pwm = NULL;
> +	} else {
> +		ret = devm_add_action_or_reset(dev, ad9910_drctl_disable_action,
> +					       st->drctl_pwm);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to add drctl disable action\n");
> +	}
> +
>  	ret = ad9910_parse_fw(st);
>  	if (ret)
>  		return ret;
>