Re: [PATCH v2 06/12] iio: dac: ad5686: consume optional reset signal

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 9 Jun 2026 12:17:01 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:

> On 09/06/26 10:29, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] Premature reset line deassertion during power ramp-up due to the use of '_deasserted' combined with a manual pulse.
> > --
> > 
> > commit 61717352bc99739a9f89195c3320a934e033ad38
> > Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > 
> > iio: dac: ad5686: consume optional reset signal
> > 
> > Add RESET pin GPIO support through an optional reset control, which is local
> > to the probe function, and include a delay for the required reset pulse width.
> >   
> > > diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> > > index fc3863274b291..700798b80f044 100644
> > > --- a/drivers/iio/dac/ad5686.c
> > > +++ b/drivers/iio/dac/ad5686.c  
> > [ ... ]  
> > > @@ -506,7 +508,15 @@ int ad5686_probe(struct device *dev,
> > >  		return dev_err_probe(dev, -EINVAL,
> > >  				     "invalid or not provided vref voltage\n");
> > >  
> > > +	rstc = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);  
> > 
> > [Severity: Medium]
> > Does this prematurely deassert the reset line during power ramp-up?
> > 
> > Because devm_reset_control_get_optional_exclusive_deasserted() automatically
> > and immediately deasserts the reset line upon acquisition, the reset line is
> > driven high while the power supply is still stabilizing, before the 5 us
> > power-up delay has elapsed.
> > 
> > This creates an unwanted rising edge on the reset pin during power ramp-up,
> > which could cause the device to enter an unknown state or latch up.
> > 
> > Should this use devm_reset_control_get_optional_exclusive() instead, which
> > does not modify the line state during acquisition, allowing the manual toggle
> > sequence below to safely reset the device after power is stable?  
> 
> It has been discussed that it is better to get deasserted and manually create
> the reset pulse. Will move this down (after the fsleep(5) power-up time). Or
> better move this up (before the regulator enable)...

I don't think we need to ensure a pulse. Just that the line is asserted before
deassert.  So to my mind sashiko is correct with it's suggestion.
Just get it in unknown state, ensure that it is asserted (may already be),
then deassert.


> 
> > > +	if (IS_ERR(rstc))
> > > +		return dev_err_probe(dev, PTR_ERR(rstc),
> > > +				     "Failed to get reset control\n");
> > > +
> > >  	fsleep(5); /* power-up time */
> > > +	reset_control_assert(rstc);
> > > +	fsleep(1); /* reset pulse: comfortably bigger than the spec */
> > > +	reset_control_deassert(rstc);
> > >  
> > >  	/* Initialize masks to all ones */
> > >  	st->pwr_down_mask = ~0;  
> > 
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260609-ad5686-new-features-v2-0-70b423f5c76d@analog.com?part=6  
>