Message-ID: 
 <PH0PR03MB714170F4494EA0E1B4112291F9082@PH0PR03MB7141.namprd03.prod.outlook.com>
Date: Tue, 16 Apr 2024 14:40:56 +0000
From: "Paller, Kim Seer" <KimSeer.Paller@...log.com>
To: Jonathan Cameron <jic23@...nel.org>, David Lechner <dlechner@...libre.com>
CC: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Lars-Peter
 Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Liam
 Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
        "Hennerich,
 Michael" <Michael.Hennerich@...log.com>
Subject: RE: [PATCH 1/4] dt-bindings: iio: dac: Add adi,ltc2664.yaml

> -----Original Message-----
> From: Jonathan Cameron <jic23@...nel.org>
> Sent: Sunday, April 14, 2024 1:10 AM
> To: David Lechner <dlechner@...libre.com>
> Cc: Paller, Kim Seer <KimSeer.Paller@...log.com>; linux-iio@...r.kernel.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Lars-Peter Clausen
> <lars@...afoo.de>; Rob Herring <robh@...nel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@...aro.org>; Conor Dooley <conor+dt@...nel.org>;
> Liam Girdwood <lgirdwood@...il.com>; Mark Brown <broonie@...nel.org>;
> Hennerich, Michael <Michael.Hennerich@...log.com>
> Subject: Re: [PATCH 1/4] dt-bindings: iio: dac: Add adi,ltc2664.yaml
> 
> [External]
> 
> On Sat, 13 Apr 2024 11:21:55 -0500
> David Lechner <dlechner@...libre.com> wrote:
> 
> > On Sat, Apr 13, 2024 at 10:06 AM Jonathan Cameron <jic23@...nel.org>
> wrote:
> > >
> > > On Fri, 12 Apr 2024 16:23:00 -0500
> > > David Lechner <dlechner@...libre.com> wrote:
> > >
> > > > On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
> > > > <kimseer.paller@...log.com> 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.
> 
> We could but given VND isn't really that special in this case I think
> I'd prefer a fixed voltage reg of 0V if someone does wire it like that.
> (I think that works, though not sure I've tried a 0V supply ;)

To clarify, does this mean we should still add an optional property for it in the binding?

> > ...
> >
> >
> > > >
> > > > * (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.
> 
> It might indeed.  But I think that's an exercise for the future if
> it matters.  Might be a debugfs control only perhaps.

We can consider potential future use cases as they become relevant.
For now, we might not to include or support this functionality.

> >
> > ...
> >
> > > >
> > > > > +
> > > > > +      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.
> Agreed - it needs more detail.
> 
> Jonathan
> 

Powered by blists - more mailing lists