Re: [PATCH v2 1/2] dt-bindings: iio: adc: add support for PAC1711

Jonathan Cameron <[email protected]> Sun, 2 Aug 2026 00:13:54 +0100
Newsgroups org.kernel.vger.linux-iio,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel
Message-ID <20260802001354.6aab1136@jic23-huawei>


> > +  microchip,gpio1-mode:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description:
> > +      Defines the function of the pin. This is a multifunction gpio digital I/O
> > +      pin which can be configured as alert1 interrupt, GPIO digital input, GPIO
> > +      digital output or slow. When functioning as SLOW pin pulling the pin high
> > +      overrides the programmed sample rate and results in a sample rate of 8 sps
> > +      (Slow mode).
> > +    enum: [alert1, gpio1_input, gpio1_output, slow]
> > +    default: gpio1_input  
> 
> Maybe I missed something in the previous discussions, but this
> seems a bit too restrictive and also a bit redundant.
> 
> interrupt-names already tells us if A0 or A1 is used for /ALERT.

Agreed - so for that make the properties mutually exclusive.

> 
> And why should we restrict GPIO usage to only input or output?
> (Binding should have gpio-controller and #gpio-cells properties
> for that too.)

Yeah. Seems that would just be gpio or allow the presence of a gpio
consumer indicate that.

> 
> It also isn't clear to me how the slow pin would be useful when
> we can also program the sample mode to the same rate over I2C.
> So maybe we should defer adding a binding for that until we have
> an application that actually requires it.
> 
> And this is missing the possibility that the pins can be used
> as a conversion trigger as well. Likely that would use a
> trigger-sources binding, but as that isn't common, I would defer
> adding that until we have a use case.
> 
> So I would just leave these properties out.

Makes sense to me. This definitely falls into the corner case
for bindings where we don't yet know what the right thing to do 
is so we don't provide full bindings from the start.

Mention this is intentionally not here as part of the patch description.