Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

["Nuno Sá" <noname.nuno@gmail.com>]

On Wed, 2025-12-03 at 14:59 +0200, Andy Shevchenko wrote:
> On Wed, Dec 03, 2025 at 11:02:45AM +0000, Nuno Sá wrote:
> > On Tue, 2025-12-02 at 23:26 +0200, Andy Shevchenko wrote:
> > > On Tue, Dec 2, 2025 at 10:55 PM Marcelo Schmitt
> > > <marcelo.schmitt@analog.com> wrote:
> 
> Nuno, may you please remove unrelated context when replying?

It was not that much. That is why I did not bothered :)

...

> 
> > 
> > Hmm, can you share why we should have a reset controller for the above? 
> 
> My point here is to have a standard way of handling "reset" pin independently
> of what's beneath in the HW — GPIO or other means to assert/deassert it.

That makes sense.

> 
> > Unless I'm missing something, even with the aux device, you'll need the code to
> > optionally add it which (I think) will already force you to check the existence for
> > the pin (which would be a bit odd IMO).
> 
> If this is the case, it needs to be fixed, but reset framework provides
> _optional() API, that's what should be used for the cases where reset is
> optional. Let reset framework to handle that.

Ok, I think I was also misunderstanding you. So you mean that instead of doing 
devm_gpiod_get_optional() we should use one of the devm_reset_control_get_*() calls? 

Ok, I went to check the reset core implementation and with [1] I take back my comment. I can see now
that the framework will automatically handle creating the auxdevice. So while I still think most of
the times we'll still see reset-gpios in bindings, it makes sense to have this HW abstraction in the
code.

One thing to note is that the reset framework always enforces reset-gpios and we do have places
where reset pins have different ids (just because that's how the datasheet defines them).

...

> 
> > Having said the above, I would be up for some kind of helper in gpiolib.
> > I still see way too often people misinterpreting the meaning of
> > GPIOD_OUT_HIGH and that the value in gpiod_set_value_cansleep() means
> > assert/deassert.
> 
> Consider this as a helper :-)

Indeed!

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/reset/core.c#n1038

- Nuno Sá