| Message ID | 20230921130818.21247-1-jonathanh@nvidia.com |
|---|---|
| Headers | show |
| Series | hwmon: ina3221: Add selective summation support | expand |
On Thu, Sep 21, 2023 at 02:08:16PM +0100, Jon Hunter wrote: > The INA3221 has a critical alert pin that can be controlled by the > summation control function. This function adds the single > shunt-voltage conversions for the desired channels in order to > compare the combined sum to the programmed limit. The Shunt-Voltage > Sum Limit register contains the programmed value that is compared > to the value in the Shunt-Voltage Sum register in order to > determine if the total summed limit is exceeded. If the > shunt-voltage sum limit value is exceeded, the critical alert pin > pulls low. > > For the summation limit to have a meaningful value, it is necessary > to use the same shunt-resistor value on all included channels. Add a new > vendor specific property, 'ti,summation-disable', to allow specific > channels to be excluded from the summation control function if the shunt > resistor is different to other channels or the channel should not be > considered for triggering the critical alert pin. You are adding this feature to the driver, but requiring a new property to disable it. So what happens with an existing user (old DT) and a kernel with the new feature? Rob
On 22/09/2023 22:01, Rob Herring wrote: > On Thu, Sep 21, 2023 at 02:08:15PM +0100, Jon Hunter wrote: >> From: Ninad Malwade <nmalwade@nvidia.com> >> >> Convert the TI INA3221 bindings from the free-form text format to >> json-schema. >> >> Note that the INA3221 input channels default to enabled in the chip. >> Unless channels are explicitly disabled in device-tree, input >> channels will be enabled. >> >> Signed-off-by: Thierry Reding <treding@nvidia.com> >> Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> .../devicetree/bindings/hwmon/ina3221.txt | 54 ---------- >> .../devicetree/bindings/hwmon/ti,ina3221.yaml | 98 +++++++++++++++++++ >> 2 files changed, 98 insertions(+), 54 deletions(-) >> delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt >> create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml ... >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + power-sensor@40 { >> + compatible = "ti,ina3221"; >> + reg = <0x40>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + input@0 { >> + reg = <0x0>; >> + status = "disabled"; > > Examples should be enabled. Yes normally that would be the case. However, per the discussion with Guenter and the comment in the changelog, for this device channels are enabled in the chip by default. And so to disable them, we need to explicitly disable in device-tree. Jon
On 22/09/2023 22:06, Rob Herring wrote: > On Thu, Sep 21, 2023 at 02:08:16PM +0100, Jon Hunter wrote: >> The INA3221 has a critical alert pin that can be controlled by the >> summation control function. This function adds the single >> shunt-voltage conversions for the desired channels in order to >> compare the combined sum to the programmed limit. The Shunt-Voltage >> Sum Limit register contains the programmed value that is compared >> to the value in the Shunt-Voltage Sum register in order to >> determine if the total summed limit is exceeded. If the >> shunt-voltage sum limit value is exceeded, the critical alert pin >> pulls low. >> >> For the summation limit to have a meaningful value, it is necessary >> to use the same shunt-resistor value on all included channels. Add a new >> vendor specific property, 'ti,summation-disable', to allow specific >> channels to be excluded from the summation control function if the shunt >> resistor is different to other channels or the channel should not be >> considered for triggering the critical alert pin. > > You are adding this feature to the driver, but requiring a new property > to disable it. So what happens with an existing user (old DT) and a > kernel with the new feature? Not exactly. The summation support has always been supported in the driver and is enabled (if the shunt resistors for all channels are the same). What we want to do is support summation but only for a subset of channels which is not supported today. So this new property allows us to explicitly tell the driver not to include a specific channel in the summation. Jon