linux-kernel - Re: [PATCH v2 1/2] dt-bindings: iio: temperature: add support for MCP998X

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250610-thong-trump-f131522201c4@spud>
Date: Tue, 10 Jun 2025 16:17:11 +0100
From: Conor Dooley <conor@...nel.org>
To: Victor.Duicu@...rochip.com
Cc: dlechner@...libre.com, nuno.sa@...log.com, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, robh@...nel.org, jic23@...nel.org,
	krzk+dt@...nel.org, andy@...nel.org, linux-kernel@...r.kernel.org,
	Marius.Cristea@...rochip.com, conor+dt@...nel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: temperature: add support for
 MCP998X

On Tue, Jun 10, 2025 at 01:29:01PM +0000, Victor.Duicu@...rochip.com wrote:
> On Fri, 2025-06-06 at 16:15 +0100, Conor Dooley wrote:
> > Jonathan,
> > 
> > On Mon, Jun 02, 2025 at 02:48:52PM +0000,
> > Victor.Duicu@...rochip.com wrote:
> > > On Fri, 2025-05-30 at 16:55 +0100, Conor Dooley wrote:
> > > > On Thu, May 29, 2025 at 01:13:38PM -0500, David Lechner wrote:
> > > > > On 5/29/25 4:36 AM, victor.duicu@...rochip.com wrote:
> > > > > > From: Victor Duicu <victor.duicu@...rochip.com>
> > > > > > +  microchip,recd12:
> > > > > > +    description:
> > > > > > +      Enable resistance error correction for external
> > > > > > channels 1
> > > > > > and 2.
> > > > > > +      Omit this tag to disable REC for channels 1 and 2.
> > > > > > +    type: boolean
> > > > > > +
> > > > > > +  microchip,recd34:
> > > > > > +    description:
> > > > > > +      Enable resistance error correction for external
> > > > > > channels 3
> > > > > > and 4.
> > > > > > +      Omit this tag to disable REC for channels 3 and 4.
> > > > 
> > > > Why are these two devicetree properties, rather than runtime
> > > > controls?
> > > 
> > > The parasitic resistance added to the series resistance is
> > > dependent
> > > only on the circuit. 
> > > It is possible for the chip and the transistor to be at some
> > > distance
> > > from each other. The manufacturer can approximate the error added
> > > and
> > > decide if resistance error correction should be applied.
> > 
> > I don't think I buy this line of argument. The property is not
> > describing the hardware, it's literally being used as a toggle for
> > some
> > software feature. It'd be more acceptable if it indicated that the
> > chip
> > and transistor were distant, leaving software to make a decision on
> > what
> > that meant. One user (say bsd) might want decide that the driver
> > should
> > always enable it, but another (say linux) might expose it as a
> > control
> > to userspace defaulting based the dt property.
> > Additionally, the name of the property is pretty awful, and does not
> > even hint at what it's doing - and there's no mention of why channel
> > 1/2
> > and 3/4 are bound together.
> > 
> You are correct that the parameters recd12 and recd34 do not directly
> describe the hardware, but they control a software feature of the chip
> itself. Resistance error correction is capable of counterbalancing
> the parasitic resistance added to the external diodes, which can be
> significant.

> The manufacturer knows where the chip and diode are and can decide if
> correction is necessary. The user does not have that insight.

The user _may_ not have it. The properties should not be written such
that they exclude the control of these things from userspace, and just
indicate that the hardware configuration has the chip and diode
sufficiently far apart that the feature can help.


> I can change the name of the parameter to something like
> resistance_err_correction and mention in the description which channels
> are affected.

No _s are allowed in properties, so bear that in mind. Again, I don't
think the property should be responsible for turning it on, just
indicate that there is a parasitic resistance, so and the name should
really be something that indicates a parasitic resistance. E.g.
microchip,parasitic-res-on-channel1. Software (be it userspace or
driver) can then made a decision about turning on the error correction
with that information.

> > > The user cannot influence the parasitic resistance nor calculate
> > > it.
> > 
> > I don't think that's super relevant here, since the property has
> > nothing
> > to do with influencing or calculating the value. I meant deciding
> > whether
> > or not the correction is applied, just as the dt property you propose
> > does now.
> > 
> > Cheers,
> > Conor.
> 
> Kind Regards,
> Victor Duicu

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