On 29/08/2023 14:48, Jon Hunter wrote: > > > On 26/08/2023 09:56, Krzysztof Kozlowski wrote: >> On 25/08/2023 18:42, Ninad Malwade 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 property, 'summation-bypass', to allow specific channels to be >>> excluded from the summation control function if the shunt resistor >>> is different to other channels. >>> >>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx> >>> Signed-off-by: Ninad Malwade <nmalwade@xxxxxxxxxx> >>> --- >>> .../devicetree/bindings/hwmon/ti,ina3221.yaml | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml >>> index 0c6d41423d8c..20c23febf575 100644 >>> --- a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml >>> +++ b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml >>> @@ -55,6 +55,24 @@ patternProperties: >>> shunt-resistor-micro-ohms: >>> description: shunt resistor value in micro-Ohm >>> >>> + summation-bypass: >> >> What is the type? There is no vendor prefix here, so you added it as a >> generic property. Which other devices use or can use it? >> >>> + description: | >>> + 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. If >>> + this is not the case for specific channels, then the >>> + 'summation-bypass' can be populated for a specific channel to "populated" is confusing here. I guess you wanted "can be used"? >>> + exclude from the summation control function. >> >> I don't understand what this property does. You described feature in the >> device, that's good, but how does it map to the property? Bypass means >> disable? > > > Yes it means 'disable'. I kept as 'bypass' to align with the original > patch [0], but if it is clearer, we could change this to be > 'summation-disable'. Sounds better, but the description could also start with it, e.g. "Disable the summation on specific channel. .... " Best regards, Krzysztof
