On Mon, Apr 1, 2024 at 2:37 PM David Lechner <dlechner@xxxxxxxxxxxx> wrote: > > On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay > <devnull+dumitru.ceclan.analog.com@xxxxxxxxxx> wrote: > > > > From: Dumitru Ceclan <dumitru.ceclan@xxxxxxxxxx> > > > > Add support for: AD4111, AD4112, AD4114, AD4115, AD4116. > > > > AD411x family ADCs support a VCOM pin, dedicated for single-ended usage. > > AD4111/AD4112 support current channels, usage is implemented by > > specifying channel reg values bigger than 15. > > > > Signed-off-by: Dumitru Ceclan <dumitru.ceclan@xxxxxxxxxx> > > --- ... > > @@ -125,10 +141,19 @@ patternProperties: > > > > properties: > > reg: > > + description: > > + Reg values 16-19 are only permitted for ad4111/ad4112 current channels. > > minimum: 0 > > - maximum: 15 > > + maximum: 19 > > This looks wrong. Isn't reg describing the number of logical channels > (# of channel config registers)? > > After reviewing the driver, I see that > 16 is used as a way of > flagging current inputs, but still seems like the wrong way to do it. > See suggestion below. > > > > > diff-channels: > > + description: > > + For using current channels specify only the positive channel. > > + (IIN2+, IIN2−) -> diff-channels = <2 0> > > I find this a bit confusing since 2 is already VIN2 and 0 is already > VIN0. I think it would make more sense to assign unique channel > numbers individually to the negative and positive current inputs. > Also, I think it makes sense to use the same numbers that the > registers in the datasheet use (8 - 11 for negative and 12 to 15 for > positive). > > So: (IIN2+, IIN2−) -> diff-channels = <13 10> Thinking about this a bit more... Since the current inputs have dedicated pins and aren't mix-and-match with multiple valid wiring configurations like the voltage inputs, do we even need to describe them in the devicetree? In the driver, the current channels would just be hard-coded like the temperature channel since there isn't any application-specific variation.