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

[<Victor.Duicu@microchip.com>]

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@microchip.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@microchip.com wrote:
> > > > > From: Victor Duicu <victor.duicu@microchip.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.

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


> > 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