diff mbox series

[09/11] hwmon: (ina2xx) Consolidate chip initialization code

Message ID 20240827153455.1344529-10-linux@roeck-us.net (mailing list archive)
State Superseded
Headers show
Series hwmon: (ina2xx) Cleanup and convert to use with_info API | expand

Commit Message

Guenter Roeck Aug. 27, 2024, 3:34 p.m. UTC
Move all chip initialization code into a single function.

No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina2xx.c | 109 ++++++++++++++++-------------------------
 1 file changed, 41 insertions(+), 68 deletions(-)

Comments

Tzung-Bi Shih Aug. 29, 2024, 2:55 p.m. UTC | #1
On Tue, Aug 27, 2024 at 08:34:53AM -0700, Guenter Roeck wrote:
> +static int ina2xx_init(struct device *dev, struct ina2xx_data *data)
> +{
> +	u16 config = data->config->config_default;
> +	struct regmap *regmap = data->regmap;
> +	u32 shunt;
> +	int ret;
[...]
> +	if (data->chip == ina226 &&
> +	    device_property_read_bool(dev, "ti,alert-polarity-active-high"))
> +		config |= INA226_ALERT_POLARITY;

This looks wrong to me.  The polarity setting should be in INA226_MASK_ENABLE
instead of INA2XX_CONFIG.

> +
> +	ret = regmap_write(regmap, INA2XX_CONFIG, config);
> +	if (ret < 0)
> +		return ret;
> +
Guenter Roeck Aug. 29, 2024, 4:57 p.m. UTC | #2
On 8/29/24 07:55, Tzung-Bi Shih wrote:
> On Tue, Aug 27, 2024 at 08:34:53AM -0700, Guenter Roeck wrote:
>> +static int ina2xx_init(struct device *dev, struct ina2xx_data *data)
>> +{
>> +	u16 config = data->config->config_default;
>> +	struct regmap *regmap = data->regmap;
>> +	u32 shunt;
>> +	int ret;
> [...]
>> +	if (data->chip == ina226 &&
>> +	    device_property_read_bool(dev, "ti,alert-polarity-active-high"))
>> +		config |= INA226_ALERT_POLARITY;
> 
> This looks wrong to me.  The polarity setting should be in INA226_MASK_ENABLE
> instead of INA2XX_CONFIG.
> 

Yes, I fixed that already. Thanks for noticing!

Guenter
diff mbox series

Patch

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index b40fc808bf3d..d1bd998645b6 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -68,9 +68,7 @@ 
 #define INA226_READ_AVG(reg)		FIELD_GET(INA226_AVG_RD_MASK, reg)
 
 #define INA226_ALERT_LATCH_ENABLE	BIT(0)
-#define INA226_ALERT_POLARITY_MASK	BIT(1)
-#define INA226_ALERT_POL_LOW		0
-#define INA226_ALERT_POL_HIGH		1
+#define INA226_ALERT_POLARITY		BIT(1)
 
 /* bit number of alert functions in Mask/Enable Register */
 #define INA226_SHUNT_OVER_VOLTAGE_MASK	BIT(15)
@@ -140,6 +138,7 @@  struct ina2xx_config {
 
 struct ina2xx_data {
 	const struct ina2xx_config *config;
+	enum ina2xx_ids chip;
 
 	long rshunt;
 	long current_lsb_uA;
@@ -210,39 +209,6 @@  static u16 ina226_interval_to_reg(unsigned long interval)
 	return FIELD_PREP(INA226_AVG_RD_MASK, avg_bits);
 }
 
-static int ina2xx_set_alert_polarity(struct ina2xx_data *data,
-				     unsigned long val)
-{
-	return regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
-				 INA226_ALERT_POLARITY_MASK,
-				 FIELD_PREP(INA226_ALERT_POLARITY_MASK, val));
-}
-
-/*
- * Calibration register is set to the best value, which eliminates
- * truncation errors on calculating current register in hardware.
- * According to datasheet (eq. 3) the best values are 2048 for
- * ina226 and 4096 for ina219. They are hardcoded as calibration_value.
- */
-static int ina2xx_calibrate(struct ina2xx_data *data)
-{
-	return regmap_write(data->regmap, INA2XX_CALIBRATION,
-			    data->config->calibration_value);
-}
-
-/*
- * Initialize the configuration and calibration registers.
- */
-static int ina2xx_init(struct ina2xx_data *data)
-{
-	int ret = regmap_write(data->regmap, INA2XX_CONFIG,
-			       data->config->config_default);
-	if (ret < 0)
-		return ret;
-
-	return ina2xx_calibrate(data);
-}
-
 static int ina2xx_read_reg(struct device *dev, int reg, unsigned int *regval)
 {
 	struct ina2xx_data *data = dev_get_drvdata(dev);
@@ -650,12 +616,46 @@  static const struct attribute_group ina226_group = {
 	.attrs = ina226_attrs,
 };
 
+/*
+ * Initialize chip
+ */
+static int ina2xx_init(struct device *dev, struct ina2xx_data *data)
+{
+	u16 config = data->config->config_default;
+	struct regmap *regmap = data->regmap;
+	u32 shunt;
+	int ret;
+
+	if (device_property_read_u32(dev, "shunt-resistor", &shunt) < 0)
+		shunt = INA2XX_RSHUNT_DEFAULT;
+
+	ret = ina2xx_set_shunt(data, shunt);
+	if (ret < 0)
+		return ret;
+
+	if (data->chip == ina226 &&
+	    device_property_read_bool(dev, "ti,alert-polarity-active-high"))
+		config |= INA226_ALERT_POLARITY;
+
+	ret = regmap_write(regmap, INA2XX_CONFIG, config);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Calibration register is set to the best value, which eliminates
+	 * truncation errors on calculating current register in hardware.
+	 * According to datasheet (eq. 3) the best values are 2048 for
+	 * ina226 and 4096 for ina219. They are hardcoded as calibration_value.
+	 */
+	return regmap_write(regmap, INA2XX_CALIBRATION,
+			    data->config->calibration_value);
+}
+
 static int ina2xx_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct ina2xx_data *data;
 	struct device *hwmon_dev;
-	u32 val;
 	int ret, group = 0;
 	enum ina2xx_ids chip;
 
@@ -667,15 +667,9 @@  static int ina2xx_probe(struct i2c_client *client)
 
 	/* set the device type */
 	data->config = &ina2xx_config[chip];
+	data->chip = chip;
 	mutex_init(&data->config_lock);
 
-	if (device_property_read_u32(dev, "shunt-resistor", &val) < 0)
-		val = INA2XX_RSHUNT_DEFAULT;
-
-	ret = ina2xx_set_shunt(data, val);
-	if (ret < 0)
-		return dev_err_probe(dev, ret, "Invalid shunt resistor value\n");
-
 	data->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);
 	if (IS_ERR(data->regmap)) {
 		dev_err(dev, "failed to allocate register map\n");
@@ -686,30 +680,9 @@  static int ina2xx_probe(struct i2c_client *client)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to enable vs regulator\n");
 
-	if (chip == ina226) {
-		if (device_property_read_bool(dev, "ti,alert-polarity-active-high")) {
-			ret = ina2xx_set_alert_polarity(data,
-							INA226_ALERT_POL_HIGH);
-			if (ret < 0) {
-				return dev_err_probe(dev, ret,
-					"failed to set alert polarity active high\n");
-			}
-		} else {
-			/* Set default value i.e active low */
-			ret = ina2xx_set_alert_polarity(data,
-							INA226_ALERT_POL_LOW);
-			if (ret < 0) {
-				return dev_err_probe(dev, ret,
-					"failed to set alert polarity active low\n");
-			}
-		}
-	}
-
-	ret = ina2xx_init(data);
-	if (ret < 0) {
-		dev_err(dev, "error configuring the device: %d\n", ret);
-		return -ENODEV;
-	}
+	ret = ina2xx_init(dev, data);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to configure device\n");
 
 	data->groups[group++] = &ina2xx_group;
 	if (chip == ina226)