| Message ID | 20240603012200.16589-5-kimseer.paller@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add driver for LTC2664 and LTC2672 | expand |
On 03/06/2024 03:21, Kim Seer Paller wrote: > Add documentation for ltc2672. > > Reported-by: Rob Herring (Arm) <robh@kernel.org> ??? There was no bug report telling you the binding is missing. Drop. > Closes: https://lore.kernel.org/all/171643825573.1037396.2749703571529285460.robh@kernel.org/ Drop > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > --- > .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 159 insertions(+) With these two fixes: Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 6/2/24 8:21 PM, Kim Seer Paller wrote: > Add documentation for ltc2672. > > Reported-by: Rob Herring (Arm) <robh@kernel.org> > Closes: https://lore.kernel.org/all/171643825573.1037396.2749703571529285460.robh@kernel.org/ > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > --- > .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 159 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > new file mode 100644 > index 000000000000..d143a9db7010 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > @@ -0,0 +1,158 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices LTC2672 DAC > + > +maintainers: > + - Michael Hennerich <michael.hennerich@analog.com> > + - Kim Seer Paller <kimseer.paller@analog.com> > + > +description: | > + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC > + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf > + > +properties: > + compatible: > + enum: > + - adi,ltc2672 The linked datasheet describes 12-bit and 16-bit versions, so should we have two compatibles here? adi,ltc2672-12, adi,ltc2672-16 I don't see any ID registers where this could be read from the chip at runtime, so seems like something that needs to be in the devicetree.
On 6/3/24 2:59 PM, David Lechner wrote: > On 6/2/24 8:21 PM, Kim Seer Paller wrote: >> Add documentation for ltc2672. >> >> Reported-by: Rob Herring (Arm) <robh@kernel.org> >> Closes: https://lore.kernel.org/all/171643825573.1037396.2749703571529285460.robh@kernel.org/ >> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> >> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> >> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> >> --- >> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 159 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >> >> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >> new file mode 100644 >> index 000000000000..d143a9db7010 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >> @@ -0,0 +1,158 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Analog Devices LTC2672 DAC >> + >> +maintainers: >> + - Michael Hennerich <michael.hennerich@analog.com> >> + - Kim Seer Paller <kimseer.paller@analog.com> >> + >> +description: | >> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC >> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf >> + >> +properties: >> + compatible: >> + enum: >> + - adi,ltc2672 > > The linked datasheet describes 12-bit and 16-bit versions, so should we have > two compatibles here? adi,ltc2672-12, adi,ltc2672-16 > > I don't see any ID registers where this could be read from the chip at > runtime, so seems like something that needs to be in the devicetree. Hmm... I guess maybe it doesn't matter for these chips (i.e. the 4 LSBs of the sample data register in 12-bit version will just always be ignored but no data needs to be shifted based on the bit-ness). I would not hurt to update the description though since it only mentions 16-bit if the compatible is meant for both versions of the chip.
On 03/06/2024 21:59, David Lechner wrote: > On 6/2/24 8:21 PM, Kim Seer Paller wrote: >> Add documentation for ltc2672. >> >> Reported-by: Rob Herring (Arm) <robh@kernel.org> >> Closes: https://lore.kernel.org/all/171643825573.1037396.2749703571529285460.robh@kernel.org/ >> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> >> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> >> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> >> --- >> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 159 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >> >> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >> new file mode 100644 >> index 000000000000..d143a9db7010 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >> @@ -0,0 +1,158 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Analog Devices LTC2672 DAC >> + >> +maintainers: >> + - Michael Hennerich <michael.hennerich@analog.com> >> + - Kim Seer Paller <kimseer.paller@analog.com> >> + >> +description: | >> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC >> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf >> + >> +properties: >> + compatible: >> + enum: >> + - adi,ltc2672 > > The linked datasheet describes 12-bit and 16-bit versions, so should we have > two compatibles here? adi,ltc2672-12, adi,ltc2672-16 Is their programming model different? Best regards, Krzysztof
On 6/4/24 1:47 AM, Krzysztof Kozlowski wrote: > On 03/06/2024 21:59, David Lechner wrote: >> On 6/2/24 8:21 PM, Kim Seer Paller wrote: >>> Add documentation for ltc2672. >>> >>> Reported-by: Rob Herring (Arm) <robh@kernel.org> >>> Closes: https://lore.kernel.org/all/171643825573.1037396.2749703571529285460.robh@kernel.org/ >>> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> >>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> >>> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> >>> --- >>> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++ >>> MAINTAINERS | 1 + >>> 2 files changed, 159 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >>> new file mode 100644 >>> index 000000000000..d143a9db7010 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml >>> @@ -0,0 +1,158 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Analog Devices LTC2672 DAC >>> + >>> +maintainers: >>> + - Michael Hennerich <michael.hennerich@analog.com> >>> + - Kim Seer Paller <kimseer.paller@analog.com> >>> + >>> +description: | >>> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC >>> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - adi,ltc2672 >> >> The linked datasheet describes 12-bit and 16-bit versions, so should we have >> two compatibles here? adi,ltc2672-12, adi,ltc2672-16 > > Is their programming model different? > I replied to myself already with the answer. After looking at it more it does not appear that is the case.
On Tue, 4 Jun 2024 08:53:27 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/4/24 1:47 AM, Krzysztof Kozlowski wrote: > > On 03/06/2024 21:59, David Lechner wrote: > >> On 6/2/24 8:21 PM, Kim Seer Paller wrote: > >>> Add documentation for ltc2672. > >>> > >>> Reported-by: Rob Herring (Arm) <robh@kernel.org> > >>> Closes: https://lore.kernel.org/all/171643825573.1037396.2749703571529285460.robh@kernel.org/ > >>> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > >>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > >>> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > >>> --- > >>> .../bindings/iio/dac/adi,ltc2672.yaml | 158 ++++++++++++++++++ > >>> MAINTAINERS | 1 + > >>> 2 files changed, 159 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > >>> new file mode 100644 > >>> index 000000000000..d143a9db7010 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml > >>> @@ -0,0 +1,158 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Analog Devices LTC2672 DAC > >>> + > >>> +maintainers: > >>> + - Michael Hennerich <michael.hennerich@analog.com> > >>> + - Kim Seer Paller <kimseer.paller@analog.com> > >>> + > >>> +description: | > >>> + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC > >>> + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf > >>> + > >>> +properties: > >>> + compatible: > >>> + enum: > >>> + - adi,ltc2672 > >> > >> The linked datasheet describes 12-bit and 16-bit versions, so should we have > >> two compatibles here? adi,ltc2672-12, adi,ltc2672-16 > > > > Is their programming model different? > > > > I replied to myself already with the answer. After looking at it more it > does not appear that is the case. > For a DAC, this is an interesting question. The wrong impressions of precision might be a problem if someone is trying to tune the value. Say they set it to +15 and look at some other sensor for the affect. They expect to see something but get no change at all. They might assume the circuit is broken. So I think yes the programming model is different and that should be discoverable (ideally from hardware, but if not from the compatible) To take an extreme example of extending the logic of these being the 'same' from a programming model point of view, would we consider a regulator that did 0 and 3V only different from one that did 0V, 1V, 2V, 3V just because the second bit in the register was ignored? I think in that case we'd consider them to have an obviously different programming model. We have a few cases where we do paper over similar differences in resolution, but within one part with different settings rather than between devices (so that's a driver limitation, not a DT thing). So I might be persuaded no one cares, but in my view the programming model is different in a significant way. Jonathan
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml new file mode 100644 index 000000000000..d143a9db7010 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml @@ -0,0 +1,158 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/dac/adi,ltc2672.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2672 DAC + +maintainers: + - Michael Hennerich <michael.hennerich@analog.com> + - Kim Seer Paller <kimseer.paller@analog.com> + +description: | + Analog Devices LTC2672 5 channel, 16 bit, 300mA DAC + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc2672.pdf + +properties: + compatible: + enum: + - adi,ltc2672 + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 50000000 + + vcc-supply: + description: Analog Supply Voltage Input. + + v-neg-supply: + description: Negative Supply Voltage Input. + + vdd0-supply: + description: Positive Supply Voltage Input for DAC OUT0. + + vdd1-supply: + description: Positive Supply Voltage Input for DAC OUT1. + + vdd2-supply: + description: Positive Supply Voltage Input for DAC OUT2. + + vdd3-supply: + description: Positive Supply Voltage Input for DAC OUT3. + + vdd4-supply: + description: Positive Supply Voltage Input for DAC OUT4. + + 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 device to the default reset code and output range, which + is zero-scale with the outputs off. The control registers are cleared to + zero. + maxItems: 1 + + adi,rfsadj-ohms: + description: + If FSADJ is tied to VCC, an internal RFSADJ (20 kΩ) is selected, which + results in nominal output ranges. When an external resistor of 19 kΩ to + 41 kΩ can be used instead by connecting the resistor between FSADJ and GND + it controls the scaling of the ranges, and the internal resistor is + automatically disconnected. + minimum: 19000 + maximum: 41000 + default: 20000 + + io-channels: + description: + ADC channel to monitor voltages and currents at the MUX pin. + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + +patternProperties: + "^channel@[0-4]$": + type: object + additionalProperties: false + + properties: + reg: + description: The channel number representing the DAC output channel. + maximum: 4 + + 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 + + adi,output-range-microamp: + description: Specify the channel output full scale range. + enum: [3125000, 6250000, 12500000, 25000000, 50000000, 100000000, + 200000000, 300000000] + + required: + - reg + - adi,output-range-microamp + +required: + - compatible + - reg + - spi-max-frequency + - vcc-supply + - iovcc-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,ltc2672"; + reg = <0>; + spi-max-frequency = <10000000>; + + vcc-supply = <&vcc>; + iovcc-supply = <&vcc>; + ref-supply = <&vref>; + v-neg-supply = <&vneg>; + + io-channels = <&adc 0>; + + #address-cells = <1>; + #size-cells = <0>; + channel@0 { + reg = <0>; + adi,toggle-mode; + adi,output-range-microamp = <3125000>; + }; + + channel@1 { + reg = <1>; + adi,output-range-microamp = <6250000>; + }; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index c7a102d7fd6a..ac1e29e26f31 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13070,6 +13070,7 @@ 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 +F: Documentation/devicetree/bindings/iio/dac/adi,ltc2672.yaml LTC2688 IIO DAC DRIVER M: Nuno Sá <nuno.sa@analog.com>