Messages in this thread | | | Subject | Re: [PATCH v3 1/2] dt-bindings: hwmon: Add ina3221 documentation | From | Guenter Roeck <> | Date | Sat, 22 Sep 2018 05:38:37 -0700 |
| |
Hi,
On 09/21/2018 03:32 PM, Nicolin Chen wrote: > Texas Instruments INA3221 is a triple-channel shunt and bus > voltage monitor. This patch adds a DT binding doc for it. > > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> > --- > Changelog > v2->v3: > * Added a simple subject in the line 1 > * Fixed the shunt resistor value in the example > v1->v2: > * Dropped channel name properties > * Added child node definitions. > * * Added shunt resistor property in the child node > * * Added status property to indicate connection status > * * Changed to use child node name as the label of input source > > .../devicetree/bindings/hwmon/ina3221.txt | 38 +++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt > > diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt > new file mode 100644 > index 000000000000..bcfd5b9c697b > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/ina3221.txt > @@ -0,0 +1,38 @@ > +Texas Instruments INA3221 Device Tree Bindings > + > +1) ina3221 node > + Required properties: > + - compatible: Must be "ti,ina3221" > + - reg: I2C address > + > + = The node contains optional child nodes for three channels = > + = Each child node describes the information of input source = > + > + Example: > + > + ina3221@40 { > + compatible = "ti,ina3221"; > + reg = <0x40>; > + [ child node definitions... ] > + }; > + > +2) child nodes > + The names of child nodes should indicate input source names > + > + Required properties: > + - input-id: Must be 1, 2 or 3 > + > + Optional properties: > + - shunt-resistor: Shunt resistor value in micro-Ohm > + - status: Should be "disabled" if no input source > + > + Example: > + > + input1 { > + input-id = <0x1>; > + status = "disabled"; > + }; > + VDD_GPU { > + input-id = <0x2>; > + shunt-resistor = <5000>; > + }; >
Using child nodes is a good idea. However, you are converting the node name into the hwmon 'label' attribute which I can not accept. First, it is undocumented, second, it effectively creates an undocumented property (if one wants to configure the shunt resistor value, one has to configure a child node which is converted into a label), and third, it violates the hwmon ABI ('input1' is not a "hint about what this voltage channel is being used for").
Guenter
| |