Re: [PATCH] ina3221: add support for summation channel control

[Thierry Reding]

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@xxxxxxxxxx>
> ---
>  .../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@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature