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 ;) > > > ... > > > > > > > > * (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. > > ... > > > > > > > > + > > > > + 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