The AD7173 family offer a complete integrated Sigma-Delta ADC solution
which can be used in high precision, low noise single channel applications
or higher speed multiplexed applications. The Sigma-Delta ADC is intended
primarily for measurement of signals close to DC but also delivers
outstanding performance with input bandwidths out to ~10kHz.
Signed-off-by: Dumitru Ceclan <[email protected]>
---
V11->V12
- Drop "binding", describe hardware in binding description
- Rename refin and refin2 to vref and vref2
- Add better description to external references to better show that the voltage
value that needs to be specified is the difference between the positive and
negative reference pins
- Add optional clocks properties
- Add optional adi,clock-select property
- Add option for second interrupt, error
- Add description to interrupts
V10->V11
- Fix example warning: '#gpio-cells' is a dependency of 'gpio-controller'
- Add description to #gpio-cells property
V9->V10
- Fix dt_binding_check type warning from adi,reference-select
V8->v9
- Add gpio-controller and "#gpio-cells" properties
- Add missing avdd2 and iovdd supplies
- Add string type to reference-select
V7->V8
- include missing fix from V6
V6->V7 <no changes>
V5->V6
- Moved global required property to proper placement
V4 -> V5
- Use string enum instead of integers for "adi,reference-select"
- Fix conditional checking in regards to compatible
V3 -> V4
- include supply attributes
- add channel attribute for selecting conversion reference
V2 -> V3
- remove redundant descriptions
- use referenced 'bipolar' property
- remove newlines from example
V1 -> V2 <no changes>
.../bindings/iio/adc/adi,ad7173.yaml | 242 ++++++++++++++++++
1 file changed, 242 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
new file mode 100644
index 000000000000..4d0870cc014c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
@@ -0,0 +1,242 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7173 ADC
+
+maintainers:
+ - Ceclan Dumitru <[email protected]>
+
+description: |
+ Analog Devices AD717x ADC's:
+ The AD717x family offer a complete integrated Sigma-Delta ADC solution which
+ can be used in high precision, low noise single channel applications
+ (Life Science measurements) or higher speed multiplexed applications
+ (Factory Automation PLC Input modules). The Sigma-Delta ADC is intended
+ primarily for measurement of signals close to DC but also delivers
+ outstanding performance with input bandwidths out to ~10kHz.
+
+ Datasheets for supported chips:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7172-2
+ - adi,ad7173-8
+ - adi,ad7175-2
+ - adi,ad7176-2
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ minItems: 1
+ description: |
+ Ready interrupt: multiplexed with SPI data out. While SPI CS is low,
+ can be used to indicate the completion of a conversion.
+
+ Error: The three error bits in the status register (ADC_ERROR, CRC_ERROR,
+ and REG_ERROR) are OR'ed, inverted, and mapped to the ERROR pin. Therefore,
+ the ERROR pin indicates that an error has occurred.
+
+ interrupt-names:
+ minItems: 1
+ items:
+ - const: rdy
+ - const: err
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ spi-max-frequency:
+ maximum: 20000000
+
+ gpio-controller:
+ description: Marks the device node as a GPIO controller.
+
+ "#gpio-cells":
+ const: 2
+ description:
+ The first cell is the GPIO number and the second cell specifies
+ GPIO flags, as defined in <dt-bindings/gpio/gpio.h>.
+
+ vref-supply:
+ description: |
+ Differential external reference supply used for conversion. The reference
+ voltage (Vref) specified here must be the voltage difference between the
+ REF+ and REF- pins: Vref = (REF+) - (REF-).
+
+ vref2-supply:
+ description: |
+ Differential external reference supply used for conversion. The reference
+ voltage (Vref2) specified here must be the voltage difference between the
+ REF2+ and REF2- pins: Vref2 = (REF2+) - (REF2-).
+
+ avdd-supply:
+ description: avdd supply, can be used as reference for conversion.
+
+ avdd2-supply:
+ description: avdd2 supply, used as the input to the internal voltage regulator.
+
+ iovdd-supply:
+ description: iovdd supply, used for the chip digital interface.
+
+ clocks:
+ maxItems: 1
+ description: |
+ Optional external clock source. Can include one clock source: external
+ clock or external crystal.
+
+ clock-names:
+ enum:
+ - ext-clk
+ - xtal
+
+ adi,clock-select:
+ description: |
+ Select the ADC clock source. Valid values are:
+ int : Internal oscillator
+ int-out : Internal oscillator with output on XTAL2 pin
+ ext-clk : External clock input on XTAL2 pin
+ xtal : External crystal on XTAL1 and XTAL2 pins
+
+ $ref: /schemas/types.yaml#/definitions/string
+ enum:
+ - int
+ - int-out
+ - ext-clk
+ - xtal
+ default: int
+
+patternProperties:
+ "^channel@[0-9a-f]$":
+ type: object
+ $ref: adc.yaml
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 15
+
+ diff-channels:
+ items:
+ minimum: 0
+ maximum: 31
+
+ adi,reference-select:
+ description: |
+ Select the reference source to use when converting on
+ the specific channel. Valid values are:
+ vref : REF+ /REF−
+ vref2 : REF2+ /REF2−
+ refout-avss: REFOUT/AVSS (Internal reference)
+ avdd : AVDD
+
+ External reference ref2 only available on ad7173-8.
+ If not specified, internal reference used.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum:
+ - vref
+ - vref2
+ - refout-avss
+ - avdd
+ default: refout-avss
+
+ required:
+ - reg
+ - diff-channels
+
+required:
+ - compatible
+ - reg
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+ - if:
+ properties:
+ compatible:
+ not:
+ contains:
+ const: adi,ad7173-8
+ then:
+ properties:
+ vref2-supply: false
+ patternProperties:
+ "^channel@[0-9a-f]$":
+ properties:
+ adi,reference-select:
+ enum:
+ - vref
+ - refout-avss
+ - avdd
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad7173-8";
+ reg = <0>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-names = "rdy";
+ interrupt-parent = <&gpio>;
+ spi-max-frequency = <5000000>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ vref-supply = <&dummy_regulator>;
+
+ channel@0 {
+ reg = <0>;
+ bipolar;
+ diff-channels = <0 1>;
+ adi,reference-select = "vref";
+ };
+
+ channel@1 {
+ reg = <1>;
+ diff-channels = <2 3>;
+ };
+
+ channel@2 {
+ reg = <2>;
+ bipolar;
+ diff-channels = <4 5>;
+ };
+
+ channel@3 {
+ reg = <3>;
+ bipolar;
+ diff-channels = <6 7>;
+ };
+
+ channel@4 {
+ reg = <4>;
+ diff-channels = <8 9>;
+ adi,reference-select = "avdd";
+ };
+ };
+ };
--
2.42.0
On Thu, Jan 18, 2024 at 02:49:22PM +0200, Dumitru Ceclan wrote:
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
>
> Signed-off-by: Dumitru Ceclan <[email protected]>
> ---
>
> V11->V12
> - Drop "binding", describe hardware in binding description
> - Rename refin and refin2 to vref and vref2
> - Add better description to external references to better show that the voltage
> value that needs to be specified is the difference between the positive and
> negative reference pins
> - Add optional clocks properties
> - Add optional adi,clock-select property
> - Add option for second interrupt, error
> - Add description to interrupts
> V10->V11
> - Fix example warning: '#gpio-cells' is a dependency of 'gpio-controller'
> - Add description to #gpio-cells property
> V9->V10
> - Fix dt_binding_check type warning from adi,reference-select
> V8->v9
> - Add gpio-controller and "#gpio-cells" properties
> - Add missing avdd2 and iovdd supplies
> - Add string type to reference-select
> V7->V8
> - include missing fix from V6
> V6->V7 <no changes>
> V5->V6
> - Moved global required property to proper placement
> V4 -> V5
> - Use string enum instead of integers for "adi,reference-select"
> - Fix conditional checking in regards to compatible
> V3 -> V4
> - include supply attributes
> - add channel attribute for selecting conversion reference
> V2 -> V3
> - remove redundant descriptions
> - use referenced 'bipolar' property
> - remove newlines from example
> V1 -> V2 <no changes>
>
> .../bindings/iio/adc/adi,ad7173.yaml | 242 ++++++++++++++++++
> 1 file changed, 242 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> new file mode 100644
> index 000000000000..4d0870cc014c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> @@ -0,0 +1,242 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7173 ADC
> +
> +maintainers:
> + - Ceclan Dumitru <[email protected]>
> +
> +description: |
> + Analog Devices AD717x ADC's:
> + The AD717x family offer a complete integrated Sigma-Delta ADC solution which
> + can be used in high precision, low noise single channel applications
> + (Life Science measurements) or higher speed multiplexed applications
> + (Factory Automation PLC Input modules). The Sigma-Delta ADC is intended
> + primarily for measurement of signals close to DC but also delivers
> + outstanding performance with input bandwidths out to ~10kHz.
> +
> + Datasheets for supported chips:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad7172-2
> + - adi,ad7173-8
> + - adi,ad7175-2
> + - adi,ad7176-2
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + description: |
> + Ready interrupt: multiplexed with SPI data out. While SPI CS is low,
> + can be used to indicate the completion of a conversion.
> +
> + Error: The three error bits in the status register (ADC_ERROR, CRC_ERROR,
> + and REG_ERROR) are OR'ed, inverted, and mapped to the ERROR pin. Therefore,
> + the ERROR pin indicates that an error has occurred.
Instead of having a single description like this, you can create an
items list with two descriptions, one for each interrupt.
> +
> + interrupt-names:
> + minItems: 1
> + items:
> + - const: rdy
> + - const: err
> + clocks:
> + maxItems: 1
> + description: |
> + Optional external clock source. Can include one clock source: external
> + clock or external crystal.
> +
> + clock-names:
> + enum:
> + - ext-clk
> + - xtal
> +
> + adi,clock-select:
> + description: |
> + Select the ADC clock source. Valid values are:
> + int : Internal oscillator
> + int-out : Internal oscillator with output on XTAL2 pin
> + ext-clk : External clock input on XTAL2 pin
> + xtal : External crystal on XTAL1 and XTAL2 pins
> +
> + $ref: /schemas/types.yaml#/definitions/string
> + enum:
> + - int
> + - int-out
> + - ext-clk
> + - xtal
> + default: int
I am not a fan of properties like this one, that in my view reimplement
things that are supported by the regular clocks properties. I've got
some questions for you so I can understand whether or not this custom
property is required.
Whether or not the ext-clk or xtal is used is known based on
clock-names - why is the custom property required to determine that?
If neither of those clocks are present, then the internal clock would be
used. Choosing to use the internal clock if an external one is provided
sounds to me like a software policy decision made by the operating
system.
Finally, if the ADC has a clock output, why can that not be represented
by making the ADC a clock-controller?
Cheers,
Conor.
On 1/18/24 17:23, Conor Dooley wrote:
> On Thu, Jan 18, 2024 at 02:49:22PM +0200, Dumitru Ceclan wrote:
..
>> + adi,clock-select:
>> + description: |
>> + Select the ADC clock source. Valid values are:
>> + int : Internal oscillator
>> + int-out : Internal oscillator with output on XTAL2 pin
>> + ext-clk : External clock input on XTAL2 pin
>> + xtal : External crystal on XTAL1 and XTAL2 pins
>> +
>> + $ref: /schemas/types.yaml#/definitions/string
>> + enum:
>> + - int
>> + - int-out
>> + - ext-clk
>> + - xtal
>> + default: int
> I am not a fan of properties like this one, that in my view reimplement
> things that are supported by the regular clocks properties. I've got
> some questions for you so I can understand whether or not this custom
> property is required.
>
> Whether or not the ext-clk or xtal is used is known based on
> clock-names - why is the custom property required to determine that?
> If neither of those clocks are present, then the internal clock would be
> used. Choosing to use the internal clock if an external one is provided
> sounds to me like a software policy decision made by the operating
> system.
If there was no int-out, sure. I considered that the choice between int
and int-out could be made here. So better for driver to choose int/int-out?
>
> Finally, if the ADC has a clock output, why can that not be represented
> by making the ADC a clock-controller?
>
Was not familiar with this/did not cross my mind. So if xtal/ext-clk is
present, the driver should detect it and disable the option for clock
output? (Common pin XTAL2)
On Thu, Jan 18, 2024 at 05:51:20PM +0200, Ceclan Dumitru wrote:
>
>
> On 1/18/24 17:23, Conor Dooley wrote:
> > On Thu, Jan 18, 2024 at 02:49:22PM +0200, Dumitru Ceclan wrote:
>
> ...
>
> >> + adi,clock-select:
> >> + description: |
> >> + Select the ADC clock source. Valid values are:
> >> + int : Internal oscillator
> >> + int-out : Internal oscillator with output on XTAL2 pin
> >> + ext-clk : External clock input on XTAL2 pin
> >> + xtal : External crystal on XTAL1 and XTAL2 pins
> >> +
> >> + $ref: /schemas/types.yaml#/definitions/string
> >> + enum:
> >> + - int
> >> + - int-out
> >> + - ext-clk
> >> + - xtal
> >> + default: int
> > I am not a fan of properties like this one, that in my view reimplement
> > things that are supported by the regular clocks properties. I've got
> > some questions for you so I can understand whether or not this custom
> > property is required.
> >
> > Whether or not the ext-clk or xtal is used is known based on
> > clock-names - why is the custom property required to determine that?
> > If neither of those clocks are present, then the internal clock would be
> > used. Choosing to use the internal clock if an external one is provided
> > sounds to me like a software policy decision made by the operating
> > system.
>
> If there was no int-out, sure. I considered that the choice between int
> and int-out could be made here. So better for driver to choose int/int-out?
This part of my comments was specifically about choosing between use of
the internal clock when ext-clk or xtal are provided, which I think
excludes the possibility of using int-out, since the XTAL2 pin is an
input.
There's 3 situations:
- no external clock provided
- ext-clk provided
- xtal provided
For the former, you know you're in that state when no "clocks" property
is present. The latter two you can differentiate based on "clock-names".
Choosing to use the internal clock if an external clock is provided
seems to be a software policy decision, unless I am mistaken.
> >
> > Finally, if the ADC has a clock output, why can that not be represented
> > by making the ADC a clock-controller?
> >
>
> Was not familiar with this/did not cross my mind. So if xtal/ext-clk is
> present, the driver should detect it and disable the option for clock
> output? (Common pin XTAL2)
Yeah, if those clocks are provided you would not register as a clock
controller. If there is a user of the output clock, it should have its
own "clocks" property that references the ADC's output.
Your dt-binding could also make clocks/clock-names & clock-controller
mutually exclusive.
Cheers,
Conor.
On Thu, Jan 18, 2024 at 6:50 AM Dumitru Ceclan <mitrutzceclan@gmailcom> wrote:
>
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
>
> Signed-off-by: Dumitru Ceclan <[email protected]>
> ---
>
> V11->V12
> - Drop "binding", describe hardware in binding description
> - Rename refin and refin2 to vref and vref2
> - Add better description to external references to better show that the voltage
> value that needs to be specified is the difference between the positive and
> negative reference pins
> - Add optional clocks properties
> - Add optional adi,clock-select property
> - Add option for second interrupt, error
> - Add description to interrupts
> V10->V11
> - Fix example warning: '#gpio-cells' is a dependency of 'gpio-controller'
> - Add description to #gpio-cells property
> V9->V10
> - Fix dt_binding_check type warning from adi,reference-select
> V8->v9
> - Add gpio-controller and "#gpio-cells" properties
> - Add missing avdd2 and iovdd supplies
> - Add string type to reference-select
> V7->V8
> - include missing fix from V6
> V6->V7 <no changes>
> V5->V6
> - Moved global required property to proper placement
> V4 -> V5
> - Use string enum instead of integers for "adi,reference-select"
> - Fix conditional checking in regards to compatible
> V3 -> V4
> - include supply attributes
> - add channel attribute for selecting conversion reference
> V2 -> V3
> - remove redundant descriptions
> - use referenced 'bipolar' property
> - remove newlines from example
> V1 -> V2 <no changes>
>
> .../bindings/iio/adc/adi,ad7173.yaml | 242 ++++++++++++++++++
> 1 file changed, 242 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> new file mode 100644
> index 000000000000..4d0870cc014c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> @@ -0,0 +1,242 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7173 ADC
> +
> +maintainers:
> + - Ceclan Dumitru <[email protected]>
> +
> +description: |
> + Analog Devices AD717x ADC's:
> + The AD717x family offer a complete integrated Sigma-Delta ADC solution which
> + can be used in high precision, low noise single channel applications
> + (Life Science measurements) or higher speed multiplexed applications
> + (Factory Automation PLC Input modules). The Sigma-Delta ADC is intended
> + primarily for measurement of signals close to DC but also delivers
> + outstanding performance with input bandwidths out to ~10kHz.
> +
> + Datasheets for supported chips:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad7172-2
> + - adi,ad7173-8
> + - adi,ad7175-2
> + - adi,ad7176-2
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + description: |
> + Ready interrupt: multiplexed with SPI data out. While SPI CS is low,
> + can be used to indicate the completion of a conversion.
> +
> + Error: The three error bits in the status register (ADC_ERROR, CRC_ERROR,
> + and REG_ERROR) are OR'ed, inverted, and mapped to the ERROR pin. Therefore,
> + the ERROR pin indicates that an error has occurred.
> +
> + interrupt-names:
> + minItems: 1
> + items:
> + - const: rdy
> + - const: err
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + spi-max-frequency:
> + maximum: 20000000
> +
> + gpio-controller:
> + description: Marks the device node as a GPIO controller.
> +
> + "#gpio-cells":
> + const: 2
> + description:
> + The first cell is the GPIO number and the second cell specifies
> + GPIO flags, as defined in <dt-bindings/gpio/gpio.h>.
> +
> + vref-supply:
> + description: |
> + Differential external reference supply used for conversion. The reference
> + voltage (Vref) specified here must be the voltage difference between the
> + REF+ and REF- pins: Vref = (REF+) - (REF-).
> +
> + vref2-supply:
> + description: |
> + Differential external reference supply used for conversion. The reference
> + voltage (Vref2) specified here must be the voltage difference between the
> + REF2+ and REF2- pins: Vref2 = (REF2+) - (REF2-).
> +
> + avdd-supply:
> + description: avdd supply, can be used as reference for conversion.
I think it would be helpful to have a description similar to
vref-supply here. This is the voltage between AVDD and AVSS. So in
both cases AVDD=5V, AVSS=0V and AVDD=+2.5V, AVSS=-2.5V, this supply
should report 5V.
> +
> + avdd2-supply:
> + description: avdd2 supply, used as the input to the internal voltage regulator.
This supply is also referenced to AVSS.
> +
> + iovdd-supply:
> + description: iovdd supply, used for the chip digital interface.
> +
> + clocks:
> + maxItems: 1
> + description: |
> + Optional external clock source. Can include one clock source: external
> + clock or external crystal.
> +
> + clock-names:
> + enum:
> + - ext-clk
> + - xtal
> +
> + adi,clock-select:
> + description: |
> + Select the ADC clock source. Valid values are:
> + int : Internal oscillator
> + int-out : Internal oscillator with output on XTAL2 pin
> + ext-clk : External clock input on XTAL2 pin
> + xtal : External crystal on XTAL1 and XTAL2 pins
> +
> + $ref: /schemas/types.yaml#/definitions/string
> + enum:
> + - int
> + - int-out
> + - ext-clk
> + - xtal
> + default: int
> +
> +patternProperties:
> + "^channel@[0-9a-f]$":
> + type: object
> + $ref: adc.yaml
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 15
> +
> + diff-channels:
> + items:
> + minimum: 0
> + maximum: 31
> +
> + adi,reference-select:
> + description: |
> + Select the reference source to use when converting on
> + the specific channel. Valid values are:
> + vref : REF+ /REF−
> + vref2 : REF2+ /REF2−
> + refout-avss: REFOUT/AVSS (Internal reference)
> + avdd : AVDD
Could write this as AVDD/AVSS to be consistent with the other 3 options.
(Or if this is really AVDD to 0V, we may need to reconsider some of
our other decisions.)
> +
> + External reference ref2 only available on ad7173-8.
> + If not specified, internal reference used.
> + $ref: /schemas/types.yaml#/definitions/string
> + enum:
> + - vref
> + - vref2
> + - refout-avss
> + - avdd
> + default: refout-avss
> +
> + required:
> + - reg
> + - diff-channels
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> + - if:
> + properties:
> + compatible:
> + not:
> + contains:
> + const: adi,ad7173-8
> + then:
> + properties:
> + vref2-supply: false
> + patternProperties:
> + "^channel@[0-9a-f]$":
> + properties:
> + adi,reference-select:
> + enum:
> + - vref
> + - refout-avss
> + - avdd
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,ad7173-8";
> + reg = <0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-names = "rdy";
> + interrupt-parent = <&gpio>;
> + spi-max-frequency = <5000000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> + vref-supply = <&dummy_regulator>;
> +
> + channel@0 {
> + reg = <0>;
> + bipolar;
> + diff-channels = <0 1>;
> + adi,reference-select = "vref";
> + };
> +
> + channel@1 {
> + reg = <1>;
> + diff-channels = <2 3>;
> + };
> +
> + channel@2 {
> + reg = <2>;
> + bipolar;
> + diff-channels = <4 5>;
> + };
> +
> + channel@3 {
> + reg = <3>;
> + bipolar;
> + diff-channels = <6 7>;
> + };
> +
> + channel@4 {
> + reg = <4>;
> + diff-channels = <8 9>;
> + adi,reference-select = "avdd";
> + };
> + };
> + };
> --
> 2.42.0
>
On Thu, 18 Jan 2024 16:06:29 +0000
Conor Dooley <[email protected]> wrote:
> On Thu, Jan 18, 2024 at 05:51:20PM +0200, Ceclan Dumitru wrote:
> >
> >
> > On 1/18/24 17:23, Conor Dooley wrote:
> > > On Thu, Jan 18, 2024 at 02:49:22PM +0200, Dumitru Ceclan wrote:
> >
> > ...
> >
> > >> + adi,clock-select:
> > >> + description: |
> > >> + Select the ADC clock source. Valid values are:
> > >> + int : Internal oscillator
> > >> + int-out : Internal oscillator with output on XTAL2 pin
> > >> + ext-clk : External clock input on XTAL2 pin
> > >> + xtal : External crystal on XTAL1 and XTAL2 pins
> > >> +
> > >> + $ref: /schemas/types.yaml#/definitions/string
> > >> + enum:
> > >> + - int
> > >> + - int-out
> > >> + - ext-clk
> > >> + - xtal
> > >> + default: int
> > > I am not a fan of properties like this one, that in my view reimplement
> > > things that are supported by the regular clocks properties. I've got
> > > some questions for you so I can understand whether or not this custom
> > > property is required.
> > >
> > > Whether or not the ext-clk or xtal is used is known based on
> > > clock-names - why is the custom property required to determine that?
>
> > > If neither of those clocks are present, then the internal clock would be
> > > used. Choosing to use the internal clock if an external one is provided
> > > sounds to me like a software policy decision made by the operating
> > > system.
> >
> > If there was no int-out, sure. I considered that the choice between int
> > and int-out could be made here. So better for driver to choose int/int-out?
>
> This part of my comments was specifically about choosing between use of
> the internal clock when ext-clk or xtal are provided, which I think
> excludes the possibility of using int-out, since the XTAL2 pin is an
> input.
>
> There's 3 situations:
> - no external clock provided
> - ext-clk provided
> - xtal provided
>
> For the former, you know you're in that state when no "clocks" property
> is present. The latter two you can differentiate based on "clock-names".
>
> Choosing to use the internal clock if an external clock is provided
> seems to be a software policy decision, unless I am mistaken.
Agreed, though it rarely makes sense as if someone put down a precision
clock they normally wanted you to use it!
So as a general rule we don't both providing policy controls beyond if
there is extra hardware (external clock source) then use that.
If someone has a good reason to want to do something else then we can
probably figure out a reasonable way to control it.
>
> > >
> > > Finally, if the ADC has a clock output, why can that not be represented
> > > by making the ADC a clock-controller?
> > >
> >
> > Was not familiar with this/did not cross my mind. So if xtal/ext-clk is
> > present, the driver should detect it and disable the option for clock
> > output? (Common pin XTAL2)
>
> Yeah, if those clocks are provided you would not register as a clock
> controller. If there is a user of the output clock, it should have its
> own "clocks" property that references the ADC's output.
>
> Your dt-binding could also make clocks/clock-names & clock-controller
> mutually exclusive.
That would indeed be the nicest solution. How this has been done
in drivers has somewhat 'evolved' over time, but this is the nicest
option from point of view of standard bindings and clarity over what
is going on.
>
> Cheers,
> Conor.
On Sun, Jan 21, 2024 at 03:41:18PM +0000, Jonathan Cameron wrote:
> On Thu, 18 Jan 2024 16:06:29 +0000
> Conor Dooley <[email protected]> wrote:
> > On Thu, Jan 18, 2024 at 05:51:20PM +0200, Ceclan Dumitru wrote:
> > > On 1/18/24 17:23, Conor Dooley wrote:
> > > > On Thu, Jan 18, 2024 at 02:49:22PM +0200, Dumitru Ceclan wrote:
> > > >> + adi,clock-select:
> > > >> + description: |
> > > >> + Select the ADC clock source. Valid values are:
> > > >> + int : Internal oscillator
> > > >> + int-out : Internal oscillator with output on XTAL2 pin
> > > >> + ext-clk : External clock input on XTAL2 pin
> > > >> + xtal : External crystal on XTAL1 and XTAL2 pins
> > > >> +
> > > >> + $ref: /schemas/types.yaml#/definitions/string
> > > >> + enum:
> > > >> + - int
> > > >> + - int-out
> > > >> + - ext-clk
> > > >> + - xtal
> > > >> + default: int
> > > > I am not a fan of properties like this one, that in my view reimplement
> > > > things that are supported by the regular clocks properties. I've got
> > > > some questions for you so I can understand whether or not this custom
> > > > property is required.
> > > >
> > > > Whether or not the ext-clk or xtal is used is known based on
> > > > clock-names - why is the custom property required to determine that?
> >
> > > > If neither of those clocks are present, then the internal clock would be
> > > > used. Choosing to use the internal clock if an external one is provided
> > > > sounds to me like a software policy decision made by the operating
> > > > system.
> > >
> > > If there was no int-out, sure. I considered that the choice between int
> > > and int-out could be made here. So better for driver to choose int/int-out?
> >
> > This part of my comments was specifically about choosing between use of
> > the internal clock when ext-clk or xtal are provided, which I think
> > excludes the possibility of using int-out, since the XTAL2 pin is an
> > input.
> >
> > There's 3 situations:
> > - no external clock provided
> > - ext-clk provided
> > - xtal provided
> >
> > For the former, you know you're in that state when no "clocks" property
> > is present. The latter two you can differentiate based on "clock-names".
> >
> > Choosing to use the internal clock if an external clock is provided
> > seems to be a software policy decision, unless I am mistaken.
>
> Agreed, though it rarely makes sense as if someone put down a precision
> clock they normally wanted you to use it!
>
> So as a general rule we don't both providing policy controls beyond if
> there is extra hardware (external clock source) then use that.
>
> If someone has a good reason to want to do something else then we can
> probably figure out a reasonable way to control it.
Yah, I figured there'd be few situations, outside of maybe debugging
hardware issues, where that internal clock is more desirable to use.
> > > > Finally, if the ADC has a clock output, why can that not be represented
> > > > by making the ADC a clock-controller?
> > > >
> > >
> > > Was not familiar with this/did not cross my mind. So if xtal/ext-clk is
> > > present, the driver should detect it and disable the option for clock
> > > output? (Common pin XTAL2)
> >
> > Yeah, if those clocks are provided you would not register as a clock
> > controller. If there is a user of the output clock, it should have its
> > own "clocks" property that references the ADC's output.
> >
> > Your dt-binding could also make clocks/clock-names & clock-controller
> > mutually exclusive.
>
> That would indeed be the nicest solution. How this has been done
> in drivers has somewhat 'evolved' over time, but this is the nicest
> option from point of view of standard bindings and clarity over what
> is going on.
Yeah, I know that this has not really been normal thing to do in some
corners of the kernel (ethernet PHYs in particular I think have been
rolling their own clock controller stuff) and I've been trying to push
back on these kinds of things for new devices.
Thanks,
Conor.