On 10/01/2017 09:48 PM, Stefan Brüns wrote:
According to the ABI documentation, the shunt resistor value should be
specificied in Ohm. As this is also used/documented for the MAX9611,
use the same for the INA2xx driver.
This poses an ABI break for anyone actually altering the shunt value
through the sysfs interface, it does not alter the default value nor
a value set from the devicetree.
Minor change: Fix comment, 1mA is 10^-3A.
I have just a minor issue. There could be an inconsistency with units as in my
patch I make current_lsb adjustable and I need it to be in uA (it used to be
hardcoded as 1 mA so to achieve better precision we need smaller units). So in
order to keep calibration register properly scaled, I convert uOhms to mOhms on
each set_calibration(). So if both my changes and your changes were applied, on
each shunt_resistore_store we would be performing multiplication by 10^6 and
then in set_calibration() division by 10^3 which seems odd to me.
I guess we could keep it as shunt_resistor_ohms instead of shunt_resistor_uohm.
We could avoid performing division on each shunt_resistor_show() and perform
multiplication by 10^3 only once in set_calibration() on each
shunt_resistore_store(). We could then change the default value and perform
division only on probing, when reading the shunt_resistance from device tree.
There are many other options. It's not a major issue so maybe we could leave it
as it is or you could suggest some changes in my patch.
Signed-off-by: Stefan Brüns <stefan.bruens@xxxxxxxxxxxxxx>
---
drivers/iio/adc/ina2xx-adc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 361fb4e459d5..1930f853e8c5 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -127,7 +127,7 @@ struct ina2xx_chip_info {
struct task_struct *task;
const struct ina2xx_config *config;
struct mutex state_lock;
- unsigned int shunt_resistor;
+ unsigned int shunt_resistor_uohm;
int avg;
int int_time_vbus; /* Bus voltage integration time uS */
int int_time_vshunt; /* Shunt voltage integration time uS */
@@ -444,7 +444,7 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,
/*
* 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.
+ * of 1.0 x10-3. 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
@@ -453,7 +453,7 @@ static ssize_t ina2xx_allow_async_readout_store(struct device *dev,
static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)
{
u16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
- chip->shunt_resistor);
+ chip->shunt_resistor_uohm);
return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
}
@@ -463,7 +463,7 @@ static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)
if (val <= 0 || val > chip->config->calibration_factor)
return -EINVAL;
- chip->shunt_resistor = val;
+ chip->shunt_resistor_uohm = val;
return 0;
}
@@ -473,8 +473,9 @@ static ssize_t ina2xx_shunt_resistor_show(struct device *dev,
char *buf)
{
struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
+ int vals[2] = { chip->shunt_resistor_uohm, 1000000 };
- return sprintf(buf, "%d\n", chip->shunt_resistor);
+ return iio_format_value(buf, IIO_VAL_FRACTIONAL, 1, vals);
}
static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
@@ -482,14 +483,13 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
const char *buf, size_t len)
{
struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
- unsigned long val;
- int ret;
+ int val, val_fract, ret;
- ret = kstrtoul((const char *) buf, 10, &val);
+ ret = iio_str_to_fixpoint(buf, 100000, &val, &val_fract);
if (ret)
return ret;
- ret = set_shunt_resistor(chip, val);
+ ret = set_shunt_resistor(chip, val * 1000000 + val_fract);
if (ret)
return ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
