The user (or an init script) may setup RShunt via sysfs after the
driver was initialized, for instance based on the EEPROM contents
of a modular probe. The calibration register must be set accordingly.
Signed-off-by: Marc Titinger <[email protected]>
---
tested with BeagleBone-black and BayLibre-acme.
---
drivers/iio/adc/ina2xx-adc.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 65909d5..e5ac120 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -350,6 +350,23 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,
return len;
}
+/*
+ * Set current LSB to 1mA, shunt is in uOhms
+ * (equation 13 in datasheet). We hardcode a Current_LSB
+ * of 1.0 x10-6. The only remaining parameter is RShunt.
+ * There is no need to expose the CALIBRATION register
+ * to the user for now. But we need to reset this register
+ * if the user updates RShunt after driver init, e.g upon
+ * reading an EEPROM/Probe-type value.
+ */
+static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)
+{
+ u16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
+ chip->shunt_resistor);
+
+ return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
+}
+
static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)
{
if (val <= 0 || val > chip->config->calibration_factor)
@@ -385,6 +402,11 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
if (ret)
return ret;
+ /* Update the Calibration register */
+ ret = ina2xx_set_calibration(chip);
+ if (ret)
+ return ret;
+
return len;
}
@@ -599,6 +621,8 @@ static const struct iio_info ina2xx_info = {
.debugfs_reg_access = ina2xx_debug_reg,
};
+
+
/* Initialize the configuration and calibration registers. */
static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
{
@@ -609,17 +633,7 @@ static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
if (ret)
return ret;
- /*
- * Set current LSB to 1mA, shunt is in uOhms
- * (equation 13 in datasheet). We hardcode a Current_LSB
- * of 1.0 x10-6. The only remaining parameter is RShunt.
- * There is no need to expose the CALIBRATION register
- * to the user for now.
- */
- regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
- chip->shunt_resistor);
-
- return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
+ return ina2xx_set_calibration(chip);
}
static int ina2xx_probe(struct i2c_client *client,
--
2.5.0
The scale would result in uV instead of expected mV.
Mostly cosmetic, since the value of 'Power' was computed OK.
Signed-off-by: Marc Titinger <[email protected]>
---
drivers/iio/adc/ina2xx-adc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index e5ac120..6ada336 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -185,9 +185,9 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SCALE:
switch (chan->address) {
case INA2XX_SHUNT_VOLTAGE:
- /* processed (mV) = raw*1000/shunt_div */
+ /* processed (mV) = raw/shunt_div */
*val2 = chip->config->shunt_div;
- *val = 1000;
+ *val = 1;
return IIO_VAL_FRACTIONAL;
case INA2XX_BUS_VOLTAGE:
--
2.5.0
On 03/11/2016 08:52 AM, Marc Titinger wrote:
> The user (or an init script) may setup RShunt via sysfs after the
> driver was initialized, for instance based on the EEPROM contents
> of a modular probe. The calibration register must be set accordingly.
>
> Signed-off-by: Marc Titinger <[email protected]>
> ---
> tested with BeagleBone-black and BayLibre-acme.
> ---
> drivers/iio/adc/ina2xx-adc.c | 36 +++++++++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 65909d5..e5ac120 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -350,6 +350,23 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,
> return len;
> }
>
> +/*
> + * Set current LSB to 1mA, shunt is in uOhms
> + * (equation 13 in datasheet). We hardcode a Current_LSB
> + * of 1.0 x10-6. The only remaining parameter is RShunt.
> + * There is no need to expose the CALIBRATION register
> + * to the user for now. But we need to reset this register
> + * if the user updates RShunt after driver init, e.g upon
> + * reading an EEPROM/Probe-type value.
> + */
> +static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)
> +{
> + u16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
> + chip->shunt_resistor);
> +
> + return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
> +}
> +
> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)
> {
> if (val <= 0 || val > chip->config->calibration_factor)
> @@ -385,6 +402,11 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
> if (ret)
> return ret;
>
> + /* Update the Calibration register */
> + ret = ina2xx_set_calibration(chip);
> + if (ret)
> + return ret;
> +
> return len;
> }
>
> @@ -599,6 +621,8 @@ static const struct iio_info ina2xx_info = {
> .debugfs_reg_access = ina2xx_debug_reg,
> };
>
> +
> +
?
> /* Initialize the configuration and calibration registers. */
> static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
> {
> @@ -609,17 +633,7 @@ static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
> if (ret)
> return ret;
>
> - /*
> - * Set current LSB to 1mA, shunt is in uOhms
> - * (equation 13 in datasheet). We hardcode a Current_LSB
> - * of 1.0 x10-6. The only remaining parameter is RShunt.
> - * There is no need to expose the CALIBRATION register
> - * to the user for now.
> - */
> - regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
> - chip->shunt_resistor);
> -
> - return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
> + return ina2xx_set_calibration(chip);
> }
>
> static int ina2xx_probe(struct i2c_client *client,
>
On 11/03/2016 16:04, Andrew F. Davis wrote:
...
>>
>> @@ -599,6 +621,8 @@ static const struct iio_info ina2xx_info = {
>> .debugfs_reg_access = ina2xx_debug_reg,
>> };
>>
>> +
>> +
>
> ?
>
Ok, will fix in v2, thanks !
M.
>> /* Initialize the configuration and calibration registers. */
>> static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
>> {
>> @@ -609,17 +633,7 @@ static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
>> if (ret)
>> return ret;
>>
>> - /*
>> - * Set current LSB to 1mA, shunt is in uOhms
>> - * (equation 13 in datasheet). We hardcode a Current_LSB
>> - * of 1.0 x10-6. The only remaining parameter is RShunt.
>> - * There is no need to expose the CALIBRATION register
>> - * to the user for now.
>> - */
>> - regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
>> - chip->shunt_resistor);
>> -
>> - return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
>> + return ina2xx_set_calibration(chip);
>> }
>>
>> static int ina2xx_probe(struct i2c_client *client,
>>
Hi Marc,
[auto build test WARNING on iio/togreg]
[also build test WARNING on next-20160311]
[cannot apply to v4.5-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Marc-Titinger/iio-ina2xx-adc-update-the-CALIB-register-when-RShunt-changes/20160311-225616
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-randconfig-s1-201610 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/iio/adc/ina2xx-adc.c: In function 'ina2xx_init':
>> drivers/iio/adc/ina2xx-adc.c:629:6: warning: unused variable 'regval' [-Wunused-variable]
u16 regval;
^
vim +/regval +629 drivers/iio/adc/ina2xx-adc.c
c43a102e Marc Titinger 2015-12-07 613 .attrs = ina2xx_attributes,
c43a102e Marc Titinger 2015-12-07 614 };
c43a102e Marc Titinger 2015-12-07 615
c43a102e Marc Titinger 2015-12-07 616 static const struct iio_info ina2xx_info = {
c43a102e Marc Titinger 2015-12-07 617 .driver_module = THIS_MODULE,
7906dd52 Andrew F. Davis 2016-02-24 618 .attrs = &ina2xx_attribute_group,
7906dd52 Andrew F. Davis 2016-02-24 619 .read_raw = ina2xx_read_raw,
7906dd52 Andrew F. Davis 2016-02-24 620 .write_raw = ina2xx_write_raw,
7906dd52 Andrew F. Davis 2016-02-24 621 .debugfs_reg_access = ina2xx_debug_reg,
c43a102e Marc Titinger 2015-12-07 622 };
c43a102e Marc Titinger 2015-12-07 623
76011646 Marc Titinger 2016-03-11 624
76011646 Marc Titinger 2016-03-11 625
c43a102e Marc Titinger 2015-12-07 626 /* Initialize the configuration and calibration registers. */
c43a102e Marc Titinger 2015-12-07 627 static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
c43a102e Marc Titinger 2015-12-07 628 {
c43a102e Marc Titinger 2015-12-07 @629 u16 regval;
7906dd52 Andrew F. Davis 2016-02-24 630 int ret;
c43a102e Marc Titinger 2015-12-07 631
7906dd52 Andrew F. Davis 2016-02-24 632 ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
7906dd52 Andrew F. Davis 2016-02-24 633 if (ret)
c43a102e Marc Titinger 2015-12-07 634 return ret;
7906dd52 Andrew F. Davis 2016-02-24 635
76011646 Marc Titinger 2016-03-11 636 return ina2xx_set_calibration(chip);
c43a102e Marc Titinger 2015-12-07 637 }
:::::: The code at line 629 was first introduced by commit
:::::: c43a102e67db99c8bfe6e8a9280cec13ff53b789 iio: ina2xx: add support for TI INA2xx Power Monitors
:::::: TO: Marc Titinger <[email protected]>
:::::: CC: Jonathan Cameron <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 11/03/16 14:52, Marc Titinger wrote:
> The user (or an init script) may setup RShunt via sysfs after the
> driver was initialized, for instance based on the EEPROM contents
> of a modular probe. The calibration register must be set accordingly.
>
> Signed-off-by: Marc Titinger <[email protected]>
Other than the two little fixes (Andrew's and the autobuilder one)
this series looks good to me. Will pick up v2.
I think both of these are fixes that will want to go through the fixes
tree after the merge window has closed anyway.
Jonathan
> ---
> tested with BeagleBone-black and BayLibre-acme.
> ---
> drivers/iio/adc/ina2xx-adc.c | 36 +++++++++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 65909d5..e5ac120 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -350,6 +350,23 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,
> return len;
> }
>
> +/*
> + * Set current LSB to 1mA, shunt is in uOhms
> + * (equation 13 in datasheet). We hardcode a Current_LSB
> + * of 1.0 x10-6. The only remaining parameter is RShunt.
> + * There is no need to expose the CALIBRATION register
> + * to the user for now. But we need to reset this register
> + * if the user updates RShunt after driver init, e.g upon
> + * reading an EEPROM/Probe-type value.
> + */
> +static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)
> +{
> + u16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
> + chip->shunt_resistor);
> +
> + return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
> +}
> +
> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)
> {
> if (val <= 0 || val > chip->config->calibration_factor)
> @@ -385,6 +402,11 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
> if (ret)
> return ret;
>
> + /* Update the Calibration register */
> + ret = ina2xx_set_calibration(chip);
> + if (ret)
> + return ret;
> +
> return len;
> }
>
> @@ -599,6 +621,8 @@ static const struct iio_info ina2xx_info = {
> .debugfs_reg_access = ina2xx_debug_reg,
> };
>
> +
> +
> /* Initialize the configuration and calibration registers. */
> static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
> {
> @@ -609,17 +633,7 @@ static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
> if (ret)
> return ret;
>
> - /*
> - * Set current LSB to 1mA, shunt is in uOhms
> - * (equation 13 in datasheet). We hardcode a Current_LSB
> - * of 1.0 x10-6. The only remaining parameter is RShunt.
> - * There is no need to expose the CALIBRATION register
> - * to the user for now.
> - */
> - regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
> - chip->shunt_resistor);
> -
> - return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
> + return ina2xx_set_calibration(chip);
> }
>
> static int ina2xx_probe(struct i2c_client *client,
>
On 12/03/2016 10:19, Jonathan Cameron wrote:
> On 11/03/16 14:52, Marc Titinger wrote:
>> The user (or an init script) may setup RShunt via sysfs after the
>> driver was initialized, for instance based on the EEPROM contents
>> of a modular probe. The calibration register must be set accordingly.
>>
>> Signed-off-by: Marc Titinger <[email protected]>
> Other than the two little fixes (Andrew's and the autobuilder one)
> this series looks good to me. Will pick up v2.
>
> I think both of these are fixes that will want to go through the fixes
> tree after the merge window has closed anyway.
Thanks Jon, sending v2 ASAP.
M.
>
> Jonathan
>> ---
>> tested with BeagleBone-black and BayLibre-acme.
>> ---
>> drivers/iio/adc/ina2xx-adc.c | 36 +++++++++++++++++++++++++-----------
>> 1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
...