linux-kernel - Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170

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: <20250623-wackiness-unaware-e0e904064990@spud>
Date: Mon, 23 Jun 2025 17:03:23 +0100
From: Conor Dooley <conor@...nel.org>
To: Jonathan Cameron <jic23@...nel.org>
Cc: David Lechner <dlechner@...libre.com>,
	Marcelo Schmitt <marcelo.schmitt@...log.com>,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
	lars@...afoo.de, Michael.Hennerich@...log.com, nuno.sa@...log.com,
	andy@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, linus.walleij@...aro.org, brgl@...ev.pl,
	marcelo.schmitt1@...il.com
Subject: Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170

On Sat, Jun 21, 2025 at 05:28:08PM +0100, Jonathan Cameron wrote:
> 
> > >> +
> > >> +$defs:
> > >> +  reference-buffer:
> > >> +    description: |
> > >> +      Enable precharge buffer, full buffer, or skip reference buffering of
> > >> +      the positive/negative voltage reference. Because the output impedance
> > >> +      of the source driving the voltage reference inputs may be dynamic,
> > >> +      resistive/capacitive combinations of those inputs can cause DC gain
> > >> +      errors if the reference inputs go unbuffered into the ADC. Enable
> > >> +      reference buffering if the provided reference source has dynamic high
> > >> +      impedance output. Note the absolute voltage allowed on REFINn+ and REFINn-
> > >> +      inputs is from AVSS - 50 mV to AVDD + 50 mV when the reference buffers are
> > >> +      disabled but narrows to AVSS to AVDD when reference buffering is enabled
> > >> +      or in precharge mode. The valid options for this property are:
> > >> +      0: Reference precharge buffer.
> > >> +      1: Full reference buffering.
> > >> +      2: Bypass reference buffers (buffering disabled).
> > >> +    $ref: /schemas/types.yaml#/definitions/uint32
> > >> +    enum: [0, 1, 2]
> > >> +    default: 1  
> > > 
> > > Why make this property a uint32, rather than a string where you can use
> > > something like "precharge", "full" and "bypass" (or "disabled")? The
> > > next similar device could use something slightly different then the
> > > binding becomes pretty clunky.
> > > Can you explain why this is a dt property rather than something
> > > adjustable at runtime?
> > > 
> > > Otherwise, what you have here looks sane enough to me - but I'd like to
> > > see some comments from Jonathan or David etc about your approach to the
> > > excitation properties.  
> > 
> > This looks like something that should be in the devicetree to me. For example
> > if the external reference supply is high impedance, buffering is pretty
> > much required. And using precharge is an application design choice to
> > reduce THD at the expense of other limitations.
> > 
> 
> Agreed that this pretty much only makes sense in DT.
> 
> In the ideal world we would have firm rules on when to enable buffering
> etc and then the DT would describe the impedance of the circuit connected
> and any other relevant properties and then we'd have the driver enable it
> only when those rigid rules dictated that we should.
> 
> Sadly no such simple rules exist (as far as I know) so we just expose the thing
> that gets set dependent on someone's judgement of the suitability of
> the buffering choice given the circuit being connected to the input.
> 
> If we pushed it to userspace we'd just end up with a per device blob
> that dictated the mode to pick on boot and left it like that.  So effectively
> another bit of firmware :(

Can't remember if I replied to David's mail here or not, I pretty much
just asked the question because I didn't understand why the usecase and
wanted an explanation for my benefit. Probably makes no sense to explain
why a knob exists in the binding when anyone using the device will not
need one, but that does mean that at times with these devices that are
unfamiliar to me I have to have it explained to me why something is set
in stone.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)