[PATCH] hwmon: ina238: Add support for shunt voltage scaling

Potin Lai posted 1 patch 11 months ago
drivers/hwmon/ina238.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
[PATCH] hwmon: ina238: Add support for shunt voltage scaling
Posted by Potin Lai 11 months ago
The INA238 sensor reports shunt voltage with microvolt precision.
However, the hwmon driver currently exposes this value only in
millivolts via `in0_input`, which results in a loss of precision for
readings within the range of ±1 mV.

This patch introduces an `in0_scale` attribute to provide the scaling
factor applied to the shunt voltage reading. By exposing this attribute,
users can accurately interpret the in0_input values in microvolts,
preserving the sensor's full precision.

Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
---
 drivers/hwmon/ina238.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 2d9f12f68d50..58737a0703dc 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -8,6 +8,7 @@
 
 #include <linux/err.h>
 #include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -108,8 +109,42 @@ struct ina238_data {
 	struct regmap *regmap;
 	u32 rshunt;
 	int gain;
+	long shunt_volt_scale;
 };
 
+static ssize_t shunt_volt_scale_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", data->shunt_volt_scale);
+}
+
+static ssize_t shunt_volt_scale_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+	long val;
+	int err;
+
+	err = kstrtol(buf, 10, &val);
+	if (err)
+		return err;
+
+	data->shunt_volt_scale = val;
+	return count;
+}
+
+static SENSOR_DEVICE_ATTR_RW(in0_scale, shunt_volt_scale, 0);
+
+static struct attribute *ina238_attrs[] = {
+	&sensor_dev_attr_in0_scale.dev_attr.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(ina238);
+
 static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
 {
 	u8 data[3];
@@ -197,8 +232,9 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 		regval = (s16)regval;
 		if (channel == 0)
 			/* gain of 1 -> LSB / 4 */
-			*val = (regval * INA238_SHUNT_VOLTAGE_LSB) /
-			       (1000 * (4 - data->gain + 1));
+			*val = (regval * INA238_SHUNT_VOLTAGE_LSB *
+				data->shunt_volt_scale) /
+				(1000 * (4 - data->gain + 1));
 		else
 			*val = (regval * INA238_BUS_VOLTAGE_LSB) / 1000;
 		break;
@@ -603,9 +639,12 @@ static int ina238_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
+	/* Setup default shunt voltage scale */
+	data->shunt_volt_scale = 1;
+
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
 							 &ina238_chip_info,
-							 NULL);
+							 ina238_groups);
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 

---
base-commit: fc033cf25e612e840e545f8d5ad2edd6ba613ed5
change-id: 20250121-potin-ina238-shunt-voltage-scaling-8a3f9dc3d2b8

Best regards,
-- 
Potin Lai <potin.lai.pt@gmail.com>

Re: [PATCH] hwmon: ina238: Add support for shunt voltage scaling
Posted by kernel test robot 11 months ago
Hi Potin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on fc033cf25e612e840e545f8d5ad2edd6ba613ed5]

url:    https://github.com/intel-lab-lkp/linux/commits/Potin-Lai/hwmon-ina238-Add-support-for-shunt-voltage-scaling/20250121-002826
base:   fc033cf25e612e840e545f8d5ad2edd6ba613ed5
patch link:    https://lore.kernel.org/r/20250121-potin-ina238-shunt-voltage-scaling-v1-1-36d5dfe027f5%40gmail.com
patch subject: [PATCH] hwmon: ina238: Add support for shunt voltage scaling
config: x86_64-buildonly-randconfig-004-20250121 (https://download.01.org/0day-ci/archive/20250121/202501211015.8Szwv3Cd-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250121/202501211015.8Szwv3Cd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501211015.8Szwv3Cd-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/ina238.c:120:30: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
     120 |         return sprintf(buf, "%d\n", data->shunt_volt_scale);
         |                              ~~     ^~~~~~~~~~~~~~~~~~~~~~
         |                              %ld
   1 warning generated.


vim +120 drivers/hwmon/ina238.c

   114	
   115	static ssize_t shunt_volt_scale_show(struct device *dev,
   116					     struct device_attribute *attr, char *buf)
   117	{
   118		struct ina238_data *data = dev_get_drvdata(dev);
   119	
 > 120		return sprintf(buf, "%d\n", data->shunt_volt_scale);
   121	}
   122	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] hwmon: ina238: Add support for shunt voltage scaling
Posted by kernel test robot 11 months ago
Hi Potin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on fc033cf25e612e840e545f8d5ad2edd6ba613ed5]

