[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<PH0PR03MB7141B14FAC992C1BA0C95112F9DA2@PH0PR03MB7141.namprd03.prod.outlook.com>
Date: Mon, 8 Jul 2024 05:55:05 +0000
From: "Paller, Kim Seer" <KimSeer.Paller@...log.com>
To: Conor Dooley <conor+dt@...nel.org>, David Lechner <dlechner@...libre.com>,
Jonathan Cameron <jic23@...nel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"linux-iio@...r.kernel.org"
<linux-iio@...r.kernel.org>,
"devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>
CC: Lars-Peter Clausen <lars@...afoo.de>, Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
Dimitri Fedrau <dima.fedrau@...il.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Rob Herring <robh@...nel.org>,
"Hennerich, Michael" <Michael.Hennerich@...log.com>,
Nuno Sá
<noname.nuno@...il.com>
Subject: RE: [PATCH v5 4/6] dt-bindings: iio: dac: Add adi,ltc2664.yaml
...
> > > + 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
Powered by blists - more mailing lists