| Message ID | 20221108045243.24143-1-nmalwade@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ina3221: add support for summation channel control | expand |
On 08/11/2022 05:52, Ninad Malwade wrote: > Add support to initialize summation channel control via kernel device > tree property "summation-bypass". The channel which has this property > is excluded from channel summation. > > Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. You skipped not only one, but all DT maintainers and mailing lists... > --- > .../devicetree/bindings/hwmon/ina3221.txt | 2 ++ > drivers/hwmon/ina3221.c | 14 ++++++++++++-- DT bindings and driver changes are separate patches. > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt > index fa63b6171407..c6e8e6aafcce 100644 > --- a/Documentation/devicetree/bindings/hwmon/ina3221.txt > +++ b/Documentation/devicetree/bindings/hwmon/ina3221.txt > @@ -29,6 +29,7 @@ Texas Instruments INA3221 Device Tree Bindings > Optional properties: > - label: Name of the input source > - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm > + - summation-bypass: exclude from channel summation. Convert to DT schema first. Best regards, Krzysztof
On Tue, Nov 08, 2022 at 02:02:54PM +0100, Krzysztof Kozlowski wrote: > On 08/11/2022 05:52, Ninad Malwade wrote: > > Add support to initialize summation channel control via kernel device > > tree property "summation-bypass". The channel which has this property > > is excluded from channel summation. > > > > Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> > > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > You skipped not only one, but all DT maintainers and mailing lists... > > > --- > > .../devicetree/bindings/hwmon/ina3221.txt | 2 ++ > > drivers/hwmon/ina3221.c | 14 ++++++++++++-- > > DT bindings and driver changes are separate patches. > > > > 2 files changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt > > index fa63b6171407..c6e8e6aafcce 100644 > > --- a/Documentation/devicetree/bindings/hwmon/ina3221.txt > > +++ b/Documentation/devicetree/bindings/hwmon/ina3221.txt > > @@ -29,6 +29,7 @@ Texas Instruments INA3221 Device Tree Bindings > > Optional properties: > > - label: Name of the input source > > - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm > > + - summation-bypass: exclude from channel summation. > > Convert to DT schema first. I do have a local patch to convert, I'll attach it here so that Ninad can include it in this series. Thierry From d9ae5bbf569516ef0f2d230d934c6a5c0d7ad3a6 Mon Sep 17 00:00:00 2001 From: Thierry Reding <treding@nvidia.com> Date: Thu, 25 Nov 2021 19:33:03 +0100 Subject: [PATCH] dt-bindings: hwmon: ina3221: Convert to json-schema Convert the TI INA3221 bindings from the free-form text format to json-schema. Signed-off-by: Thierry Reding <treding@nvidia.com> --- .../devicetree/bindings/hwmon/ina3221.txt | 54 --------- .../devicetree/bindings/hwmon/ti,ina3221.yaml | 109 ++++++++++++++++++ 2 files changed, 109 insertions(+), 54 deletions(-) delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt deleted file mode 100644 index fa63b6171407..000000000000 --- a/Documentation/devicetree/bindings/hwmon/ina3221.txt +++ /dev/null @@ -1,54 +0,0 @@ -Texas Instruments INA3221 Device Tree Bindings - -1) ina3221 node - Required properties: - - compatible: Must be "ti,ina3221" - - reg: I2C address - - Optional properties: - - ti,single-shot: This chip has two power modes: single-shot (chip takes one - measurement and then shuts itself down) and continuous ( - chip takes continuous measurements). The continuous mode is - more reliable and suitable for hardware monitor type device, - but the single-shot mode is more power-friendly and useful - for battery-powered device which cares power consumptions - while still needs some measurements occasionally. - If this property is present, the single-shot mode will be - used, instead of the default continuous one for monitoring. - - = The node contains optional child nodes for three channels = - = Each child node describes the information of input source = - - - #address-cells: Required only if a child node is present. Must be 1. - - #size-cells: Required only if a child node is present. Must be 0. - -2) child nodes - Required properties: - - reg: Must be 0, 1 or 2, corresponding to IN1, IN2 or IN3 port of INA3221 - - Optional properties: - - label: Name of the input source - - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm - -Example: - -ina3221@40 { - compatible = "ti,ina3221"; - reg = <0x40>; - #address-cells = <1>; - #size-cells = <0>; - - input@0 { - reg = <0x0>; - status = "disabled"; - }; - input@1 { - reg = <0x1>; - shunt-resistor-micro-ohms = <5000>; - }; - input@2 { - reg = <0x2>; - label = "VDD_5V"; - shunt-resistor-micro-ohms = <5000>; - }; -}; diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file mode 100644 index 000000000000..0c6d41423d8c --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/ti,ina3221.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments INA3221 Current and Voltage Monitor + +maintainers: + - Jean Delvare <jdelvare@suse.com> + - Guenter Roeck <linux@roeck-us.net> + +properties: + compatible: + const: ti,ina3221 + + reg: + maxItems: 1 + + ti,single-shot: + description: | + This chip has two power modes: single-shot (chip takes one measurement + and then shuts itself down) and continuous (chip takes continuous + measurements). The continuous mode is more reliable and suitable for + hardware monitor type device, but the single-shot mode is more power- + friendly and useful for battery-powered device which cares power + consumptions while still needs some measurements occasionally. + + If this property is present, the single-shot mode will be used, instead + of the default continuous one for monitoring. + $ref: /schemas/types.yaml#/definitions/flag + + "#address-cells": + description: Required only if a child node is present. + const: 1 + + "#size-cells": + description: Required only if a child node is present. + const: 0 + +patternProperties: + "^input@[0-2]$": + description: The node contains optional child nodes for three channels. + Each child node describes the information of input source. + type: object + properties: + reg: + description: Must be 0, 1 and 2, corresponding to the IN1, IN2 or IN3 + ports of the INA3221, respectively. + enum: [ 0, 1, 2 ] + + label: + description: name of the input source + + shunt-resistor-micro-ohms: + description: shunt resistor value in micro-Ohm + + additionalProperties: false + + required: + - reg + +additionalProperties: false + +required: + - compatible + - reg + +examples: + - | + #include <dt-bindings/clock/tegra186-clock.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/reset/tegra186-reset.h> + + i2c@3160000 { + compatible = "nvidia,tegra186-i2c"; + reg = <0x03160000 0x10000>; + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&bpmp TEGRA186_CLK_I2C1>; + clock-names = "div-clk"; + resets = <&bpmp TEGRA186_RESET_I2C1>; + reset-names = "i2c"; + + #address-cells = <1>; + #size-cells = <0>; + + ina3221@40 { + compatible = "ti,ina3221"; + reg = <0x40>; + #address-cells = <1>; + #size-cells = <0>; + + input@0 { + reg = <0x0>; + status = "disabled"; + }; + + input@1 { + reg = <0x1>; + shunt-resistor-micro-ohms = <5000>; + }; + + input@2 { + reg = <0x2>; + label = "VDD_5V"; + shunt-resistor-micro-ohms = <5000>; + }; + }; + };
On Tue, Nov 08, 2022 at 12:52:43PM +0800, Ninad Malwade wrote: > Add support to initialize summation channel control via kernel device > tree property "summation-bypass". The channel which has this property > is excluded from channel summation. > > Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> > --- > .../devicetree/bindings/hwmon/ina3221.txt | 2 ++ > drivers/hwmon/ina3221.c | 14 ++++++++++++-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt > index fa63b6171407..c6e8e6aafcce 100644 > --- a/Documentation/devicetree/bindings/hwmon/ina3221.txt > +++ b/Documentation/devicetree/bindings/hwmon/ina3221.txt > @@ -29,6 +29,7 @@ Texas Instruments INA3221 Device Tree Bindings > Optional properties: > - label: Name of the input source > - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm > + - summation-bypass: exclude from channel summation. > > Example: > > @@ -41,6 +42,7 @@ ina3221@40 { > input@0 { > reg = <0x0>; > status = "disabled"; > + summation-bypass; > }; > input@1 { > reg = <0x1>; > diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c > index 2a57f4b60c29..ba0d6da06947 100644 > --- a/drivers/hwmon/ina3221.c > +++ b/drivers/hwmon/ina3221.c > @@ -104,6 +104,7 @@ struct ina3221_input { > const char *label; > int shunt_resistor; > bool disconnected; > + bool summation_bypass; > }; > > /** > @@ -125,6 +126,7 @@ struct ina3221_data { > struct mutex lock; > u32 reg_config; > int summation_shunt_resistor; > + u32 summation_channel_control; > > bool single_shot; > }; > @@ -154,7 +156,8 @@ static inline int ina3221_summation_shunt_resistor(struct ina3221_data *ina) > int i, shunt_resistor = 0; > > for (i = 0; i < INA3221_NUM_CHANNELS; i++) { > - if (input[i].disconnected || !input[i].shunt_resistor) > + if (input[i].disconnected || !input[i].shunt_resistor || > + input[i].summation_bypass) > continue; > if (!shunt_resistor) { > /* Found the reference shunt resistor value */ > @@ -786,6 +789,9 @@ static int ina3221_probe_child_from_dt(struct device *dev, > /* Save the connected input label if available */ > of_property_read_string(child, "label", &input->label); > > + /* summation channel control */ > + input->summation_bypass = of_property_read_bool(child, "summation-bypass"); > + > /* Overwrite default shunt resistor value optionally */ > if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", &val)) { > if (val < 1 || val > INT_MAX) { > @@ -873,6 +879,10 @@ static int ina3221_probe(struct i2c_client *client) > > /* Initialize summation_shunt_resistor for summation channel control */ > ina->summation_shunt_resistor = ina3221_summation_shunt_resistor(ina); > + for (i = 0; i < INA3221_NUM_CHANNELS; i++) { > + if (!ina->inputs[i].summation_bypass) > + ina->summation_channel_control |= (BIT(14 - i)); No need for the outer parentheses. Otherwise looks good, so with those parentheses dropped, this is: Reviewed-by: Thierry Reding <treding@nvidia.com>
diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt index fa63b6171407..c6e8e6aafcce 100644 --- a/Documentation/devicetree/bindings/hwmon/ina3221.txt +++ b/Documentation/devicetree/bindings/hwmon/ina3221.txt @@ -29,6 +29,7 @@ Texas Instruments INA3221 Device Tree Bindings Optional properties: - label: Name of the input source - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm + - summation-bypass: exclude from channel summation. Example: @@ -41,6 +42,7 @@ ina3221@40 { input@0 { reg = <0x0>; status = "disabled"; + summation-bypass; }; input@1 { reg = <0x1>; diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c index 2a57f4b60c29..ba0d6da06947 100644 --- a/drivers/hwmon/ina3221.c +++ b/drivers/hwmon/ina3221.c @@ -104,6 +104,7 @@ struct ina3221_input { const char *label; int shunt_resistor; bool disconnected; + bool summation_bypass; }; /** @@ -125,6 +126,7 @@ struct ina3221_data { struct mutex lock; u32 reg_config; int summation_shunt_resistor; + u32 summation_channel_control; bool single_shot; }; @@ -154,7 +156,8 @@ static inline int ina3221_summation_shunt_resistor(struct ina3221_data *ina) int i, shunt_resistor = 0; for (i = 0; i < INA3221_NUM_CHANNELS; i++) { - if (input[i].disconnected || !input[i].shunt_resistor) + if (input[i].disconnected || !input[i].shunt_resistor || + input[i].summation_bypass) continue; if (!shunt_resistor) { /* Found the reference shunt resistor value */ @@ -786,6 +789,9 @@ static int ina3221_probe_child_from_dt(struct device *dev, /* Save the connected input label if available */ of_property_read_string(child, "label", &input->label); + /* summation channel control */ + input->summation_bypass = of_property_read_bool(child, "summation-bypass"); + /* Overwrite default shunt resistor value optionally */ if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", &val)) { if (val < 1 || val > INT_MAX) { @@ -873,6 +879,10 @@ static int ina3221_probe(struct i2c_client *client) /* Initialize summation_shunt_resistor for summation channel control */ ina->summation_shunt_resistor = ina3221_summation_shunt_resistor(ina); + for (i = 0; i < INA3221_NUM_CHANNELS; i++) { + if (!ina->inputs[i].summation_bypass) + ina->summation_channel_control |= (BIT(14 - i)); + } ina->pm_dev = dev; mutex_init(&ina->lock); @@ -984,7 +994,7 @@ static int ina3221_resume(struct device *dev) */ ret = regmap_update_bits(ina->regmap, INA3221_MASK_ENABLE, INA3221_MASK_ENABLE_SCC_MASK, - INA3221_MASK_ENABLE_SCC_MASK); + ina->summation_channel_control); if (ret) { dev_err(dev, "Unable to control summation channel\n"); return ret;
Add support to initialize summation channel control via kernel device tree property "summation-bypass". The channel which has this property is excluded from channel summation. Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> --- .../devicetree/bindings/hwmon/ina3221.txt | 2 ++ drivers/hwmon/ina3221.c | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-)