[PATCH-vs-togreg 1/2] iio: ina2xx-adc: update the CALIB. register when RShunt changes

[PATCH-vs-togreg 1/2] iio: ina2xx-adc: update the CALIB. register when RShunt changes

[Marc Titinger]

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 <marc.titinger@baylibre.com>
---
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

[PATCH-vs-togreg 2/2] iio: ina2xx-adc: fix scale for VShunt

[Marc Titinger]

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 <marc.titinger@baylibre.com>
---
 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

Re: [PATCH-vs-togreg 1/2] iio: ina2xx-adc: update the CALIB. register when RShunt changes

[Andrew F. Davis]

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 <marc.titinger@baylibre.com>
> ---
> 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,
> 

Re: [PATCH-vs-togreg 1/2] iio: ina2xx-adc: update the CALIB. register when RShunt changes

[Marc Titinger]

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,
>>

Re: [PATCH-vs-togreg 1/2] iio: ina2xx-adc: update the CALIB. register when RShunt changes

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]

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 <mtitinger@baylibre.com>
:::::: CC: Jonathan Cameron <jic23@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 21608 bytes --]

Re: [PATCH-vs-togreg 1/2] iio: ina2xx-adc: update the CALIB. register when RShunt changes

[Jonathan Cameron]

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 <marc.titinger@baylibre.com>
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,
> 

Re: [PATCH-vs-togreg 1/2] iio: ina2xx-adc: update the CALIB. register when RShunt changes

[Marc Titinger]

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 <marc.titinger@baylibre.com>
> 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

...