| Message ID | 20180921223216.634-2-nicoleotsuka@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add an initial DT binding doc for ina3221 | expand |
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
> >+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"). Oh. I see the point here now. Then a child name could be just input[123], and I will add a separate optional child property to indicate the label. Will fix it in next ver. Thanks Nicolin
On 09/22/2018 11:03 AM, Nicolin Chen wrote: >>> +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"). > > Oh. I see the point here now. Then a child name could be just input[123], > and I will add a separate optional child property to indicate the label. > Exactly. "label" is quite widely used as property name, so that should be acceptable. I am not sure about index; we'll see if Rob has any comments. Thanks, Guenter
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>; + };
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