Re: [PATCH v6 3/3] hwmon: (isl28022) support shunt voltage for ISL28022 power monitor

From: Guenter Roeck
Date: Fri Sep 06 2024 - 07:57:48 EST


On 9/5/24 23:14, Delphine CC Chiu wrote:
Added support reading shunt voltage in mV and revise code
for Renesas ISL28022.

Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@xxxxxxxxxx>
---
drivers/hwmon/isl28022.c | 93 ++++++++++++++++++++++++++++------------
1 file changed, 66 insertions(+), 27 deletions(-)

diff --git a/drivers/hwmon/isl28022.c b/drivers/hwmon/isl28022.c
index f0494c3bd483..01220fad813d 100644
--- a/drivers/hwmon/isl28022.c
+++ b/drivers/hwmon/isl28022.c
@@ -85,8 +85,6 @@ struct isl28022_data {
u32 shunt;
u32 gain;
u32 average;
- u16 config;
- u16 calib;

I don't see the point of separating this part of the driver from the first patch
in the series. It makes me review code only to be replaced later. I am not going
to do that.

};
static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
@@ -95,20 +93,61 @@ static int isl28022_read(struct device *dev, enum hwmon_sensor_types type,
struct isl28022_data *data = dev_get_drvdata(dev);
unsigned int regval;
int err;
+ u16 sign_bit;
switch (type) {
case hwmon_in:
- switch (attr) {
- case hwmon_in_input:
- err = regmap_read(data->regmap,
- ISL28022_REG_BUS, &regval);
- if (err < 0)
- return err;
- /* driver supports only 60V mode (BRNG 11) */
- *val = (long)(((u16)regval) & 0xFFFC);
+ switch (channel) {
+ case 0:
+ switch (attr) {
+ case hwmon_in_input:
+ err = regmap_read(data->regmap,
+ ISL28022_REG_BUS, &regval);
+ if (err < 0)
+ return err;
+ /* driver supports only 60V mode (BRNG 11) */
+ *val = (long)(((u16)regval) & 0xFFFC);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+ break;
+ case 1:
+ switch (attr) {
+ case hwmon_in_input:
+ err = regmap_read(data->regmap,
+ ISL28022_REG_SHUNT, &regval);
+ if (err < 0)
+ return err;
+ switch (data->gain) {
+ case 8:
+ sign_bit = (regval >> 15) & 0x01;
+ *val = (long)((((u16)regval) & 0x7FFF) -
+ (sign_bit * 32768)) / 100;
+ break;
+ case 4:
+ sign_bit = (regval >> 14) & 0x01;
+ *val = (long)((((u16)regval) & 0x3FFF) -
+ (sign_bit * 16384)) / 100;
+ break;
+ case 2:
+ sign_bit = (regval >> 13) & 0x01;
+ *val = (long)((((u16)regval) & 0x1FFF) -
+ (sign_bit * 8192)) / 100;
+ break;
+ case 1:
+ sign_bit = (regval >> 12) & 0x01;
+ *val = (long)((((u16)regval) & 0x0FFF) -
+ (sign_bit * 4096)) / 100;
+ break;
+ }
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }

Separate into its own voltage read function, and also provide separate functions
for current and power.

Guenter