> -----Original Message----- > From: David Lechner <dlechner@xxxxxxxxxxxx> > Sent: Tuesday, June 25, 2024 12:49 AM > To: Paller, Kim Seer <KimSeer.Paller@xxxxxxxxxx>; Conor Dooley > <conor@xxxxxxxxxx> > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; Jonathan Cameron <jic23@xxxxxxxxxx>; Lars-Peter > Clausen <lars@xxxxxxxxxx>; Liam Girdwood <lgirdwood@xxxxxxxxx>; Mark > Brown <broonie@xxxxxxxxxx>; Dimitri Fedrau <dima.fedrau@xxxxxxxxx>; > Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; > Conor Dooley <conor+dt@xxxxxxxxxx>; Hennerich, Michael > <Michael.Hennerich@xxxxxxxxxx>; Nuno Sá <noname.nuno@xxxxxxxxx> > Subject: Re: [PATCH v4 3/5] dt-bindings: iio: dac: Add adi,ltc2664.yaml > > [External] > > > >>> + 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] > >> > >> Can you explain why this property is required, when below there's one that > sets > >> the ranges in microvolts? Isn't the only new information that this provides > the > >> reset values (in a few cases that it is not 0). > >> What am I missing? > > > > For specifying output range and reset options without relying on software > initialization > > routines, and also for enabling the softspan feature, I think this property > seems essential. > > So in other words, this describes how the MSP pins are hardwired and > the per-channel adi,output-range-microvolt is only permissible if > > adi,manual-span-operation-config = <7>; > > (or omitted since 7 is the default) > > because in that case each individual pin could have a different > required range based on what is wire up to it? > > But if adi,manual-span-operation-config is anything other than 7, > then adi,output-range-microvolt should be not allowed since all > channels will have the same range because of the hard-wired pins. > > correct? That's correct. > The description could probably just be simplified to say that > this describes how the 3 pins are hardwired and to see Table 4 > in the datasheet to understand the actual implications rather > than reproducing that table here. > > But I do agree that we need both properties. I think we are > just missing: > > - if: > properties: > adi,manual-span-operation-config: > const: 7 > then: > patternProperties: > "^channel@[0-3]$": > adi,output-range-microvolt: false > > (not tested - may need two ifs, one with > > - if: > required: > - adi,manual-span-operation-config > properties: > adi,manual-span-operation-config: > const: 7 > > and one with > > - if: > not: > required: > - adi,manual-span-operation-config > > to make it work properly) > > > > >>> + 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]$": > >>> + 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 > >>> + > >>> + adi,output-range-microvolt: > >>> + description: Specify the channel output full scale range. > >>> + oneOf: > >>> + - items: > >>> + - const: 0 > >>> + - enum: [5000000, 10000000] > >>> + - items: > >>> + - const: -5000000 > >>> + - const: 5000000 > >>> + - items: > >>> + - const: -10000000 > >>> + - const: 10000000 > >>> + - items: > >>> + - const: -2500000 > >>> + - const: 2500000 > >>> + > >>> + required: > >>> + - reg > >>> + - adi,output-range-microvolt > > And adi,output-range-microvolt should not be required. When SoftSpan > is not available (because MSP != 0x7), then the range is determined > by adi,manual-span-operation-config. > > And even when adi,manual-span-operation-config = <7>, there is still > a default range, so adi,output-range-microvolt should still not be > required in that case. This makes sense how this operates. Appreciate the detailed feedback.
