Re: [PATCH v3 11/11] dt-bindings: hwmon: allow specifying channels for tmp421

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dnia Sat, Oct 02, 2021 at 07:22:19AM -0700, Guenter Roeck napisał(a):
On Thu, Sep 30, 2021 at 09:19:49AM +0200, Krzysztof Adamski wrote:
Add binding description for the per temperature channel configuration
like labels and n-factor.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx>
---
 .../devicetree/bindings/hwmon/ti,tmp421.yaml  | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
index 47040ace4f73..0d4ea2209500 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
@@ -24,12 +24,49 @@ properties:
   reg:
     maxItems: 1

+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
 required:
   - compatible
   - reg

 additionalProperties: false

+patternProperties:
+  "^input@([0-4])$":

Was there agreement on "input" ? It is a somewhat odd name for a temperature
sensor. If that name can be used to distinguish child sensor types, it might
make sense to have a well defined name to state that this is a temperature
sensor.

Nope, no conclusion on that, yet, thus I did not change that and I was
still using the same approach I had on v1. For me it can be a "channel@X", a
"temperature@X".. whatever you decide.

However I'm in favor of some generic name, like "channel" or "input",
and using some "type property", if required, instead of calling the
nodes "temperatue@X", "voltage@X".

+    type: object
+    description: |
+      Represents channels of the device and their specific configuration.
+
+    properties:
+      reg:
+        description: |
+          The channel number. 0 is local channel, 1-4 are remote channels

Which of the supported chips has 4 remote channels ?

True, there is no TMP424. I will fix that in v4.


+        items:
+          minimum: 0
+          maximum: 4
+
+      label:
+        description: |
+          A descriptive name for this channel, like "ambient" or "psu".
+
+      n-factor:

n-factor or "ti,n-factor" ? The unit is chip specific, after all.

Or ti,nfactor, as used by tmp513? Again, there was no clear conclusion
so I didn't change that. Let me know what is your decision and I will
obey that.


+        description: |
+          The value (two's complement) to be programmed in the channel specific N correction register.
+          For remote channels only.
+        items:
+          minimum: 0
+          maximum: 1

Is this the correct value range ? The value range (in integer form) is
-128 .. 127 (or 0 .. 255 as unsigned), not 0..1.

True, I must have misunderstood this minimum/maximum and confused it
with the number of items or something. Now, since DT does not really
handle signed values and considers everything an unsigned, should I use
0..255 or -128..127?

Krzysztof



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux