Received-SPF: pass (google.com: domain of linux-kernel+bounces-195325-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <17280af8-186d-47f1-a0c3-368f39e7c76a@gmail.com>
Date: Thu, 30 May 2024 14:55:00 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 1/6] dt-bindings: adc: ad7173: add support for ad411x
To: =?UTF-8?Q?Nuno_S=C3=A1?= <noname.nuno@gmail.com>,
 Conor Dooley <conor@kernel.org>
Cc: dumitru.ceclan@analog.com, Lars-Peter Clausen <lars@metafoo.de>,
 Michael Hennerich <Michael.Hennerich@analog.com>,
 Jonathan Cameron <jic23@kernel.org>, Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, David Lechner <dlechner@baylibre.com>,
 linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240527-ad4111-v3-0-7e9eddbbd3eb@analog.com>
 <20240527-ad4111-v3-1-7e9eddbbd3eb@analog.com>
 <20240527-arguably-said-361184ad848e@spud>
 <d87ae6ef-090d-4e47-bde4-4d08fd445ac1@gmail.com>
 <20240528-filtrate-cloning-b9152322a3da@spud>
 <a1c75105-6447-4b67-b7d2-326ad9b19b82@gmail.com>
 <20240529-slit-verse-0fb06f3556fb@spud>
 <e0be3356bf809035963c4801f05b9db2675c111e.camel@gmail.com>
Content-Language: en-US
From: "Ceclan, Dumitru" <mitrutzceclan@gmail.com>
In-Reply-To: <e0be3356bf809035963c4801f05b9db2675c111e.camel@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 30/05/2024 10:50, Nuno Sá wrote:
> On Wed, 2024-05-29 at 17:04 +0100, Conor Dooley wrote:
>> On Wed, May 29, 2024 at 04:38:53PM +0300, Ceclan, Dumitru wrote:
>>> On 28/05/2024 20:52, Conor Dooley wrote:
>>>> On Tue, May 28, 2024 at 03:16:07PM +0300, Ceclan, Dumitru wrote:
>>>>> On 27/05/2024 20:48, Conor Dooley wrote:
>>>>>> On Mon, May 27, 2024 at 08:02:34PM +0300, Dumitru Ceclan via B4 Relay
>>>>>> wrote:
>>>>>>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>>>>>> +      adi,channel-type:
>>>>>>> +        description:
>>>>>>> +          Used to differentiate between different channel types as the
>>>>>>> device
>>>>>>> +           register configurations are the same for all usage types.
>>>>>>> +          Both pseudo-differential and single-ended channels will use
>>>>>>> the
>>>>>>> +           single-ended specifier.
>>>>>>> +        $ref: /schemas/types.yaml#/definitions/string
>>>>>>> +        enum:
>>>>>>> +          - single-ended
>>>>>>> +          - differential
>>>>>>> +        default: differential
>>>>>>
>>>>>> I dunno if my brain just ain't workin' right today, or if this is not
>>>>>> sufficiently explained, but why is this property needed? You've got
>>>>>> diff-channels and single-channels already, why can you not infer the
>>>>>> information you need from them? What should software do with this
>>>>>> information?
>>>>>> Additionally, "pseudo-differential" is not explained in this binding.
>>>>>
>>>>> In previous thread we arrived to the conclusion single-ended and
>>>>> pseudo-differential channels should be marked with the flag
>>>>> "differential=false" in the IIO channel struct. This cannot
>>>>> really be inferred as any input pair could be used in that
>>>>> manner and the only difference would be in external wiring.
>>>>>
>>>>> Single-channels cannot be used to define such a channel as
>>>>> two voltage inputs need to be selected. Also, we are already
>>>>> using single-channel to define the current channels.
>>>>
>>>> If I understand correctly, the property could be simplified to a flag
>>>> then, since it's only the pseudo differential mode that you cannot be
>>>> sure of?
>>>> You know when you're single-ended based on single-channel, so the
>>>> additional info you need is only in the pseudo-differential case.
>>>>
>>> Yes, it could just be a boolean flag. The only thing I have against
>>> that is the awkwardness of having both diff-channels and
>>> differential=false within a channel definition.
>>
>> What I was suggesting was more like "adi,pseudo-differential" (you don't
>> need to set the =false or w/e, flag properties work based on present/not
>> present). I think that would avoid the awkwardness?
>>
> 
> Yeah, that was also my understanding of your reply... But I think you're also
> mentioning to have this flag together with the single-channel property? 
> 
> I'm a bit confused because it seems to me that single-channel only gets one input
> while we need to select two for pseudo-differential/single-ended. Is this correct
> Dumitru?
>

Yes, that is correct.
 
> FWIW, I think we already have that awkwardness in the current form. If I'm not
> missing anything, what we have in the driver is pretty much:
> 
> if (not diff && single-channel)
> 	// then current channel
> else
> 	// relies on the channel-type stuff
> 
> So, effectively with the above we have
> 
> diff-channels = <1 0>;
> 
> but then wait, not so fast
> 

This comment properly and comically describes the hot mess 
that I've come up with :)))

> adi,channel-type = "single-ended"
> 
> To me the above is equally awkward (not sure if there's any precedence in using diff-
> channels like this though)...
> 
> I would like for this to be explicit... If we have diff-channels, then it's surely
> differential.
> 
> If not we could use the single-channel property and instead of using an extra flag we
> could make the property having either 1 or 2 items. If we have 1, then it's a current
> channel. If we have 2, then it's voltage single-ended/pseudo-differential. 
> 
> David's suggestion is also pretty good (and I like it's more explicit about what's
> going on) so I would likely go with it...
> 
> - Nuno Sá
> 
> 

Yup, as neat as it could be, I'll do it that way.