| Message ID | 20221125220903.8632-1-samuel@sholland.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | dt-bindings: iio: adc: ti,adc081c: Document the binding | expand |
On 25/11/2022 23:09, Samuel Holland wrote: > Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but > the compatible strings were undocumented. Add a binding for them. The > hardware has an alert interrupt output, but existing ti,adc081c users > do not provide the 'interrupts' property, so leave it as optional. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > .../bindings/iio/adc/ti,adc081c.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml > new file mode 100644 > index 000000000000..caaad777580c > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml > @@ -0,0 +1,55 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TI Single-channel I2C ADCs > + > +maintainers: > + - Jonathan Cameron <jic23@kernel.org> > + - Lars-Peter Clausen <lars@metafoo.de> > + > +description: | > + Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts. > + > +properties: > + compatible: > + enum: > + - ti,adc081c > + - ti,adc101c > + - ti,adc121c > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + vref-supply: > + description: > + Regulator for the combined power supply and voltage reference > + > + "#io-channel-cells": > + const: 1 > + > +required: > + - compatible > + - reg Why not requiring io-channel-cells? If it is an IIO ADC provider, you need the cells, right? Best regards, Krzysztof
On Sun, 27 Nov 2022 13:51:19 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 25/11/2022 23:09, Samuel Holland wrote: > > Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but > > the compatible strings were undocumented. Add a binding for them. The > > hardware has an alert interrupt output, but existing ti,adc081c users > > do not provide the 'interrupts' property, so leave it as optional. > > > > Signed-off-by: Samuel Holland <samuel@sholland.org> > > --- > > > > .../bindings/iio/adc/ti,adc081c.yaml | 55 +++++++++++++++++++ > > 1 file changed, 55 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml > > new file mode 100644 > > index 000000000000..caaad777580c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml > > @@ -0,0 +1,55 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: TI Single-channel I2C ADCs > > + > > +maintainers: > > + - Jonathan Cameron <jic23@kernel.org> > > + - Lars-Peter Clausen <lars@metafoo.de> > > + > > +description: | > > + Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts. > > + > > +properties: > > + compatible: > > + enum: > > + - ti,adc081c > > + - ti,adc101c > > + - ti,adc121c > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + vref-supply: > > + description: > > + Regulator for the combined power supply and voltage reference > > + > > + "#io-channel-cells": > > + const: 1 > > + > > +required: > > + - compatible > > + - reg > > Why not requiring io-channel-cells? If it is an IIO ADC provider, you > need the cells, right? Only if anyone is using it as a provider. If it's purely being used via IIO then there are no consumers registered. So historically I've left it up to those defining the binding to decide if they think #io-channel-cells should be required or optional. It gets a bit non obvious with some of the more complex special ADCs on whether they will ever be consumed. This one is generic, so quite likely it will be. Jonathan > > Best regards, > Krzysztof >
On 11/27/22 11:42, Jonathan Cameron wrote: > On Sun, 27 Nov 2022 13:51:19 +0100 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 25/11/2022 23:09, Samuel Holland wrote: >>> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but >>> the compatible strings were undocumented. Add a binding for them. The >>> hardware has an alert interrupt output, but existing ti,adc081c users >>> do not provide the 'interrupts' property, so leave it as optional. >>> >>> Signed-off-by: Samuel Holland <samuel@sholland.org> >>> --- >>> >>> .../bindings/iio/adc/ti,adc081c.yaml | 55 +++++++++++++++++++ >>> 1 file changed, 55 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>> new file mode 100644 >>> index 000000000000..caaad777580c >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>> @@ -0,0 +1,55 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: TI Single-channel I2C ADCs >>> + >>> +maintainers: >>> + - Jonathan Cameron <jic23@kernel.org> >>> + - Lars-Peter Clausen <lars@metafoo.de> >>> + >>> +description: | >>> + Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - ti,adc081c >>> + - ti,adc101c >>> + - ti,adc121c >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + vref-supply: >>> + description: >>> + Regulator for the combined power supply and voltage reference >>> + >>> + "#io-channel-cells": >>> + const: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >> >> Why not requiring io-channel-cells? If it is an IIO ADC provider, you >> need the cells, right? > > Only if anyone is using it as a provider. If it's purely being used via > IIO then there are no consumers registered. > > So historically I've left it up to those defining the binding to decide if > they think #io-channel-cells should be required or optional. > > It gets a bit non obvious with some of the more complex special ADCs on whether > they will ever be consumed. This one is generic, so quite likely it will be. I kept #io-channel-cells optional because there are already a handful of boards using ti,adc081c without it. On the board I am adding (Clockwork DevTerm), the ADC is used a temperature sensor for a thermal printer. So whether or not the ADC is used as an OF provider depends on how the printer driver gets implemented. Regards, Samuel
On 27/11/2022 19:01, Samuel Holland wrote: > On 11/27/22 11:42, Jonathan Cameron wrote: >> On Sun, 27 Nov 2022 13:51:19 +0100 >> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >> >>> On 25/11/2022 23:09, Samuel Holland wrote: >>>> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but >>>> the compatible strings were undocumented. Add a binding for them. The >>>> hardware has an alert interrupt output, but existing ti,adc081c users >>>> do not provide the 'interrupts' property, so leave it as optional. >>>> >>>> Signed-off-by: Samuel Holland <samuel@sholland.org> >>>> --- >>>> >>>> .../bindings/iio/adc/ti,adc081c.yaml | 55 +++++++++++++++++++ >>>> 1 file changed, 55 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>>> new file mode 100644 >>>> index 000000000000..caaad777580c >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>>> @@ -0,0 +1,55 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: TI Single-channel I2C ADCs >>>> + >>>> +maintainers: >>>> + - Jonathan Cameron <jic23@kernel.org> >>>> + - Lars-Peter Clausen <lars@metafoo.de> >>>> + >>>> +description: | >>>> + Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts. >>>> + >>>> +properties: >>>> + compatible: >>>> + enum: >>>> + - ti,adc081c >>>> + - ti,adc101c >>>> + - ti,adc121c >>>> + >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + interrupts: >>>> + maxItems: 1 >>>> + >>>> + vref-supply: >>>> + description: >>>> + Regulator for the combined power supply and voltage reference >>>> + >>>> + "#io-channel-cells": >>>> + const: 1 >>>> + >>>> +required: >>>> + - compatible >>>> + - reg >>> >>> Why not requiring io-channel-cells? If it is an IIO ADC provider, you >>> need the cells, right? >> >> Only if anyone is using it as a provider. If it's purely being used via >> IIO then there are no consumers registered. >> >> So historically I've left it up to those defining the binding to decide if >> they think #io-channel-cells should be required or optional. >> >> It gets a bit non obvious with some of the more complex special ADCs on whether >> they will ever be consumed. This one is generic, so quite likely it will be. > > I kept #io-channel-cells optional because there are already a handful of > boards using ti,adc081c without it. > > On the board I am adding (Clockwork DevTerm), the ADC is used a > temperature sensor for a thermal printer. So whether or not the ADC is > used as an OF provider depends on how the printer driver gets implemented. Thanks, it's fine. Best regards, Krzysztof
On 27/11/2022 18:42, Jonathan Cameron wrote: > On Sun, 27 Nov 2022 13:51:19 +0100 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 25/11/2022 23:09, Samuel Holland wrote: >>> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but >>> the compatible strings were undocumented. Add a binding for them. The >>> hardware has an alert interrupt output, but existing ti,adc081c users >>> do not provide the 'interrupts' property, so leave it as optional. >>> >>> Signed-off-by: Samuel Holland <samuel@sholland.org> >>> --- >>> >>> .../bindings/iio/adc/ti,adc081c.yaml | 55 +++++++++++++++++++ >>> 1 file changed, 55 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>> new file mode 100644 >>> index 000000000000..caaad777580c >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml >>> @@ -0,0 +1,55 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: TI Single-channel I2C ADCs >>> + >>> +maintainers: >>> + - Jonathan Cameron <jic23@kernel.org> >>> + - Lars-Peter Clausen <lars@metafoo.de> >>> + >>> +description: | >>> + Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - ti,adc081c >>> + - ti,adc101c >>> + - ti,adc121c >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + vref-supply: >>> + description: >>> + Regulator for the combined power supply and voltage reference >>> + >>> + "#io-channel-cells": >>> + const: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >> >> Why not requiring io-channel-cells? If it is an IIO ADC provider, you >> need the cells, right? > > Only if anyone is using it as a provider. If it's purely being used via > IIO then there are no consumers registered. > > So historically I've left it up to those defining the binding to decide if > they think #io-channel-cells should be required or optional. > > It gets a bit non obvious with some of the more complex special ADCs on whether > they will ever be consumed. This one is generic, so quite likely it will be. I remember I asked some time ago and got the same answer... need to write it down into my notes :) Best regards, Krzysztof
On 25/11/2022 23:09, Samuel Holland wrote: > Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but > the compatible strings were undocumented. Add a binding for them. The > hardware has an alert interrupt output, but existing ti,adc081c users > do not provide the 'interrupts' property, so leave it as optional. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Sun, 27 Nov 2022 22:09:52 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 25/11/2022 23:09, Samuel Holland wrote: > > Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but > > the compatible strings were undocumented. Add a binding for them. The > > hardware has an alert interrupt output, but existing ti,adc081c users > > do not provide the 'interrupts' property, so leave it as optional. > > > > Signed-off-by: Samuel Holland <samuel@sholland.org> > > --- > > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Applied, but now we are so close to merge window, I'm queuing things for 6.3. Obviously wouldn't really matter for this, but it is too late for other things in my tree to get enough time in linux-next etc. Jonathan > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml new file mode 100644 index 000000000000..caaad777580c --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI Single-channel I2C ADCs + +maintainers: + - Jonathan Cameron <jic23@kernel.org> + - Lars-Peter Clausen <lars@metafoo.de> + +description: | + Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts. + +properties: + compatible: + enum: + - ti,adc081c + - ti,adc101c + - ti,adc121c + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + vref-supply: + description: + Regulator for the combined power supply and voltage reference + + "#io-channel-cells": + const: 1 + +required: + - compatible + - reg + - vref-supply + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + adc@52 { + compatible = "ti,adc081c"; + reg = <0x52>; + vref-supply = <®_2p5v>; + }; + }; +...
Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but the compatible strings were undocumented. Add a binding for them. The hardware has an alert interrupt output, but existing ti,adc081c users do not provide the 'interrupts' property, so leave it as optional. Signed-off-by: Samuel Holland <samuel@sholland.org> --- .../bindings/iio/adc/ti,adc081c.yaml | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml