From: Victor Duicu <victor.duicu@microchip.com>
This is the devicetree schema for Microchip MCP998X/33 and
MCP998XD/33D Multichannel Automotive Temperature Monitor Family.
Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
---
.../iio/temperature/microchip,mcp9982.yaml | 174 ++++++++++++++++++
1 file changed, 174 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
new file mode 100644
index 000000000000..249470c8953b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
@@ -0,0 +1,174 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9982.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MCP998X/33 and MCP998XD/33D Multichannel Automotive
+ Temperature Monitor Family
+
+maintainers:
+ - Victor Duicu <victor.duicu@microchip.com>
+
+description: |
+ The MCP998X/33 and MCP998XD/33D family is a high-accuracy 2-wire multichannel
+ automotive temperature monitor.
+ The datasheet can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP998X-Family-Data-Sheet-DS20006827.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,mcp9933
+ - microchip,mcp9933d
+ - microchip,mcp9982
+ - microchip,mcp9982d
+ - microchip,mcp9983
+ - microchip,mcp9983d
+ - microchip,mcp9984
+ - microchip,mcp9984d
+ - microchip,mcp9985
+ - microchip,mcp9985d
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 2
+
+ interrupt-names:
+ description:
+ alert1 indicates a HIGH or LOW limit was exceeded.
+ alert2 indicates a THERM limit was exceeded.
+ items:
+ - const: alert1
+ - const: alert2
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ microchip,apdd-state:
+ description:
+ Enable anti-parallel diode mode operation.
+ MCP9984/84D/85/85D and MCP9933/33D support reading two external diodes
+ in anti-parallel connection on the same set of pins.
+ Omit this tag to disable anti-parallel diode mode.
+ type: boolean
+
+ 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.
+ type: boolean
+
+ label:
+ description: Unique name to identify which device this is.
+
+ vdd-supply: true
+
+patternProperties:
+ "^channel@[1-4]$":
+ description:
+ Represents the external temperature channels to which
+ a remote diode is connected.
+ type: object
+
+ properties:
+ reg:
+ items:
+ minimum: 1
+ maximum: 4
+
+ microchip,ideality-factor:
+ description:
+ Each channel has an ideality factor.
+ Beta compensation and resistance error correction automatically
+ correct for most ideality errors. So ideality factor does not need
+ to be adjusted in general.
+ Omit this tag in order to set the default value.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 18
+
+ label:
+ description: Unique name to identify which channel this is.
+
+ required:
+ - reg
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,mcp9982
+ - microchip,mcp9982d
+ - microchip,mcp9983
+ - microchip,mcp9983d
+ then:
+ properties:
+ microchip,apdd-state: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,mcp9982
+ - microchip,mcp9982d
+ - microchip,mcp9933
+ - microchip,mcp9933d
+ then:
+ properties:
+ microchip,recd34: false
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ temperature-sensor@4c {
+ compatible = "microchip,mcp9985";
+ reg = <0x4c>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ label = "temperature-sensor";
+
+ microchip,apdd-state;
+ microchip,recd12;
+ microchip,recd34;
+ vdd-supply = <&vdd>;
+
+ channel@1 {
+ reg = <0x1>;
+ label = "CPU Temperature";
+ };
+
+ channel@2 {
+ reg = <0x2>;
+ label = "GPU Temperature";
+ };
+ };
+ };
+
+...
--
2.48.1On 5/29/25 4:36 AM, victor.duicu@microchip.com wrote:
> From: Victor Duicu <victor.duicu@microchip.com>
>
> This is the devicetree schema for Microchip MCP998X/33 and
> MCP998XD/33D Multichannel Automotive Temperature Monitor Family.
>
> Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
> ---
> .../iio/temperature/microchip,mcp9982.yaml | 174 ++++++++++++++++++
> 1 file changed, 174 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
> new file mode 100644
> index 000000000000..249470c8953b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
> @@ -0,0 +1,174 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9982.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MCP998X/33 and MCP998XD/33D Multichannel Automotive
> + Temperature Monitor Family
> +
> +maintainers:
> + - Victor Duicu <victor.duicu@microchip.com>
> +
> +description: |
> + The MCP998X/33 and MCP998XD/33D family is a high-accuracy 2-wire multichannel
> + automotive temperature monitor.
> + The datasheet can be found here:
> + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP998X-Family-Data-Sheet-DS20006827.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - microchip,mcp9933
> + - microchip,mcp9933d
> + - microchip,mcp9982
> + - microchip,mcp9982d
> + - microchip,mcp9983
> + - microchip,mcp9983d
> + - microchip,mcp9984
> + - microchip,mcp9984d
> + - microchip,mcp9985
> + - microchip,mcp9985d
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 2
> +
> + interrupt-names:
> + description:
> + alert1 indicates a HIGH or LOW limit was exceeded.
> + alert2 indicates a THERM limit was exceeded.
I think we need minItems: 1 here.
> + items:
> + - const: alert1
> + - const: alert2
Typically, interrupts are named after the pin they are wired to, not
the signal. This is especially true when a single pin can be configured
for different signals as is the case here.
There is a /ALERT//THERM pin on all chips and a /THERM//ADDR pin on some
chips.
So I would expect the names to match that:
items:
- const: alert-therm
- const: therm-addr
And then extra descriptions probably aren't needed.
If we want to be extra careful, we could also add an -if: below to set
maxItems: 1 for interrupts and interrupt-names on chips that only have
the one pin.
And I assume that the /SYS_SHDN pin would never be wired up as an interrupt?
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + microchip,apdd-state:
> + description:
> + Enable anti-parallel diode mode operation.
> + MCP9984/84D/85/85D and MCP9933/33D support reading two external diodes
> + in anti-parallel connection on the same set of pins.
> + Omit this tag to disable anti-parallel diode mode.
> + type: boolean
> +
> + 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.
> + type: boolean
> +
> + label:
> + description: Unique name to identify which device this is.
> +
> + vdd-supply: true
> +
> +patternProperties:
> + "^channel@[1-4]$":
> + description:
> + Represents the external temperature channels to which
> + a remote diode is connected.
> + type: object
> +
> + properties:
> + reg:
> + items:
> + minimum: 1
> + maximum: 4
> +
> + microchip,ideality-factor:
> + description:
> + Each channel has an ideality factor.
> + Beta compensation and resistance error correction automatically
> + correct for most ideality errors. So ideality factor does not need
> + to be adjusted in general.
> + Omit this tag in order to set the default value.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 18
> +
> + label:
> + description: Unique name to identify which channel this is.
> +
> + required:
> + - reg
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - vdd-supply
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - microchip,mcp9982
> + - microchip,mcp9982d
> + - microchip,mcp9983
> + - microchip,mcp9983d
> + then:
> + properties:
> + microchip,apdd-state: false
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - microchip,mcp9982
> + - microchip,mcp9982d
> + - microchip,mcp9933
> + - microchip,mcp9933d
> + then:
> + properties:
> + microchip,recd34: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + temperature-sensor@4c {
> + compatible = "microchip,mcp9985";
> + reg = <0x4c>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + label = "temperature-sensor";
This is the same as the node name, so probably not the best
example of a label.
> +
> + microchip,apdd-state;
> + microchip,recd12;
> + microchip,recd34;
> + vdd-supply = <&vdd>;
> +
> + channel@1 {
> + reg = <0x1>;
Why 0x here?
> + label = "CPU Temperature";
> + };
> +
> + channel@2 {
> + reg = <0x2>;
> + label = "GPU Temperature";
> + };
> + };
> + };
> +
> +...
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>
> >
> > This is the devicetree schema for Microchip MCP998X/33 and
> > MCP998XD/33D Multichannel Automotive Temperature Monitor Family.
> >
> > Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
> > ---
> > .../iio/temperature/microchip,mcp9982.yaml | 174 ++++++++++++++++++
> > 1 file changed, 174 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
> > new file mode 100644
> > index 000000000000..249470c8953b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9982.yaml
> > @@ -0,0 +1,174 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9982.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip MCP998X/33 and MCP998XD/33D Multichannel Automotive
> > + Temperature Monitor Family
> > +
> > +maintainers:
> > + - Victor Duicu <victor.duicu@microchip.com>
> > +
> > +description: |
> > + The MCP998X/33 and MCP998XD/33D family is a high-accuracy 2-wire multichannel
> > + automotive temperature monitor.
> > + The datasheet can be found here:
> > + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP998X-Family-Data-Sheet-DS20006827.pdf
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - microchip,mcp9933
> > + - microchip,mcp9933d
> > + - microchip,mcp9982
> > + - microchip,mcp9982d
> > + - microchip,mcp9983
> > + - microchip,mcp9983d
> > + - microchip,mcp9984
> > + - microchip,mcp9984d
> > + - microchip,mcp9985
> > + - microchip,mcp9985d
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 2
> > +
> > + interrupt-names:
> > + description:
> > + alert1 indicates a HIGH or LOW limit was exceeded.
> > + alert2 indicates a THERM limit was exceeded.
>
> I think we need minItems: 1 here.
>
> > + items:
> > + - const: alert1
> > + - const: alert2
>
> Typically, interrupts are named after the pin they are wired to, not
> the signal. This is especially true when a single pin can be configured
> for different signals as is the case here.
>
> There is a /ALERT//THERM pin on all chips and a /THERM//ADDR pin on some
> chips.
>
> So I would expect the names to match that:
>
> items:
> - const: alert-therm
> - const: therm-addr
>
> And then extra descriptions probably aren't needed.
>
> If we want to be extra careful, we could also add an -if: below to set
> maxItems: 1 for interrupts and interrupt-names on chips that only have
> the one pin.
>
> And I assume that the /SYS_SHDN pin would never be wired up as an interrupt?
>
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + microchip,apdd-state:
> > + description:
> > + Enable anti-parallel diode mode operation.
> > + MCP9984/84D/85/85D and MCP9933/33D support reading two external diodes
> > + in anti-parallel connection on the same set of pins.
> > + Omit this tag to disable anti-parallel diode mode.
> > + type: boolean
> > +
> > + 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?
> > + type: boolean
> > +
> > + label:
> > + description: Unique name to identify which device this is.
> > +
> > + vdd-supply: true
> > +
> > +patternProperties:
> > + "^channel@[1-4]$":
> > + description:
> > + Represents the external temperature channels to which
> > + a remote diode is connected.
> > + type: object
> > +
> > + properties:
> > + reg:
> > + items:
> > + minimum: 1
> > + maximum: 4
> > +
> > + microchip,ideality-factor:
> > + description:
> > + Each channel has an ideality factor.
> > + Beta compensation and resistance error correction automatically
> > + correct for most ideality errors. So ideality factor does not need
> > + to be adjusted in general.
> > + Omit this tag in order to set the default value.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + default: 18
> > +
> > + label:
> > + description: Unique name to identify which channel this is.
> > +
> > + required:
> > + - reg
> > +
> > + additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - vdd-supply
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - microchip,mcp9982
> > + - microchip,mcp9982d
> > + - microchip,mcp9983
> > + - microchip,mcp9983d
> > + then:
> > + properties:
> > + microchip,apdd-state: false
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - microchip,mcp9982
> > + - microchip,mcp9982d
> > + - microchip,mcp9933
> > + - microchip,mcp9933d
> > + then:
> > + properties:
> > + microchip,recd34: false
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + temperature-sensor@4c {
> > + compatible = "microchip,mcp9985";
> > + reg = <0x4c>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + label = "temperature-sensor";
>
> This is the same as the node name, so probably not the best
> example of a label.
Ye, I'm not convinced this property has any value, when the channels
themselves can have labels.
>
> > +
> > + microchip,apdd-state;
> > + microchip,recd12;
> > + microchip,recd34;
> > + vdd-supply = <&vdd>;
> > +
> > + channel@1 {
> > + reg = <0x1>;
>
> Why 0x here?
>
> > + label = "CPU Temperature";
> > + };
> > +
> > + channel@2 {
> > + reg = <0x2>;
> > + label = "GPU Temperature";
> > + };
> > + };
> > + };
> > +
> > +...
>
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> > > > > > > Hi Conor, ... > > > + 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. The user cannot influence the parasitic resistance nor calculate it. With Kind Regards, Victor Duicu
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. > 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.
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
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: > > 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. 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
© 2016 - 2025 Red Hat, Inc.