Re: [PATCH 1/4] dt-bindings: iio: dac: Add adi,ltc2664.yaml

[David Lechner]

On Sat, Apr 13, 2024 at 10:06 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Fri, 12 Apr 2024 16:23:00 -0500
> David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
> > On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
> > <kimseer.paller@xxxxxxxxxx> wrote:
> > >

..

> >
> > And there is V~ on both which can be between -5.5V/-15.75V and GND, so
> > optional v-neg-supply seems appropriate.
>
> Only make it optional in the binding if the settings of the device change
> depending on whether it is there or not.  Looks like there is an internal
> reference, so maybe it really is optional.

I suggested optional with the thinking that if the pin is tied to GND,
then the property would be omitted.


..


> >
> > * (both) The MUX/MUXOUT pins look like we have an embedded pin mux, so
> > it could mean we need #pinctrl-cells. ltc2664 would also need
> > muxin-gpios for this.
> Not convinced that's the right approach - looks more like a channel
> selector than a conventional mux or pin control. Sure that's a mux, but
> we want a clean userspace control to let us choose a signal to measure
> at runtime
>
> If you wanted to support this I'd have the binding describe optional
> stuff to act as a consumer of an ADC channel on another device.
> The IIO driver would then provide a bunch of input channels to allow
> measurement of each of the signals.
>
> Look at io-channels etc in existing bindings for how to do that.
>

Right. I was thinking that this pin might be connected to something
else external rather than the signal coming back to the SoC (or
whatever has the SPI controller). But it makes more sense that we
would want it as extra channels being read back by the SoC for
diagnostics.

..

> >
> > > +
> > > +      patternProperties:
> > > +        "^channel@([0-3])$":
> > > +          $ref: '#/$defs/toggle-operation'
> > > +          unevaluatedProperties: false
> > > +
> > > +          description: Channel in toggle functionality.
> > > +
> > > +          properties:
> > > +            adi,output-range-microvolt:
> > > +              description: Specify the channel output full scale range.
> >
> > How would someone writing a .dts know what values to select for this
> > property? Or is this something that should be configured at runtime
> > instead of in the devicetree? Or should this info come from the
> > missing voltage supplies I mentioned?
>
> Sometimes this one is a wiring related choice.  Sometimes to the extent
> that picking the wrong one from any userspace control can cause damage
> or is at least nonsense.
>
> You look to be right though that the possible values here aren' fine
> if the internal reference is used, but not the external.
>
> However, it's keyed off MPS pins so you can't control it if they aren't
> tied to all high.  So I'd imagine if the board can be damaged it will
> be hard wired.  Hence these could be controlled form userspace.
> It's a bit fiddly though as combines scale and offset controls and
> you can end trying to set things to an invalid combination.
> E.g. scale set to cover 20V range and offset set to 0V
> To get around that you have to clamp one parameter to nearest
> possible when the other is changed.
>

Thanks for the explanation. It sounds like I missed something in the
datasheet that would be helpful to call out in the description for
this property.