linux-kernel - Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250113201858.4b8b1926@jic23-huawei>
Date: Mon, 13 Jan 2025 20:18:58 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: "Miclaus, Antoniu" <Antoniu.Miclaus@...log.com>, Nuno Sá
 <noname.nuno@...il.com>, "robh@...nel.org" <robh@...nel.org>,
 "conor+dt@...nel.org" <conor+dt@...nel.org>, "linux-iio@...r.kernel.org"
 <linux-iio@...r.kernel.org>, "devicetree@...r.kernel.org"
 <devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "linux-pwm@...r.kernel.org"
 <linux-pwm@...r.kernel.org>
Subject: Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

On Mon, 13 Jan 2025 13:05:30 +0100
Krzysztof Kozlowski <krzk@...nel.org> wrote:

> On 13/01/2025 12:50, Miclaus, Antoniu wrote:
> >>>
> >>> Oh sure... Makes sense and I forgot that the property is not new...
> >>>  
> >>>> But looking for pins brought second point - here you claim these are
> >>>> mutually exclusive while datasheet suggests that both inputs can be
> >>>> connected. Unless they come from the same source always?
> >>>>  
> >>>
> >>> If you have a single ended input then only one pin (the positive one) will be
> >>> used. If the input signal is differential, then both pins will be used. So they  
> >>
> >> But the clocks describe input pins, at least in typical case, so that's
> >> my question: how many clock sources do you have here? One or two?
> >>  
> >>> are mutually exclusive... You either have single ended or a differential input.
> >>> And depending on the input type, the limit of the input frequency varies.  
> >>
> >> Based on this, this is the same clock, so using "diff" is not a property
> >> of "clocks". Look at other bindings how they encode differential choice
> >> for some signals - usually bool property, but not always - see other
> >> adi/admv devices.  
> > 
> > This approach that you suggest was implemented in patch series v1 (as boolean) / v3 (as enum).
> > Based on feedback in v3 received from Jonathan I switched to this. Should I revert it?  
> 
> I see:
> https://lore.kernel.org/lkml/20241220195220.1e1e1d6f@jic23-huawei/
> 
> I think that v3 was preferred with arguments above. You have one clock
> input and you want to configure the device differently, based on how
> this clock is wired. But it is still one clock.
> 
> Wait for Jonathan before reverting to v3.

This is following the approach we've previously used for crystal (xtal) vs clock
(typically one pin shared in those cases as well).

They are effectively different types of clock that might be connected.

e.g. adi,ad7173.yaml or adi,ad7192.yaml

I'm struggling to find the original discussion of that case.
Easy enough to find where they were added, but they mysteriously
just appear and I'm sure we had a detailed discussion prior to that.

I don't mind that much if we go to a boolean, the clock names just seems
cleaner to me as they are effectively different possible clock inputs
(with one shared pin). Whereas one of the pins is not shared as only applies
in the differential case.

Jonathan

> 
> Best regards,
> Krzysztof