> -----Original Message----- > From: David Lechner <dlechner@xxxxxxxxxxxx> > Sent: Tuesday, July 2, 2024 11:36 PM > To: Paller, Kim Seer <KimSeer.Paller@xxxxxxxxxx>; linux- > kernel@xxxxxxxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > Cc: 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 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