On 2/21/22 01:07, Krzysztof Adamski wrote:
Dnia Fri, Feb 18, 2022 at 07:09:07AM -0800, Guenter Roeck napisał(a):
From: Agathe Porte <agathe.porte@xxxxxxxxx>
Add basic description of the tmp464 driver DT bindings.
Signed-off-by: Agathe Porte <agathe.porte@xxxxxxxxx>
Cc: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx>
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
v5:
- Dropped ti,n-factor from channel@0 example. Added additional
channel to examples to show positive ti,n-factor property.
v4:
- No changes
v3:
- Addedd support for TMP468.
- Changed number of channels from 0..3 (which was wrong anyway) to 0..8.
- Changed value range for ti,n-factor to int8, with an example for
a negative value.
- Added myself as driver maintainer.
.../devicetree/bindings/hwmon/ti,tmp464.yaml | 114 ++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 121 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/ti,tmp464.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp464.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp464.yaml
new file mode 100644
index 000000000000..14f6a3412b8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp464.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/ti,tmp464.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TMP464 and TMP468 temperature sensors
+
+maintainers:
+ - Agathe Porte <agathe.porte@xxxxxxxxx>
+
+description: |
+ ±0.0625°C Remote and Local temperature sensor
+ https://www.ti.com/lit/ds/symlink/tmp464.pdf
+ https://www.ti.com/lit/ds/symlink/tmp468.pdf
+
+properties:
+ compatible:
+ enum:
+ - ti,tmp464
+ - ti,tmp468
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+patternProperties:
+ "^channel@([0-8])$":
+ type: object
+ description: |
+ Represents channels of the device and their specific configuration.
+
+ properties:
+ reg:
+ description: |
+ The channel number. 0 is local channel, 1-8 are remote channels.
+ items:
+ minimum: 0
+ maximum: 8
+
+ label:
+ description: |
+ A descriptive name for this channel, like "ambient" or "psu".
+
+ ti,n-factor:
+ description: |
+ The value (two's complement) to be programmed in the channel specific N correction register.
+ For remote channels only.
+ $ref: /schemas/types.yaml#/definitions/int8
+ items:
+ minimum: -128
+ maximum: 127
I still thing we should have the same format here and in tmp421, for
consistency. If use the same property name, "ti,n-factor" but on tmp421
you have use 32bit value while here you have to use 8bit (which is weird
in DT, BTW), it might be confusing.
Back when we did this for TMP421, there was some discussion and we
settled on this approach, why do it differently now?
I seem to recall from that discussion that there was supposedly no way to
express negative numbers in devicetree. Obviously that is incorrect.
In addition to that, I strongly suspect that the tmp421 code as written
does not work. Its value range is specified as 0..255, but it is read with
err = of_property_read_s32(child, "ti,n-factor", &val);
and range checked with
if (val > 127 || val < -128) {
dev_err(dev, "n-factor for channel %d invalid (%d)\n",
i, val);
return -EINVAL;
}
That just looks wrong. Either the value range is 0..255 and checked
as 0 .. 255, or it is -128 .. 127 and must be both checked and specified
accordingly. This made me look into the code and I found how negative
numbers are supposed to be handled.
We can go either way, but whatever it is should be correct and be confirmed
to work. Rob, any thoughts ?
Guenter
