linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] iio: adc: ina2xx: Shift bus voltage register to mask flag bits
       [not found] <20171028211249.24148-1-stefan.bruens@rwth-aachen.de>
@ 2017-10-28 21:12 ` Stefan Brüns
  2017-11-19 16:13   ` Jonathan Cameron
  2017-10-28 21:12 ` [PATCH v2 2/3] iio: adc: ina2xx: Use LSB specifier instead of divider in config Stefan Brüns
  2017-10-28 21:12 ` [PATCH v2 3/3] iio: adc: ina2xx: Allow setting Shunt Voltage PGA gain and Bus Voltage range Stefan Brüns
  2 siblings, 1 reply; 7+ messages in thread
From: Stefan Brüns @ 2017-10-28 21:12 UTC (permalink / raw)
  To: linux-iio
  Cc: Peter Meerwald-Stadler, Stefan Brüns, linux-kernel,
	Andrew F . Davis, Lars-Peter Clausen, Jonathan Cameron,
	Hartmut Knaack, Javier Martinez Canillas

Lower bits of the INA219/220 bus voltage register are conversion
status flags, properly shift the value.

When reading via IIO buffer, the value is passed on unaltered,
shifting is the responsibility of the user.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

---

Changes in v2:
- Apply to the shunt voltage, not bus voltage register
- Shift instead of masking the LSBs

 drivers/iio/adc/ina2xx-adc.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 3aff9556678f..84094235ff7e 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -44,7 +44,6 @@
 
 #define INA226_MASK_ENABLE		0x06
 #define INA226_CVRF			BIT(3)
-#define INA219_CNVR			BIT(1)
 
 #define INA2XX_MAX_REGISTERS            8
 
@@ -79,6 +78,11 @@
 #define INA226_ITS_MASK		GENMASK(5, 3)
 #define INA226_SHIFT_ITS(val)	((val) << 3)
 
+/* INA219 Bus voltage register, low bits are flags */
+#define INA219_OVF		BIT(0)
+#define INA219_CNVR		BIT(1)
+#define INA219_BUS_VOLTAGE_SHIFT	3
+
 /* Cosmetic macro giving the sampling period for a full P=UxI cycle */
 #define SAMPLING_PERIOD(c)	((c->int_time_vbus + c->int_time_vshunt) \
 				 * c->avg)
@@ -112,7 +116,7 @@ struct ina2xx_config {
 	u16 config_default;
 	int calibration_factor;
 	int shunt_div;
-	int bus_voltage_shift;
+	int bus_voltage_shift;	/* position of lsb */
 	int bus_voltage_lsb;	/* uV */
 	int power_lsb;		/* uW */
 	enum ina2xx_ids chip_id;
@@ -135,7 +139,7 @@ static const struct ina2xx_config ina2xx_config[] = {
 		.config_default = INA219_CONFIG_DEFAULT,
 		.calibration_factor = 40960000,
 		.shunt_div = 100,
-		.bus_voltage_shift = 3,
+		.bus_voltage_shift = INA219_BUS_VOLTAGE_SHIFT,
 		.bus_voltage_lsb = 4000,
 		.power_lsb = 20000,
 		.chip_id = ina219,
@@ -170,6 +174,9 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
 		else
 			*val  = regval;
 
+		if (chan->address == INA2XX_BUS_VOLTAGE)
+			*val >>= chip->config->bus_voltage_shift;
+
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -203,9 +210,9 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
 			return IIO_VAL_FRACTIONAL;
 
 		case INA2XX_BUS_VOLTAGE:
-			/* processed (mV) = raw*lsb (uV) / (1000 << shift) */
+			/* processed (mV) = raw * lsb (uV) / 1000 */
 			*val = chip->config->bus_voltage_lsb;
-			*val2 = 1000 << chip->config->bus_voltage_shift;
+			*val2 = 1000;
 			return IIO_VAL_FRACTIONAL;
 
 		case INA2XX_POWER:
@@ -532,7 +539,7 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
  * Sampling Freq is a consequence of the integration times of
  * the Voltage channels.
  */
-#define INA219_CHAN_VOLTAGE(_index, _address) { \
+#define INA219_CHAN_VOLTAGE(_index, _address, _shift) { \
 	.type = IIO_VOLTAGE, \
 	.address = (_address), \
 	.indexed = 1, \
@@ -544,7 +551,8 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
 	.scan_index = (_index), \
 	.scan_type = { \
 		.sign = 'u', \
-		.realbits = 16, \
+		.shift = _shift, \
+		.realbits = 16 - _shift, \
 		.storagebits = 16, \
 		.endianness = IIO_LE, \
 	} \
@@ -579,8 +587,8 @@ static const struct iio_chan_spec ina226_channels[] = {
 };
 
 static const struct iio_chan_spec ina219_channels[] = {
-	INA219_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
-	INA219_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
+	INA219_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE, 0),
+	INA219_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE, INA219_BUS_VOLTAGE_SHIFT),
 	INA219_CHAN(IIO_POWER, 2, INA2XX_POWER),
 	INA219_CHAN(IIO_CURRENT, 3, INA2XX_CURRENT),
 	IIO_CHAN_SOFT_TIMESTAMP(4),
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/3] iio: adc: ina2xx: Use LSB specifier instead of divider in config
       [not found] <20171028211249.24148-1-stefan.bruens@rwth-aachen.de>
  2017-10-28 21:12 ` [PATCH v2 1/3] iio: adc: ina2xx: Shift bus voltage register to mask flag bits Stefan Brüns
@ 2017-10-28 21:12 ` Stefan Brüns
  2017-11-19 16:15   ` Jonathan Cameron
  2017-10-28 21:12 ` [PATCH v2 3/3] iio: adc: ina2xx: Allow setting Shunt Voltage PGA gain and Bus Voltage range Stefan Brüns
  2 siblings, 1 reply; 7+ messages in thread
From: Stefan Brüns @ 2017-10-28 21:12 UTC (permalink / raw)
  To: linux-iio
  Cc: Peter Meerwald-Stadler, Stefan Brüns, linux-kernel,
	Andrew F . Davis, Lars-Peter Clausen, Jonathan Cameron,
	Hartmut Knaack, Javier Martinez Canillas

While the config uses the physical value corresponding to the LSB
for both the power and the bus voltage register, the shunt voltage is
specified as parts of 1 mV. Use the LSB physical value for all registers.

No functional change.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

---

Changes in v2:
- new patch

 drivers/iio/adc/ina2xx-adc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 84094235ff7e..c59843de3999 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -115,7 +115,7 @@ enum ina2xx_ids { ina219, ina226 };
 struct ina2xx_config {
 	u16 config_default;
 	int calibration_factor;
-	int shunt_div;
+	int shunt_voltage_lsb;	/* nV */
 	int bus_voltage_shift;	/* position of lsb */
 	int bus_voltage_lsb;	/* uV */
 	int power_lsb;		/* uW */
@@ -138,7 +138,7 @@ static const struct ina2xx_config ina2xx_config[] = {
 	[ina219] = {
 		.config_default = INA219_CONFIG_DEFAULT,
 		.calibration_factor = 40960000,
-		.shunt_div = 100,
+		.shunt_voltage_lsb = 10000,
 		.bus_voltage_shift = INA219_BUS_VOLTAGE_SHIFT,
 		.bus_voltage_lsb = 4000,
 		.power_lsb = 20000,
@@ -147,7 +147,7 @@ static const struct ina2xx_config ina2xx_config[] = {
 	[ina226] = {
 		.config_default = INA226_CONFIG_DEFAULT,
 		.calibration_factor = 5120000,
-		.shunt_div = 400,
+		.shunt_voltage_lsb = 2500,
 		.bus_voltage_shift = 0,
 		.bus_voltage_lsb = 1250,
 		.power_lsb = 25000,
@@ -204,9 +204,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/shunt_div */
-			*val2 = chip->config->shunt_div;
-			*val = 1;
+			/* processed (mV) = raw * lsb(nV) / 1000000 */
+			*val = chip->config->shunt_voltage_lsb;
+			*val2 = 1000000;
 			return IIO_VAL_FRACTIONAL;
 
 		case INA2XX_BUS_VOLTAGE:
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/3] iio: adc: ina2xx: Allow setting Shunt Voltage PGA gain and Bus Voltage range
       [not found] <20171028211249.24148-1-stefan.bruens@rwth-aachen.de>
  2017-10-28 21:12 ` [PATCH v2 1/3] iio: adc: ina2xx: Shift bus voltage register to mask flag bits Stefan Brüns
  2017-10-28 21:12 ` [PATCH v2 2/3] iio: adc: ina2xx: Use LSB specifier instead of divider in config Stefan Brüns
@ 2017-10-28 21:12 ` Stefan Brüns
  2017-11-19 16:16   ` Jonathan Cameron
  2 siblings, 1 reply; 7+ messages in thread
From: Stefan Brüns @ 2017-10-28 21:12 UTC (permalink / raw)
  To: linux-iio
  Cc: Peter Meerwald-Stadler, Stefan Brüns, linux-kernel,
	Andrew F . Davis, Lars-Peter Clausen, Jonathan Cameron,
	Hartmut Knaack, Javier Martinez Canillas

Reducing shunt and bus voltage range improves the accuracy, so allow
altering the default settings.

Both settings are exposed as gain values. While for the shunt voltage
this is straightforward, the bus range settings of 32V (default) and 16V
are mapped to gain values of 1 resp. 2, to provide a uniform API to
userspace.

As the gain settings are incorporated into the raw values by the sensor
itself, adjusting of the scale attributes is not necessary.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

---

Changes in v2:
- Fix conversion for shunt gain, function expects milli, not micro

 drivers/iio/adc/ina2xx-adc.c | 111 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index c59843de3999..d05cb1b8efe6 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -48,8 +48,10 @@
 #define INA2XX_MAX_REGISTERS            8
 
 /* settings - depend on use case */
-#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=8 */
+#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=1/8, BRNG=32V */
 #define INA219_DEFAULT_IT		532
+#define INA219_DEFAULT_BRNG             1   /* 32V */
+#define INA219_DEFAULT_PGA              125 /* 1000/8 */
 #define INA226_CONFIG_DEFAULT           0x4327
 #define INA226_DEFAULT_AVG              4
 #define INA226_DEFAULT_IT		1110
@@ -62,6 +64,14 @@
  */
 #define INA2XX_MODE_MASK	GENMASK(3, 0)
 
+/* Gain for VShunt: 1/8 (default), 1/4, 1/2, 1 */
+#define INA219_PGA_MASK		GENMASK(12, 11)
+#define INA219_SHIFT_PGA(val)	((val) << 11)
+
+/* VBus range: 32V (default), 16V */
+#define INA219_BRNG_MASK	BIT(13)
+#define INA219_SHIFT_BRNG(val)	((val) << 13)
+
 /* Averaging for VBus/VShunt/Power */
 #define INA226_AVG_MASK		GENMASK(11, 9)
 #define INA226_SHIFT_AVG(val)	((val) << 9)
@@ -131,6 +141,8 @@ struct ina2xx_chip_info {
 	int avg;
 	int int_time_vbus; /* Bus voltage integration time uS */
 	int int_time_vshunt; /* Shunt voltage integration time uS */
+	int range_vbus; /* Bus voltage maximum in V */
+	int pga_gain_vshunt; /* Shunt voltage PGA gain */
 	bool allow_async_readout;
 };
 
@@ -226,6 +238,18 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
 			*val = 1;
 			return IIO_VAL_INT;
 		}
+
+	case IIO_CHAN_INFO_HARDWAREGAIN:
+		switch (chan->address) {
+		case INA2XX_SHUNT_VOLTAGE:
+			*val = chip->pga_gain_vshunt;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
+
+		case INA2XX_BUS_VOLTAGE:
+			*val = chip->range_vbus == 32 ? 1 : 2;
+			return IIO_VAL_INT;
+		}
 	}
 
 	return -EINVAL;
@@ -360,6 +384,74 @@ static int ina219_set_int_time_vshunt(struct ina2xx_chip_info *chip,
 	return 0;
 }
 
+static const int ina219_vbus_range_tab[] = { 1, 2 };
+static int ina219_set_vbus_range_denom(struct ina2xx_chip_info *chip,
+				       unsigned int range,
+				       unsigned int *config)
+{
+	if (range == 1)
+		chip->range_vbus = 32;
+	else if (range == 2)
+		chip->range_vbus = 16;
+	else
+		return -EINVAL;
+
+	*config &= ~INA219_BRNG_MASK;
+	*config |= INA219_SHIFT_BRNG(range == 1 ? 1 : 0) & INA219_BRNG_MASK;
+
+	return 0;
+}
+
+static const int ina219_vshunt_gain_tab[] = { 125, 250, 500, 1000 };
+static const int ina219_vshunt_gain_frac[] = {
+	125, 1000, 250, 1000, 500, 1000, 1000, 1000 };
+
+static int ina219_set_vshunt_pga_gain(struct ina2xx_chip_info *chip,
+				      unsigned int gain,
+				      unsigned int *config)
+{
+	int bits;
+
+	if (gain < 125 || gain > 1000)
+		return -EINVAL;
+
+	bits = find_closest(gain, ina219_vshunt_gain_tab,
+			    ARRAY_SIZE(ina219_vshunt_gain_tab));
+
+	chip->pga_gain_vshunt = ina219_vshunt_gain_tab[bits];
+	bits = 3 - bits;
+
+	*config &= ~INA219_PGA_MASK;
+	*config |= INA219_SHIFT_PGA(bits) & INA219_PGA_MASK;
+
+	return 0;
+}
+
+static int ina2xx_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_HARDWAREGAIN:
+		switch (chan->address) {
+		case INA2XX_SHUNT_VOLTAGE:
+			*type = IIO_VAL_FRACTIONAL;
+			*length = sizeof(ina219_vshunt_gain_frac) / sizeof(int);
+			*vals = ina219_vshunt_gain_frac;
+			return IIO_AVAIL_LIST;
+
+		case INA2XX_BUS_VOLTAGE:
+			*type = IIO_VAL_INT;
+			*length = sizeof(ina219_vbus_range_tab) / sizeof(int);
+			*vals = ina219_vbus_range_tab;
+			return IIO_AVAIL_LIST;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static int ina2xx_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int val, int val2, long mask)
@@ -402,6 +494,14 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev,
 		}
 		break;
 
+	case IIO_CHAN_INFO_HARDWAREGAIN:
+		if (chan->address == INA2XX_SHUNT_VOLTAGE)
+			ret = ina219_set_vshunt_pga_gain(chip, val * 1000 +
+							 val2 / 1000, &tmp);
+		else
+			ret = ina219_set_vbus_range_denom(chip, val, &tmp);
+		break;
+
 	default:
 		ret = -EINVAL;
 	}
@@ -546,7 +646,10 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
 	.channel = (_index), \
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
 			      BIT(IIO_CHAN_INFO_SCALE) | \
-			      BIT(IIO_CHAN_INFO_INT_TIME), \
+			      BIT(IIO_CHAN_INFO_INT_TIME) | \
+			      BIT(IIO_CHAN_INFO_HARDWAREGAIN), \
+	.info_mask_separate_available = \
+			      BIT(IIO_CHAN_INFO_HARDWAREGAIN), \
 	.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
 	.scan_index = (_index), \
 	.scan_type = { \
@@ -754,7 +857,6 @@ static IIO_CONST_ATTR_NAMED(ina226_integration_time_available,
 			    integration_time_available,
 			    "0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
 
-
 static IIO_DEVICE_ATTR(in_allow_async_readout, S_IRUGO | S_IWUSR,
 		       ina2xx_allow_async_readout_show,
 		       ina2xx_allow_async_readout_store, 0);
@@ -789,6 +891,7 @@ static const struct iio_info ina219_info = {
 	.driver_module = THIS_MODULE,
 	.attrs = &ina219_attribute_group,
 	.read_raw = ina2xx_read_raw,
+	.read_avail = ina2xx_read_avail,
 	.write_raw = ina2xx_write_raw,
 	.debugfs_reg_access = ina2xx_debug_reg,
 };
@@ -870,6 +973,8 @@ static int ina2xx_probe(struct i2c_client *client,
 		chip->avg = 1;
 		ina219_set_int_time_vbus(chip, INA219_DEFAULT_IT, &val);
 		ina219_set_int_time_vshunt(chip, INA219_DEFAULT_IT, &val);
+		ina219_set_vbus_range_denom(chip, INA219_DEFAULT_BRNG, &val);
+		ina219_set_vshunt_pga_gain(chip, INA219_DEFAULT_PGA, &val);
 	}
 
 	ret = ina2xx_init(chip, val);
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] iio: adc: ina2xx: Shift bus voltage register to mask flag bits
  2017-10-28 21:12 ` [PATCH v2 1/3] iio: adc: ina2xx: Shift bus voltage register to mask flag bits Stefan Brüns
@ 2017-11-19 16:13   ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2017-11-19 16:13 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-iio, Peter Meerwald-Stadler, linux-kernel,
	Andrew F . Davis, Lars-Peter Clausen, Hartmut Knaack,
	Javier Martinez Canillas

On Sat, 28 Oct 2017 23:12:46 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> Lower bits of the INA219/220 bus voltage register are conversion
> status flags, properly shift the value.
> 
> When reading via IIO buffer, the value is passed on unaltered,
> shifting is the responsibility of the user.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> 
I thought about sending this as a fix, but as you haven't marked
it as such and the effect is very minor I haven't done so.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,
Jonathan
> ---
> 
> Changes in v2:
> - Apply to the shunt voltage, not bus voltage register
> - Shift instead of masking the LSBs
> 
>  drivers/iio/adc/ina2xx-adc.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 3aff9556678f..84094235ff7e 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -44,7 +44,6 @@
>  
>  #define INA226_MASK_ENABLE		0x06
>  #define INA226_CVRF			BIT(3)
> -#define INA219_CNVR			BIT(1)
>  
>  #define INA2XX_MAX_REGISTERS            8
>  
> @@ -79,6 +78,11 @@
>  #define INA226_ITS_MASK		GENMASK(5, 3)
>  #define INA226_SHIFT_ITS(val)	((val) << 3)
>  
> +/* INA219 Bus voltage register, low bits are flags */
> +#define INA219_OVF		BIT(0)
> +#define INA219_CNVR		BIT(1)
> +#define INA219_BUS_VOLTAGE_SHIFT	3
> +
>  /* Cosmetic macro giving the sampling period for a full P=UxI cycle */
>  #define SAMPLING_PERIOD(c)	((c->int_time_vbus + c->int_time_vshunt) \
>  				 * c->avg)
> @@ -112,7 +116,7 @@ struct ina2xx_config {
>  	u16 config_default;
>  	int calibration_factor;
>  	int shunt_div;
> -	int bus_voltage_shift;
> +	int bus_voltage_shift;	/* position of lsb */
>  	int bus_voltage_lsb;	/* uV */
>  	int power_lsb;		/* uW */
>  	enum ina2xx_ids chip_id;
> @@ -135,7 +139,7 @@ static const struct ina2xx_config ina2xx_config[] = {
>  		.config_default = INA219_CONFIG_DEFAULT,
>  		.calibration_factor = 40960000,
>  		.shunt_div = 100,
> -		.bus_voltage_shift = 3,
> +		.bus_voltage_shift = INA219_BUS_VOLTAGE_SHIFT,
>  		.bus_voltage_lsb = 4000,
>  		.power_lsb = 20000,
>  		.chip_id = ina219,
> @@ -170,6 +174,9 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
>  		else
>  			*val  = regval;
>  
> +		if (chan->address == INA2XX_BUS_VOLTAGE)
> +			*val >>= chip->config->bus_voltage_shift;
> +
>  		return IIO_VAL_INT;
>  
>  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> @@ -203,9 +210,9 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
>  			return IIO_VAL_FRACTIONAL;
>  
>  		case INA2XX_BUS_VOLTAGE:
> -			/* processed (mV) = raw*lsb (uV) / (1000 << shift) */
> +			/* processed (mV) = raw * lsb (uV) / 1000 */
>  			*val = chip->config->bus_voltage_lsb;
> -			*val2 = 1000 << chip->config->bus_voltage_shift;
> +			*val2 = 1000;
>  			return IIO_VAL_FRACTIONAL;
>  
>  		case INA2XX_POWER:
> @@ -532,7 +539,7 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
>   * Sampling Freq is a consequence of the integration times of
>   * the Voltage channels.
>   */
> -#define INA219_CHAN_VOLTAGE(_index, _address) { \
> +#define INA219_CHAN_VOLTAGE(_index, _address, _shift) { \
>  	.type = IIO_VOLTAGE, \
>  	.address = (_address), \
>  	.indexed = 1, \
> @@ -544,7 +551,8 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
>  	.scan_index = (_index), \
>  	.scan_type = { \
>  		.sign = 'u', \
> -		.realbits = 16, \
> +		.shift = _shift, \
> +		.realbits = 16 - _shift, \
>  		.storagebits = 16, \
>  		.endianness = IIO_LE, \
>  	} \
> @@ -579,8 +587,8 @@ static const struct iio_chan_spec ina226_channels[] = {
>  };
>  
>  static const struct iio_chan_spec ina219_channels[] = {
> -	INA219_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
> -	INA219_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
> +	INA219_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE, 0),
> +	INA219_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE, INA219_BUS_VOLTAGE_SHIFT),
>  	INA219_CHAN(IIO_POWER, 2, INA2XX_POWER),
>  	INA219_CHAN(IIO_CURRENT, 3, INA2XX_CURRENT),
>  	IIO_CHAN_SOFT_TIMESTAMP(4),

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/3] iio: adc: ina2xx: Use LSB specifier instead of divider in config
  2017-10-28 21:12 ` [PATCH v2 2/3] iio: adc: ina2xx: Use LSB specifier instead of divider in config Stefan Brüns
@ 2017-11-19 16:15   ` Jonathan Cameron
  2017-11-25 22:05     ` Stefan Brüns
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2017-11-19 16:15 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-iio, Peter Meerwald-Stadler, linux-kernel,
	Andrew F . Davis, Lars-Peter Clausen, Hartmut Knaack,
	Javier Martinez Canillas

On Sat, 28 Oct 2017 23:12:47 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> While the config uses the physical value corresponding to the LSB
> for both the power and the bus voltage register, the shunt voltage is
> specified as parts of 1 mV. Use the LSB physical value for all registers.
> 
> No functional change.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> 
Applied
> ---
> 
> Changes in v2:
> - new patch
> 
>  drivers/iio/adc/ina2xx-adc.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 84094235ff7e..c59843de3999 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -115,7 +115,7 @@ enum ina2xx_ids { ina219, ina226 };
>  struct ina2xx_config {
>  	u16 config_default;
>  	int calibration_factor;
> -	int shunt_div;
> +	int shunt_voltage_lsb;	/* nV */
>  	int bus_voltage_shift;	/* position of lsb */
>  	int bus_voltage_lsb;	/* uV */
>  	int power_lsb;		/* uW */
> @@ -138,7 +138,7 @@ static const struct ina2xx_config ina2xx_config[] = {
>  	[ina219] = {
>  		.config_default = INA219_CONFIG_DEFAULT,
>  		.calibration_factor = 40960000,
> -		.shunt_div = 100,
> +		.shunt_voltage_lsb = 10000,
>  		.bus_voltage_shift = INA219_BUS_VOLTAGE_SHIFT,
>  		.bus_voltage_lsb = 4000,
>  		.power_lsb = 20000,
> @@ -147,7 +147,7 @@ static const struct ina2xx_config ina2xx_config[] = {
>  	[ina226] = {
>  		.config_default = INA226_CONFIG_DEFAULT,
>  		.calibration_factor = 5120000,
> -		.shunt_div = 400,
> +		.shunt_voltage_lsb = 2500,
>  		.bus_voltage_shift = 0,
>  		.bus_voltage_lsb = 1250,
>  		.power_lsb = 25000,
> @@ -204,9 +204,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/shunt_div */
> -			*val2 = chip->config->shunt_div;
> -			*val = 1;
> +			/* processed (mV) = raw * lsb(nV) / 1000000 */
> +			*val = chip->config->shunt_voltage_lsb;
> +			*val2 = 1000000;
>  			return IIO_VAL_FRACTIONAL;
>  
>  		case INA2XX_BUS_VOLTAGE:

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 3/3] iio: adc: ina2xx: Allow setting Shunt Voltage PGA gain and Bus Voltage range
  2017-10-28 21:12 ` [PATCH v2 3/3] iio: adc: ina2xx: Allow setting Shunt Voltage PGA gain and Bus Voltage range Stefan Brüns
@ 2017-11-19 16:16   ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2017-11-19 16:16 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-iio, Peter Meerwald-Stadler, linux-kernel,
	Andrew F . Davis, Lars-Peter Clausen, Hartmut Knaack,
	Javier Martinez Canillas

On Sat, 28 Oct 2017 23:12:48 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> Reducing shunt and bus voltage range improves the accuracy, so allow
> altering the default settings.
> 
> Both settings are exposed as gain values. While for the shunt voltage
> this is straightforward, the bus range settings of 32V (default) and 16V
> are mapped to gain values of 1 resp. 2, to provide a uniform API to
> userspace.
> 
> As the gain settings are incorporated into the raw values by the sensor
> itself, adjusting of the scale attributes is not necessary.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
Applied to the togreg branch of iio.git and pushed out as testing.

thanks,

Jonathan
> 
> ---
> 
> Changes in v2:
> - Fix conversion for shunt gain, function expects milli, not micro
> 
>  drivers/iio/adc/ina2xx-adc.c | 111 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 108 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index c59843de3999..d05cb1b8efe6 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -48,8 +48,10 @@
>  #define INA2XX_MAX_REGISTERS            8
>  
>  /* settings - depend on use case */
> -#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=8 */
> +#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=1/8, BRNG=32V */
>  #define INA219_DEFAULT_IT		532
> +#define INA219_DEFAULT_BRNG             1   /* 32V */
> +#define INA219_DEFAULT_PGA              125 /* 1000/8 */
>  #define INA226_CONFIG_DEFAULT           0x4327
>  #define INA226_DEFAULT_AVG              4
>  #define INA226_DEFAULT_IT		1110
> @@ -62,6 +64,14 @@
>   */
>  #define INA2XX_MODE_MASK	GENMASK(3, 0)
>  
> +/* Gain for VShunt: 1/8 (default), 1/4, 1/2, 1 */
> +#define INA219_PGA_MASK		GENMASK(12, 11)
> +#define INA219_SHIFT_PGA(val)	((val) << 11)
> +
> +/* VBus range: 32V (default), 16V */
> +#define INA219_BRNG_MASK	BIT(13)
> +#define INA219_SHIFT_BRNG(val)	((val) << 13)
> +
>  /* Averaging for VBus/VShunt/Power */
>  #define INA226_AVG_MASK		GENMASK(11, 9)
>  #define INA226_SHIFT_AVG(val)	((val) << 9)
> @@ -131,6 +141,8 @@ struct ina2xx_chip_info {
>  	int avg;
>  	int int_time_vbus; /* Bus voltage integration time uS */
>  	int int_time_vshunt; /* Shunt voltage integration time uS */
> +	int range_vbus; /* Bus voltage maximum in V */
> +	int pga_gain_vshunt; /* Shunt voltage PGA gain */
>  	bool allow_async_readout;
>  };
>  
> @@ -226,6 +238,18 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
>  			*val = 1;
>  			return IIO_VAL_INT;
>  		}
> +
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		switch (chan->address) {
> +		case INA2XX_SHUNT_VOLTAGE:
> +			*val = chip->pga_gain_vshunt;
> +			*val2 = 1000;
> +			return IIO_VAL_FRACTIONAL;
> +
> +		case INA2XX_BUS_VOLTAGE:
> +			*val = chip->range_vbus == 32 ? 1 : 2;
> +			return IIO_VAL_INT;
> +		}
>  	}
>  
>  	return -EINVAL;
> @@ -360,6 +384,74 @@ static int ina219_set_int_time_vshunt(struct ina2xx_chip_info *chip,
>  	return 0;
>  }
>  
> +static const int ina219_vbus_range_tab[] = { 1, 2 };
> +static int ina219_set_vbus_range_denom(struct ina2xx_chip_info *chip,
> +				       unsigned int range,
> +				       unsigned int *config)
> +{
> +	if (range == 1)
> +		chip->range_vbus = 32;
> +	else if (range == 2)
> +		chip->range_vbus = 16;
> +	else
> +		return -EINVAL;
> +
> +	*config &= ~INA219_BRNG_MASK;
> +	*config |= INA219_SHIFT_BRNG(range == 1 ? 1 : 0) & INA219_BRNG_MASK;
> +
> +	return 0;
> +}
> +
> +static const int ina219_vshunt_gain_tab[] = { 125, 250, 500, 1000 };
> +static const int ina219_vshunt_gain_frac[] = {
> +	125, 1000, 250, 1000, 500, 1000, 1000, 1000 };
> +
> +static int ina219_set_vshunt_pga_gain(struct ina2xx_chip_info *chip,
> +				      unsigned int gain,
> +				      unsigned int *config)
> +{
> +	int bits;
> +
> +	if (gain < 125 || gain > 1000)
> +		return -EINVAL;
> +
> +	bits = find_closest(gain, ina219_vshunt_gain_tab,
> +			    ARRAY_SIZE(ina219_vshunt_gain_tab));
> +
> +	chip->pga_gain_vshunt = ina219_vshunt_gain_tab[bits];
> +	bits = 3 - bits;
> +
> +	*config &= ~INA219_PGA_MASK;
> +	*config |= INA219_SHIFT_PGA(bits) & INA219_PGA_MASK;
> +
> +	return 0;
> +}
> +
> +static int ina2xx_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type, int *length,
> +			     long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		switch (chan->address) {
> +		case INA2XX_SHUNT_VOLTAGE:
> +			*type = IIO_VAL_FRACTIONAL;
> +			*length = sizeof(ina219_vshunt_gain_frac) / sizeof(int);
> +			*vals = ina219_vshunt_gain_frac;
> +			return IIO_AVAIL_LIST;
> +
> +		case INA2XX_BUS_VOLTAGE:
> +			*type = IIO_VAL_INT;
> +			*length = sizeof(ina219_vbus_range_tab) / sizeof(int);
> +			*vals = ina219_vbus_range_tab;
> +			return IIO_AVAIL_LIST;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int ina2xx_write_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int val, int val2, long mask)
> @@ -402,6 +494,14 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev,
>  		}
>  		break;
>  
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		if (chan->address == INA2XX_SHUNT_VOLTAGE)
> +			ret = ina219_set_vshunt_pga_gain(chip, val * 1000 +
> +							 val2 / 1000, &tmp);
> +		else
> +			ret = ina219_set_vbus_range_denom(chip, val, &tmp);
> +		break;
> +
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -546,7 +646,10 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
>  	.channel = (_index), \
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>  			      BIT(IIO_CHAN_INFO_SCALE) | \
> -			      BIT(IIO_CHAN_INFO_INT_TIME), \
> +			      BIT(IIO_CHAN_INFO_INT_TIME) | \
> +			      BIT(IIO_CHAN_INFO_HARDWAREGAIN), \
> +	.info_mask_separate_available = \
> +			      BIT(IIO_CHAN_INFO_HARDWAREGAIN), \
>  	.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>  	.scan_index = (_index), \
>  	.scan_type = { \
> @@ -754,7 +857,6 @@ static IIO_CONST_ATTR_NAMED(ina226_integration_time_available,
>  			    integration_time_available,
>  			    "0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
>  
> -
>  static IIO_DEVICE_ATTR(in_allow_async_readout, S_IRUGO | S_IWUSR,
>  		       ina2xx_allow_async_readout_show,
>  		       ina2xx_allow_async_readout_store, 0);
> @@ -789,6 +891,7 @@ static const struct iio_info ina219_info = {
>  	.driver_module = THIS_MODULE,
>  	.attrs = &ina219_attribute_group,
>  	.read_raw = ina2xx_read_raw,
> +	.read_avail = ina2xx_read_avail,
>  	.write_raw = ina2xx_write_raw,
>  	.debugfs_reg_access = ina2xx_debug_reg,
>  };
> @@ -870,6 +973,8 @@ static int ina2xx_probe(struct i2c_client *client,
>  		chip->avg = 1;
>  		ina219_set_int_time_vbus(chip, INA219_DEFAULT_IT, &val);
>  		ina219_set_int_time_vshunt(chip, INA219_DEFAULT_IT, &val);
> +		ina219_set_vbus_range_denom(chip, INA219_DEFAULT_BRNG, &val);
> +		ina219_set_vshunt_pga_gain(chip, INA219_DEFAULT_PGA, &val);
>  	}
>  
>  	ret = ina2xx_init(chip, val);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/3] iio: adc: ina2xx: Use LSB specifier instead of divider in config
  2017-11-19 16:15   ` Jonathan Cameron
@ 2017-11-25 22:05     ` Stefan Brüns
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Brüns @ 2017-11-25 22:05 UTC (permalink / raw)
  To: Maciej Purski
  Cc: Jonathan Cameron, linux-iio, Peter Meerwald-Stadler,
	linux-kernel, Andrew F . Davis, Lars-Peter Clausen,
	Hartmut Knaack, Javier Martinez Canillas

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

On Sunday, November 19, 2017 5:15:03 PM CET Jonathan Cameron wrote:
> On Sat, 28 Oct 2017 23:12:47 +0200
> 
> Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:
> > While the config uses the physical value corresponding to the LSB
> > for both the power and the bus voltage register, the shunt voltage is
> > specified as parts of 1 mV. Use the LSB physical value for all registers.
> > 
> > No functional change.
> > 
> > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> 
> Applied

Hi Maciej,

I think you will have to rebase you calibration register patch on top of this 
series ...

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-11-25 22:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20171028211249.24148-1-stefan.bruens@rwth-aachen.de>
2017-10-28 21:12 ` [PATCH v2 1/3] iio: adc: ina2xx: Shift bus voltage register to mask flag bits Stefan Brüns
2017-11-19 16:13   ` Jonathan Cameron
2017-10-28 21:12 ` [PATCH v2 2/3] iio: adc: ina2xx: Use LSB specifier instead of divider in config Stefan Brüns
2017-11-19 16:15   ` Jonathan Cameron
2017-11-25 22:05     ` Stefan Brüns
2017-10-28 21:12 ` [PATCH v2 3/3] iio: adc: ina2xx: Allow setting Shunt Voltage PGA gain and Bus Voltage range Stefan Brüns
2017-11-19 16:16   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).