On 06/04/2024 17:26, Jonathan Cameron wrote: > On Thu, 4 Apr 2024 16:08:56 +0300 > "Ceclan, Dumitru" <mitrutzceclan@xxxxxxxxx> wrote: > >> On 03/04/2024 18:22, David Lechner wrote: >>> On Wed, Apr 3, 2024 at 2:50 AM Ceclan, Dumitru <mitrutzceclan@xxxxxxxxx> wrote: >>>> On 02/04/2024 00:16, David Lechner wrote: >>>>> 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> >>>>>>> >>>> ... >>>> >>>>>>> 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. >>>> Sure, but we still need to offer the user a way to configure which >>>> current inputs he wants and if they should use bipolar or unipolar coding. >>> From the datasheet, it looks like only positive current input is >>> allowed so I'm not sure bipolar applies here. But, yes, if there is >>> some other variation in wiring or electrical signal that needs to be >>> describe here, then it makes sense to allow a channel configuration >>> node for it. >> >> AD4111 datasheet pg.29: >> When the ADC is configured for bipolar operation, the output >> code is offset binary with a negative full-scale voltage resulting >> in a code of 000 … 000, a zero differential input voltage resulting in >> a code of 100 … 000, and a positive full-scale input voltage >> resulting in a code of 111 … 111. The output code for any >> analog input voltage can be represented as >> Code = 2^(N – 1) × ((V_IN × 0.1/V REF) + 1) >> The output code for any input current is represented as >> Code = 2^(N − 1) × ((I_IN × 50 Ω/V REF) + 1) >> >> I would say bipolar applies here, not a great idea because of the limitation on >> the negative side (Input Current Range min:−0.5 max:+24 mA) so still, the option >> is available. > Just to check I am correct in thinking you 'might' use bipolar if you want > to be able to measure small negative currents, but the range is much larger > in the positive direction? > > J Yes, exactly
