Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134
From: Nuno Sá
Date: Wed Dec 03 2025 - 09:48:07 EST
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@xxxxxxxxxx> 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á