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

[Nuno Sá]

On Fri, Oct 23, 2026 at 07:40:32AM -0300, Rodrigo Alencar wrote:
> On 22/10/26 12:19, Rodrigo Alencar wrote:
> > 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.
> 
> ...
> 
> > +	st->drctl_pwm = devm_pwm_get(dev, "drctl");
> > +	if (IS_ERR(st->drctl_pwm)) {
> > +		ret = PTR_ERR(st->drctl_pwm);
> > +		if (ret != -ENOENT && ret != -ENODEV)
> 
> Sashiko points out the problem with passing a non-nul con_id to devm_pwm_get(),
> which would get us EINVAL or ENODATA from of_property_match_string(), and that
> would break the intent here for an optional pwm. Similar to what we tried to
> achieve with an optional IIO backend. I wonder what is the best strategy here
> (ascending order of complexity):
> 
> * Use con_id = NULL as DRCTL is the only pwm signal being considered at this point.

Likely the best option for now. If you really envision having other PWM
sources I think the most important place to actually define the name is
in bindings because then we need to care about backward compatibility
(typically case for clocks when we need to grow the number of needed
clocks).

> * Local workaround by checking for EINVAL and ENODATA too

Don't love the above

> * Convert error codes from of_property_match_string() in of_pwm_get()
>   implementation and create a devm_pwm_get_optional(), which would only check
>   for ENODEV and ENOENT.

I guess we would need more users but those should not be hard to find in
tree (But up to Uwe I would say)

- Nuno Sá

> 
> The pwm is optional as the digital ramp generator is one of the possible modes
> to be used.
> 
> > +			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");
> > +	}
> 
> -- 
> Kind regards,
> 
> Rodrigo Alencar