> -----Original Message----- > From: Jonathan Cameron <jic23@xxxxxxxxxx> > Sent: Sunday, April 14, 2024 1:10 AM > To: David Lechner <dlechner@xxxxxxxxxxxx> > Cc: Paller, Kim Seer <KimSeer.Paller@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Lars-Peter Clausen > <lars@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; > Liam Girdwood <lgirdwood@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; > Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx> > 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@xxxxxxxxxxxx> wrote: > > > 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. > > 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 >