On Tue, 16 Apr 2024 14:40:56 +0000 "Paller, Kim Seer" <KimSeer.Paller@xxxxxxxxxx> wrote: > > -----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? I think it should not be optional. Should be fine providing a 0V fixed regulator via DT if it is wired to 0. Given that is a little unusual, check it works! > > > > ... > > > > > > > > > > > > > > > > * (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. The pins should be in the DT binding, though the driver support can of course be a future exercise. Make them optional. > > > > > > > ... > > > > > > > > > > > > > > + > > > > > > + 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 > > >
