| Message ID | 20240702030025.57078-5-kimseer.paller@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add driver for LTC2664 and LTC2672 | expand |
On Tue, Jul 02, 2024 at 11:00:23AM +0800, Kim Seer Paller wrote: > + adi,manual-span-operation-config: > + description: > + This property must mimic the MSPAN pin configurations. By tying the MSPAN > + pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can be > + hardware-configured with different mid-scale or zero-scale reset options. > + The hardware configuration is latched during power on reset for proper > + operation. > + 0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V) > + 1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V) > + 2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V) > + 3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V) > + 4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V) > + 5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V) > + 6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V) > + 7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables SoftSpan) > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3, 4, 5, 6, 7] > + default: 7 I still don't like this property, but I still don't really understand some of the iteraction between it and the output ranges and cannot suggest anything better, so Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
On 7/1/24 10:00 PM, Kim Seer Paller wrote: > Add documentation for ltc2664. > ... > + adi,manual-span-operation-config: > + description: > + This property must mimic the MSPAN pin configurations. By tying the MSPAN > + pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can be > + hardware-configured with different mid-scale or zero-scale reset options. > + The hardware configuration is latched during power on reset for proper > + operation. > + 0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V) > + 1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V) > + 2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V) > + 3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V) > + 4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V) > + 5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V) > + 6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V) > + 7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables SoftSpan) > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3, 4, 5, 6, 7] > + default: 7 > + > + io-channels: > + description: > + ADC channel to monitor voltages and temperature at the MUXOUT pin. > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > +patternProperties: > + "^channel@[0-3]$": > + $ref: dac.yaml > + type: object > + additionalProperties: false > + > + properties: > + reg: > + description: The channel number representing the DAC output channel. > + maximum: 3 > + > + adi,toggle-mode: > + description: > + Set the channel as a toggle enabled channel. Toggle operation enables > + fast switching of a DAC output between two different DAC codes without > + any SPI transaction. > + type: boolean > + > + output-range-microvolt: Could be helpful to add a description that says this property is only allowed when SoftSpan is enabled rather than requiring people to reason through the logic. > + oneOf: > + - items: > + - const: 0 > + - enum: [5000000, 10000000] > + - items: > + - const: -5000000 > + - const: 5000000 > + - items: > + - const: -10000000 > + - const: 10000000 > + - items: > + - const: -2500000 > + - const: 2500000 default: [0, 5000000] > + > + required: > + - reg > + > + allOf: > + - if: > + properties: > + adi,manual-span-operation-config: > + const: 7 > + then: > + patternProperties: > + "^channel@[0-3]$": > + required: [output-range-microvolt] This logic doesn't look right to me. If adi,manual-span-operation-config is not 7, then SoftSpan is disabled, so we should have: output-range-microvolt: false In that case since individual channels can't have a per-channel configuration because SoftSpan is not enabled (unless I am misunderstanding the datasheet?). Also, output-range-microvolt should never be required, even when adi,manual-span-operation-config is 7 because there is already a default value range (0V to 5V) specified by the adi,manual-span-operation-config property. I think the correct logic would be: - if: not: properties: adi,manual-span-operation-config: const: 7 then: patternProperties: "^channel@[0-3]$": properties: output-range-microvolt: false
> -----Original Message----- > From: David Lechner <dlechner@baylibre.com> > Sent: Tuesday, July 2, 2024 11:36 PM > To: Paller, Kim Seer <KimSeer.Paller@analog.com>; linux- > kernel@vger.kernel.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org > Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen > <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown > <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>; Krzysztof > Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; Conor > Dooley <conor+dt@kernel.org>; Hennerich, Michael > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com> > Subject: Re: [PATCH v5 4/6] dt-bindings: iio: dac: Add adi,ltc2664.yaml > > [External] > > On 7/1/24 10:00 PM, Kim Seer Paller wrote: > > Add documentation for ltc2664. > > > > ... > > > + adi,manual-span-operation-config: > > + description: > > + This property must mimic the MSPAN pin configurations. By tying the > MSPAN > > + pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can > be > > + hardware-configured with different mid-scale or zero-scale reset > options. > > + The hardware configuration is latched during power on reset for proper > > + operation. > > + 0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V) > > + 1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V) > > + 2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V) > > + 3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V) > > + 4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V) > > + 5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V) > > + 6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V) > > + 7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables > SoftSpan) > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2, 3, 4, 5, 6, 7] > > + default: 7 > > + > > + io-channels: > > + description: > > + ADC channel to monitor voltages and temperature at the MUXOUT pin. > > + maxItems: 1 > > + > > + '#address-cells': > > + const: 1 > > + > > + '#size-cells': > > + const: 0 > > + > > +patternProperties: > > + "^channel@[0-3]$": > > + $ref: dac.yaml > > + type: object > > + additionalProperties: false > > + > > + properties: > > + reg: > > + description: The channel number representing the DAC output channel. > > + maximum: 3 > > + > > + adi,toggle-mode: > > + description: > > + Set the channel as a toggle enabled channel. Toggle operation enables > > + fast switching of a DAC output between two different DAC codes > without > > + any SPI transaction. > > + type: boolean > > + > > + output-range-microvolt: > > Could be helpful to add a description that says this property is only allowed > when > SoftSpan is enabled rather than requiring people to reason through the logic. > > > + oneOf: > > + - items: > > + - const: 0 > > + - enum: [5000000, 10000000] > > + - items: > > + - const: -5000000 > > + - const: 5000000 > > + - items: > > + - const: -10000000 > > + - const: 10000000 > > + - items: > > + - const: -2500000 > > + - const: 2500000 > > default: [0, 5000000] Adding a default value directly within the schema causes validation error. Given this, is it acceptable documenting the intended default value within the description? > > > + > > + required: > > + - reg > > + > > + allOf: > > + - if: > > + properties: > > + adi,manual-span-operation-config: > > + const: 7 > > + then: > > + patternProperties: > > + "^channel@[0-3]$": > > + required: [output-range-microvolt] > > > This logic doesn't look right to me. If adi,manual-span-operation-config > is not 7, then SoftSpan is disabled, so we should have: > > output-range-microvolt: false > > In that case since individual channels can't have a per-channel > configuration because SoftSpan is not enabled (unless I am misunderstanding > the datasheet?). > > Also, output-range-microvolt should never be required, even when > adi,manual-span-operation-config is 7 because there is already a default > value range (0V to 5V) specified by the adi,manual-span-operation-config > property. > > I think the correct logic would be: > > - if: > not: > properties: > adi,manual-span-operation-config: > const: 7 > then: > patternProperties: > "^channel@[0-3]$": > properties: > output-range-microvolt: false
... > > > + adi,manual-span-operation-config: > > > + description: > > > + This property must mimic the MSPAN pin configurations. By > > > + tying the > > MSPAN > > > + pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output > > > + range can > > be > > > + hardware-configured with different mid-scale or zero-scale > > > + reset > > options. > > > + The hardware configuration is latched during power on reset for proper > > > + operation. > > > + 0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V) > > > + 1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V) > > > + 2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V) > > > + 3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V) > > > + 4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V) > > > + 5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V) > > > + 6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V) > > > + 7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, > > > + enables > > SoftSpan) > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + enum: [0, 1, 2, 3, 4, 5, 6, 7] > > > + default: 7 > > > + > > > + io-channels: > > > + description: > > > + ADC channel to monitor voltages and temperature at the MUXOUT pin. > > > + maxItems: 1 > > > + > > > + '#address-cells': > > > + const: 1 > > > + > > > + '#size-cells': > > > + const: 0 > > > + > > > +patternProperties: > > > + "^channel@[0-3]$": > > > + $ref: dac.yaml > > > + type: object > > > + additionalProperties: false > > > + > > > + properties: > > > + reg: > > > + description: The channel number representing the DAC output > channel. > > > + maximum: 3 > > > + > > > + adi,toggle-mode: > > > + description: > > > + Set the channel as a toggle enabled channel. Toggle operation > enables > > > + fast switching of a DAC output between two different DAC > > > + codes > > without > > > + any SPI transaction. > > > + type: boolean > > > + > > > + output-range-microvolt: > > > > Could be helpful to add a description that says this property is only > > allowed when SoftSpan is enabled rather than requiring people to > > reason through the logic. > > > > > + oneOf: > > > + - items: > > > + - const: 0 > > > + - enum: [5000000, 10000000] > > > + - items: > > > + - const: -5000000 > > > + - const: 5000000 > > > + - items: > > > + - const: -10000000 > > > + - const: 10000000 > > > + - items: > > > + - const: -2500000 > > > + - const: 2500000 > > > > default: [0, 5000000] > > Adding a default value directly within the schema causes validation error. > Given this, is it acceptable documenting the intended default value within the > description? Before sending the patch, I would like to confirm if it is acceptable to just document the intended default value within the description, since adding a default value directly to the schema causes validation error and considering adding the logic, - if: not: properties: adi,manual-span-operation-config: const: 7 then: patternProperties: "^channel@[0-3]$": properties: output-range-microvolt: false > > > > > + > > > + required: > > > + - reg > > > + > > > + allOf: > > > + - if: > > > + properties: > > > + adi,manual-span-operation-config: > > > + const: 7 > > > + then: > > > + patternProperties: > > > + "^channel@[0-3]$": > > > + required: [output-range-microvolt] > > > > > > This logic doesn't look right to me. If > > adi,manual-span-operation-config is not 7, then SoftSpan is disabled, so we > should have: > > > > output-range-microvolt: false > > > > In that case since individual channels can't have a per-channel > > configuration because SoftSpan is not enabled (unless I am > > misunderstanding the datasheet?). > > > > Also, output-range-microvolt should never be required, even when > > adi,manual-span-operation-config is 7 because there is already a > > default value range (0V to 5V) specified by the > > adi,manual-span-operation-config property. > > > > I think the correct logic would be: > > > > - if: > > not: > > properties: > > adi,manual-span-operation-config: > > const: 7 > > then: > > patternProperties: > > "^channel@[0-3]$": > > properties: > > output-range-microvolt: false
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml new file mode 100644 index 000000000000..51c2e8502b9c --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml @@ -0,0 +1,176 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2664.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2664 DAC + +maintainers: + - Michael Hennerich <michael.hennerich@analog.com> + - Kim Seer Paller <kimseer.paller@analog.com> + +description: | + Analog Devices LTC2664 4 channel, 12-/16-Bit, +-10V DAC + https://www.analog.com/media/en/technical-documentation/data-sheets/2664fa.pdf + +properties: + compatible: + enum: + - adi,ltc2664 + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 50000000 + + vcc-supply: + description: Analog Supply Voltage Input. + + v-pos-supply: + description: Positive Supply Voltage Input. + + v-neg-supply: + description: Negative Supply Voltage Input. + + iovcc-supply: + description: Digital Input/Output Supply Voltage. + + ref-supply: + description: + Reference Input/Output. The voltage at the REF pin sets the full-scale + range of all channels. If not provided the internal reference is used and + also provided on the VREF pin. + + reset-gpios: + description: + Active-low Asynchronous Clear Input. A logic low at this level-triggered + input clears the part to the reset code and range determined by the + hardwired option chosen using the MSPAN pins. The control registers are + cleared to zero. + maxItems: 1 + + adi,manual-span-operation-config: + description: + This property must mimic the MSPAN pin configurations. By tying the MSPAN + pins (MSP2, MSP1 and MSP0) to GND and/or VCC, any output range can be + hardware-configured with different mid-scale or zero-scale reset options. + The hardware configuration is latched during power on reset for proper + operation. + 0 - MPS2=GND, MPS1=GND, MSP0=GND (+-10V, reset to 0V) + 1 - MPS2=GND, MPS1=GND, MSP0=VCC (+-5V, reset to 0V) + 2 - MPS2=GND, MPS1=VCC, MSP0=GND (+-2.5V, reset to 0V) + 3 - MPS2=GND, MPS1=VCC, MSP0=VCC (0V to 10, reset to 0V) + 4 - MPS2=VCC, MPS1=GND, MSP0=GND (0V to 10V, reset to 5V) + 5 - MPS2=VCC, MPS1=GND, MSP0=VCC (0V to 5V, reset to 0V) + 6 - MPS2=VCC, MPS1=VCC, MSP0=GND (0V to 5V, reset to 2.5V) + 7 - MPS2=VCC, MPS1=VCC, MSP0=VCC (0V to 5V, reset to 0V, enables SoftSpan) + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3, 4, 5, 6, 7] + default: 7 + + io-channels: + description: + ADC channel to monitor voltages and temperature at the MUXOUT pin. + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + +patternProperties: + "^channel@[0-3]$": + $ref: dac.yaml + type: object + additionalProperties: false + + properties: + reg: + description: The channel number representing the DAC output channel. + maximum: 3 + + adi,toggle-mode: + description: + Set the channel as a toggle enabled channel. Toggle operation enables + fast switching of a DAC output between two different DAC codes without + any SPI transaction. + type: boolean + + output-range-microvolt: + oneOf: + - items: + - const: 0 + - enum: [5000000, 10000000] + - items: + - const: -5000000 + - const: 5000000 + - items: + - const: -10000000 + - const: 10000000 + - items: + - const: -2500000 + - const: 2500000 + + required: + - reg + + allOf: + - if: + properties: + adi,manual-span-operation-config: + const: 7 + then: + patternProperties: + "^channel@[0-3]$": + required: [output-range-microvolt] + +required: + - compatible + - reg + - spi-max-frequency + - vcc-supply + - iovcc-supply + - v-pos-supply + - v-neg-supply + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +additionalProperties: false + +examples: + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + dac@0 { + compatible = "adi,ltc2664"; + reg = <0>; + spi-max-frequency = <10000000>; + + vcc-supply = <&vcc>; + iovcc-supply = <&vcc>; + ref-supply = <&vref>; + v-pos-supply = <&vpos>; + v-neg-supply = <&vneg>; + + io-channels = <&adc 0>; + + #address-cells = <1>; + #size-cells = <0>; + channel@0 { + reg = <0>; + adi,toggle-mode; + output-range-microvolt = <(-10000000) 10000000>; + }; + + channel@1 { + reg = <1>; + output-range-microvolt= <0 10000000>; + }; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 0cbeb03847f5..2629e818ffdc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13080,6 +13080,14 @@ S: Maintained F: Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.yaml F: drivers/iio/dac/ltc1660.c +LTC2664 IIO DAC DRIVER +M: Michael Hennerich <michael.hennerich@analog.com> +M: Kim Seer Paller <kimseer.paller@analog.com> +L: linux-iio@vger.kernel.org +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml + LTC2688 IIO DAC DRIVER M: Nuno Sá <nuno.sa@analog.com> L: linux-iio@vger.kernel.org