url:    https://github.com/intel-lab-lkp/linux/commits/Potin-Lai/hwmon-ina238-Add-support-for-shunt-voltage-scaling/20250121-002826
base:   fc033cf25e612e840e545f8d5ad2edd6ba613ed5
patch link:    https://lore.kernel.org/r/20250121-potin-ina238-shunt-voltage-scaling-v1-1-36d5dfe027f5%40gmail.com
patch subject: [PATCH] hwmon: ina238: Add support for shunt voltage scaling
config: i386-buildonly-randconfig-004-20250121 (https://download.01.org/0day-ci/archive/20250121/202501210454.YUnMEUaA-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250121/202501210454.YUnMEUaA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501210454.YUnMEUaA-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/hwmon/ina238.c: In function 'shunt_volt_scale_show':
>> drivers/hwmon/ina238.c:120:31: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
     120 |         return sprintf(buf, "%d\n", data->shunt_volt_scale);
         |                              ~^     ~~~~~~~~~~~~~~~~~~~~~~
         |                               |         |
         |                               int       long int
         |                              %ld


vim +120 drivers/hwmon/ina238.c

   114	
   115	static ssize_t shunt_volt_scale_show(struct device *dev,
   116					     struct device_attribute *attr, char *buf)
   117	{
   118		struct ina238_data *data = dev_get_drvdata(dev);
   119	
 > 120		return sprintf(buf, "%d\n", data->shunt_volt_scale);
   121	}
   122	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] hwmon: ina238: Add support for shunt voltage scaling
Posted by Guenter Roeck 11 months ago
On 1/20/25 08:23, Potin Lai wrote:
> The INA238 sensor reports shunt voltage with microvolt precision.
> However, the hwmon driver currently exposes this value only in
> millivolts via `in0_input`, which results in a loss of precision for
> readings within the range of ±1 mV.
> 
> This patch introduces an `in0_scale` attribute to provide the scaling
> factor applied to the shunt voltage reading. By exposing this attribute,
> users can accurately interpret the in0_input values in microvolts,
> preserving the sensor's full precision.
> 
> Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>

Sorry, this is an ABI violation and thus a no-go. in0_input is and will
always be reported in mV.

Guenter

Re: [PATCH] hwmon: ina238: Add support for shunt voltage scaling
Posted by Potin Lai 11 months ago
On Tue, Jan 21, 2025 at 12:38 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 1/20/25 08:23, Potin Lai wrote:
> > The INA238 sensor reports shunt voltage with microvolt precision.
> > However, the hwmon driver currently exposes this value only in
> > millivolts via `in0_input`, which results in a loss of precision for
> > readings within the range of ±1 mV.
> >
> > This patch introduces an `in0_scale` attribute to provide the scaling
> > factor applied to the shunt voltage reading. By exposing this attribute,
> > users can accurately interpret the in0_input values in microvolts,
> > preserving the sensor's full precision.
> >
> > Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
>
> Sorry, this is an ABI violation and thus a no-go. in0_input is and will
> always be reported in mV.
>
> Guenter
>

Hi Guenter,
In our use case, the shunt voltage is less than 1 mv most of the time.
I would like to get your advice on the correct way of getting
microvolt reading from the driver? Thank you.

Best regards,
Potin
Re: [PATCH] hwmon: ina238: Add support for shunt voltage scaling
Posted by Guenter Roeck 11 months ago
On 1/20/25 17:18, Potin Lai wrote:
> On Tue, Jan 21, 2025 at 12:38 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 1/20/25 08:23, Potin Lai wrote:
>>> The INA238 sensor reports shunt voltage with microvolt precision.
>>> However, the hwmon driver currently exposes this value only in
>>> millivolts via `in0_input`, which results in a loss of precision for
>>> readings within the range of ±1 mV.
>>>
>>> This patch introduces an `in0_scale` attribute to provide the scaling
>>> factor applied to the shunt voltage reading. By exposing this attribute,
>>> users can accurately interpret the in0_input values in microvolts,
>>> preserving the sensor's full precision.
>>>
>>> Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
>>
>> Sorry, this is an ABI violation and thus a no-go. in0_input is and will
>> always be reported in mV.
>>
>> Guenter
>>
> 
> Hi Guenter,
> In our use case, the shunt voltage is less than 1 mv most of the time.
> I would like to get your advice on the correct way of getting
> microvolt reading from the driver? Thank you.
> 

Given that V = I * R, assuming you know the shunt resistor value,
it is an easy calculation which does not require any code changes.

Possible alternatives:
- Create a debugfs file to display the voltage in uV.
   See drivers/hwmon/isl28022.c for an example.
- Implement a hwmon -> iio bridge and use IIO to report
   the voltage.

Having said that, do you have an actual use case ? What is the point
of knowing the voltage across the shunt resistor ? "Because the chip
reports it" is not a use case. Literally _every_ current sensor chip
ultimately reports the shunt resistor voltage, after all (typically
as current; calculation see above). Commit 63fb21afc1f5 ("hwmon:
(ina2xx) Use shunt voltage to calculate current") is a case in point.

Guenter