Re: [PATCH V2 3/4] hwmon: ina3221: add support for summation channel control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Aug 26, 2023 at 12:42:48AM +0800, Ninad Malwade wrote:
> The INA3221 allows the Critical alert pin to 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,
> we have to use the same shunt-resistor value on all included
> channels. Unless equal shunt-resistor values are used for
> each channel, we can't use summation control function to add
> the individual conversion values directly together in the
> Shunt-Voltage Sum register to report the total current.
> 
> To address this we add support to BYPASS channels
> via kernel device tree property "summation-bypass".
> 
> The channel which has this property would be excluded from
> the calculation of summation control function, so we can easily
> exclude the one which uses different shunt-resistor value or
> bus voltage.
> 
> For example, summation control function calculates
> Shunt-Voltage Sum like
> - input_shunt_voltage_summaion = input_shunt_voltage_channel1

summation

>                                + input_shunt_voltage_channel2
>                                + input_shunt_voltage_channel3
> 
> But if we want the summation to consider only channel1
> and channel3, we can add 'summation-bypass' property
> in device tree node of channel2.
> 
> Then the calculation will skip channel2.
> - input_shunt_voltage_summaion = input_shunt_voltage_channel1
>                                + input_shunt_voltage_channel3
> 
summation

> Please note that we only want the channel to be skipped
> for summation control function rather than completely disabled.
> Therefore, even if we add the device tree node, the functionality
> of the single channel would still be retained.
> 
> The below sysfs nodes are added to check if the channel is included
> or excluded from the summation control function.
> 
> in*_sum_bypass = 0 --> channel voltage is included for sum of
>                        shunt voltages.
> 
> in*_sum_bypass = 1 --> channel voltage is excluded for sum
>                        of shunt voltages.
> 
This is not an acceptable sysfs attribute. Use debugfs.

> Signed-off-by: Rajkumar Kasirajan <rkasirajan@xxxxxxxxxx>
> Signed-off-by: Ninad Malwade <nmalwade@xxxxxxxxxx>
> ---
>  drivers/hwmon/ina3221.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
> index 5ab944056ec0..093ebf9f1f8d 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 */
> @@ -731,10 +734,29 @@ static SENSOR_DEVICE_ATTR_RW(shunt1_resistor, ina3221_shunt, INA3221_CHANNEL1);
>  static SENSOR_DEVICE_ATTR_RW(shunt2_resistor, ina3221_shunt, INA3221_CHANNEL2);
>  static SENSOR_DEVICE_ATTR_RW(shunt3_resistor, ina3221_shunt, INA3221_CHANNEL3);
>  
> +static ssize_t ina3221_summation_bypass_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr);
> +	struct ina3221_data *ina = dev_get_drvdata(dev);
> +	unsigned int channel = sd_attr->index;
> +	struct ina3221_input *input = &ina->inputs[channel];
> +
> +	return sysfs_emit(buf, "%d\n", input->summation_bypass);
> +}
> +
> +/* summation bypass */
> +static SENSOR_DEVICE_ATTR_RO(in1_sum_bypass, ina3221_summation_bypass, INA3221_CHANNEL1);
> +static SENSOR_DEVICE_ATTR_RO(in2_sum_bypass, ina3221_summation_bypass, INA3221_CHANNEL2);
> +static SENSOR_DEVICE_ATTR_RO(in3_sum_bypass, ina3221_summation_bypass, INA3221_CHANNEL3);
> +

As mentioned, use debugfs to display this information.

>  static struct attribute *ina3221_attrs[] = {
>  	&sensor_dev_attr_shunt1_resistor.dev_attr.attr,
>  	&sensor_dev_attr_shunt2_resistor.dev_attr.attr,
>  	&sensor_dev_attr_shunt3_resistor.dev_attr.attr,
> +	&sensor_dev_attr_in1_sum_bypass.dev_attr.attr,
> +	&sensor_dev_attr_in2_sum_bypass.dev_attr.attr,
> +	&sensor_dev_attr_in3_sum_bypass.dev_attr.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(ina3221);
> @@ -786,6 +808,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 +898,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));

Unnecessary ( ) around BIT().

> +	}
>  
>  	ina->pm_dev = dev;
>  	mutex_init(&ina->lock);
> @@ -978,13 +1007,13 @@ static int ina3221_resume(struct device *dev)
>  	/* Initialize summation channel control */
>  	if (ina->summation_shunt_resistor) {
>  		/*
> -		 * Take all three channels into summation by default
> -		 * Shunt measurements of disconnected channels should
> -		 * be 0, so it does not matter for summation.
> +		 * Sum only channels that are not 'bypassed' for summation
> +		 * by default. Shunt measurements of disconnected channels

Drop "by default".

> +		 * should be 0, so it does not matter for summation.
>  		 */
>  		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;
> -- 
> 2.17.1
> 



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux