[PATCH v2 1/2] hwmon: ina2xx: convert driver to using regmap

From: Marc Titinger
Date: Tue Oct 27 2015 - 05:52:18 EST

Any sysfs "show" read access from the client app will result in reading
all registers (8 with ina226). Depending on the host this can limit the
best achievable read rate.

This changeset allows for individual register accesses through regmap.

Tested with BeagleBone Black (Baylibre-ACME) and ina226.

Signed-off-by: Marc Titinger <mtitinger@xxxxxxxxxxxx>
---

v2:
- rename 'rv' to 'regval' for clarity
- fix missed smbus_xxx api change to regmap
- rename ina2xx_do_update to ina2xx_read_reg
- fix indentation

drivers/hwmon/ina2xx.c | 211 +++++++++++++++++++------------------------------
1 file changed, 82 insertions(+), 129 deletions(-)

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 4d28150..5e7ada8 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -37,6 +37,7 @@
#include <linux/of.h>
#include <linux/delay.h>
#include <linux/util_macros.h>
+#include <linux/regmap.h>

#include <linux/platform_data/ina2xx.h>

@@ -84,6 +85,11 @@
*/
#define INA226_TOTAL_CONV_TIME_DEFAULT 2200

+static struct regmap_config ina2xx_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+};
+
enum ina2xx_ids { ina219, ina226 };

struct ina2xx_config {
@@ -97,20 +103,13 @@ struct ina2xx_config {
};

struct ina2xx_data {
- struct i2c_client *client;
const struct ina2xx_config *config;

long rshunt;
- u16 curr_config;
-
- struct mutex update_lock;
- bool valid;
- unsigned long last_updated;
- int update_interval; /* in jiffies */
+ struct regmap *regmap;

int kind;
const struct attribute_group *groups[INA2XX_MAX_ATTRIBUTE_GROUPS];
- u16 regs[INA2XX_MAX_REGISTERS];
};

static const struct ina2xx_config ina2xx_config[] = {
@@ -153,7 +152,11 @@ static int ina226_reg_to_interval(u16 config)
return DIV_ROUND_CLOSEST(avg * INA226_TOTAL_CONV_TIME_DEFAULT, 1000);
}

-static u16 ina226_interval_to_reg(int interval, u16 config)
+/*
+ * Return the new, shifted AVG field value of CONFIG register,
+ * to use with regmap_update_bits
+ */
+static u16 ina226_interval_to_reg(int interval)
{
int avg, avg_bits;

@@ -162,15 +165,7 @@ static u16 ina226_interval_to_reg(int interval, u16 config)
avg_bits = find_closest(avg, ina226_avg_tab,
ARRAY_SIZE(ina226_avg_tab));

- return (config & ~INA226_AVG_RD_MASK) | INA226_SHIFT_AVG(avg_bits);
-}
-
-static void ina226_set_update_interval(struct ina2xx_data *data)
-{
- int ms;
-
- ms = ina226_reg_to_interval(data->curr_config);
- data->update_interval = msecs_to_jiffies(ms);
+ return INA226_SHIFT_AVG(avg_bits);
}

static int ina2xx_calibrate(struct ina2xx_data *data)
@@ -178,8 +173,7 @@ static int ina2xx_calibrate(struct ina2xx_data *data)
u16 val = DIV_ROUND_CLOSEST(data->config->calibration_factor,
data->rshunt);

- return i2c_smbus_write_word_swapped(data->client,
- INA2XX_CALIBRATION, val);
+ return regmap_write(data->regmap, INA2XX_CALIBRATION, val);
}

/*
@@ -187,12 +181,8 @@ static int ina2xx_calibrate(struct ina2xx_data *data)
*/
static int ina2xx_init(struct ina2xx_data *data)
{
- struct i2c_client *client = data->client;
- int ret;
-
- /* device configuration */
- ret = i2c_smbus_write_word_swapped(client, INA2XX_CONFIG,
- data->curr_config);
+ int ret = regmap_write(data->regmap, INA2XX_CONFIG,
+ data->config->config_default);
if (ret < 0)
return ret;

@@ -203,47 +193,52 @@ static int ina2xx_init(struct ina2xx_data *data)
return ina2xx_calibrate(data);
}

-static int ina2xx_do_update(struct device *dev)
+static int ina2xx_read_reg(struct device *dev, int reg, unsigned int *regval)
{
struct ina2xx_data *data = dev_get_drvdata(dev);
- struct i2c_client *client = data->client;
- int i, rv, retry;
+ int ret, retry;

- dev_dbg(&client->dev, "Starting ina2xx update\n");
+ dev_dbg(dev, "Starting register %d read\n", reg);

for (retry = 5; retry; retry--) {
- /* Read all registers */
- for (i = 0; i < data->config->registers; i++) {
- rv = i2c_smbus_read_word_swapped(client, i);
- if (rv < 0)
- return rv;
- data->regs[i] = rv;
- }
+
+ ret = regmap_read(data->regmap, reg, regval);
+ if (ret < 0)
+ return ret;
+
+ dev_dbg(dev, "read %d, val = 0x%04x\n", reg, *regval);

/*
* If the current value in the calibration register is 0, the
* power and current registers will also remain at 0. In case
* the chip has been reset let's check the calibration
* register and reinitialize if needed.
+ * We do that extra read of the calibration register if there
+ * is some hint of a chip reset.
*/
- if (data->regs[INA2XX_CALIBRATION] == 0) {
- dev_warn(dev, "chip not calibrated, reinitializing\n");
-
- rv = ina2xx_init(data);
- if (rv < 0)
- return rv;
-
- /*
- * Let's make sure the power and current registers
- * have been updated before trying again.
- */
- msleep(INA2XX_MAX_DELAY);
- continue;
+ if (*regval == 0) {
+ unsigned int cal;
+
+ ret = regmap_read(data->regmap, INA2XX_CALIBRATION,
+ &cal);
+ if (ret < 0)
+ return ret;
+
+ if (cal == 0) {
+ dev_warn(dev, "chip not calibrated, reinitializing\n");
+
+ ret = ina2xx_init(data);
+ if (ret < 0)
+ return ret;
+ /*
+ * Let's make sure the power and current
+ * registers have been updated before trying
+ * again.
+ */
+ msleep(INA2XX_MAX_DELAY);
+ continue;
+ }
}
-
- data->last_updated = jiffies;
- data->valid = 1;
-
return 0;
}

@@ -256,51 +251,31 @@ static int ina2xx_do_update(struct device *dev)
return -ENODEV;
}

-static struct ina2xx_data *ina2xx_update_device(struct device *dev)
-{
- struct ina2xx_data *data = dev_get_drvdata(dev);
- struct ina2xx_data *ret = data;
- unsigned long after;
- int rv;
-
- mutex_lock(&data->update_lock);
-
- after = data->last_updated + data->update_interval;
- if (time_after(jiffies, after) || !data->valid) {
- rv = ina2xx_do_update(dev);
- if (rv < 0)
- ret = ERR_PTR(rv);
- }
-
- mutex_unlock(&data->update_lock);
- return ret;
-}
-
-static int ina2xx_get_value(struct ina2xx_data *data, u8 reg)
+static int ina2xx_get_value(struct ina2xx_data *data, u8 reg,
+ unsigned int regval)
{
int val;

switch (reg) {
case INA2XX_SHUNT_VOLTAGE:
/* signed register */
- val = DIV_ROUND_CLOSEST((s16)data->regs[reg],
- data->config->shunt_div);
+ val = DIV_ROUND_CLOSEST((s16)regval, data->config->shunt_div);
break;
case INA2XX_BUS_VOLTAGE:
- val = (data->regs[reg] >> data->config->bus_voltage_shift)
+ val = (regval >> data->config->bus_voltage_shift)
* data->config->bus_voltage_lsb;
val = DIV_ROUND_CLOSEST(val, 1000);
break;
case INA2XX_POWER:
- val = data->regs[reg] * data->config->power_lsb;
+ val = regval * data->config->power_lsb;
break;
case INA2XX_CURRENT:
/* signed register, LSB=1mA (selected), in mA */
- val = (s16)data->regs[reg];
+ val = (s16)regval;
break;
case INA2XX_CALIBRATION:
val = DIV_ROUND_CLOSEST(data->config->calibration_factor,
- data->regs[reg]);
+ regval);
break;
default:
/* programmer goofed */
@@ -316,25 +291,25 @@ static ssize_t ina2xx_show_value(struct device *dev,
struct device_attribute *da, char *buf)
{
struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
- struct ina2xx_data *data = ina2xx_update_device(dev);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ unsigned int regval;
+
+ int err = ina2xx_read_reg(dev, attr->index, &regval);

- if (IS_ERR(data))
- return PTR_ERR(data);
+ if (err < 0)
+ return err;

return snprintf(buf, PAGE_SIZE, "%d\n",
- ina2xx_get_value(data, attr->index));
+ ina2xx_get_value(data, attr->index, regval));
}

static ssize_t ina2xx_set_shunt(struct device *dev,
struct device_attribute *da,
const char *buf, size_t count)
{
- struct ina2xx_data *data = ina2xx_update_device(dev);
unsigned long val;
int status;
-
- if (IS_ERR(data))
- return PTR_ERR(data);
+ struct ina2xx_data *data = dev_get_drvdata(dev);

status = kstrtoul(buf, 10, &val);
if (status < 0)
@@ -345,10 +320,8 @@ static ssize_t ina2xx_set_shunt(struct device *dev,
val > data->config->calibration_factor)
return -EINVAL;

- mutex_lock(&data->update_lock);
data->rshunt = val;
status = ina2xx_calibrate(data);
- mutex_unlock(&data->update_lock);
if (status < 0)
return status;

@@ -370,17 +343,9 @@ static ssize_t ina226_set_interval(struct device *dev,
if (val > INT_MAX || val == 0)
return -EINVAL;

- mutex_lock(&data->update_lock);
- data->curr_config = ina226_interval_to_reg(val,
- data->regs[INA2XX_CONFIG]);
- status = i2c_smbus_write_word_swapped(data->client,
- INA2XX_CONFIG,
- data->curr_config);
-
- ina226_set_update_interval(data);
- /* Make sure the next access re-reads all registers. */
- data->valid = 0;
- mutex_unlock(&data->update_lock);
+ status = regmap_update_bits(data->regmap, INA2XX_CONFIG,
+ INA226_AVG_RD_MASK,
+ ina226_interval_to_reg(val));
if (status < 0)
return status;

@@ -390,18 +355,15 @@ static ssize_t ina226_set_interval(struct device *dev,
static ssize_t ina226_show_interval(struct device *dev,
struct device_attribute *da, char *buf)
{
- struct ina2xx_data *data = ina2xx_update_device(dev);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ int status;
+ unsigned int regval;

- if (IS_ERR(data))
- return PTR_ERR(data);
+ status = regmap_read(data->regmap, INA2XX_CONFIG, &regval);
+ if (status)
+ return status;

- /*
- * We don't use data->update_interval here as we want to display
- * the actual interval used by the chip and jiffies_to_msecs()
- * doesn't seem to be accurate enough.
- */
- return snprintf(buf, PAGE_SIZE, "%d\n",
- ina226_reg_to_interval(data->regs[INA2XX_CONFIG]));
+ return snprintf(buf, PAGE_SIZE, "%d\n", ina226_reg_to_interval(regval));
}

/* shunt voltage */
@@ -455,7 +417,6 @@ static const struct attribute_group ina226_group = {
static int ina2xx_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- struct i2c_adapter *adapter = client->adapter;
struct ina2xx_platform_data *pdata;
struct device *dev = &client->dev;
struct ina2xx_data *data;
@@ -463,9 +424,6 @@ static int ina2xx_probe(struct i2c_client *client,
u32 val;
int ret, group = 0;

- if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
- return -ENODEV;
-
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -483,30 +441,25 @@ static int ina2xx_probe(struct i2c_client *client,
/* set the device type */
data->kind = id->driver_data;
data->config = &ina2xx_config[data->kind];
- data->curr_config = data->config->config_default;
- data->client = client;
-
- /*
- * Ina226 has a variable update_interval. For ina219 we
- * use a constant value.
- */
- if (data->kind == ina226)
- ina226_set_update_interval(data);
- else
- data->update_interval = HZ / INA2XX_CONVERSION_RATE;

if (data->rshunt <= 0 ||
data->rshunt > data->config->calibration_factor)
return -ENODEV;

+ ina2xx_regmap_config.max_register = data->config->registers;
+
+ data->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);
+ if (IS_ERR(data->regmap)) {
+ dev_err(dev, "failed to allocate register map\n");
+ return PTR_ERR(data->regmap);
+ }
+
ret = ina2xx_init(data);
if (ret < 0) {
dev_err(dev, "error configuring the device: %d\n", ret);
return -ENODEV;
}

- mutex_init(&data->update_lock);
-
data->groups[group++] = &ina2xx_group;
if (data->kind == ina226)
data->groups[group++] = &ina226_group;
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/