On Tue, Jun 10, 2025 at 01:29:01PM +0000, Victor.Duicu@microchip.com wrote:

On Fri, 2025-06-06 at 16:15 +0100, Conor Dooley wrote:

quoted

Jonathan,

On Mon, Jun 02, 2025 at 02:48:52PM +0000,
Victor.Duicu@microchip.com wrote:

quoted

On Fri, 2025-05-30 at 16:55 +0100, Conor Dooley wrote:

quoted

On Thu, May 29, 2025 at 01:13:38PM -0500, David Lechner wrote:

quoted

On 5/29/25 4:36 AM, victor.duicu@microchip.com wrote:

quoted

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.

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.

quoted

quoted

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