[PATCH 1/4] iio: adc: ina2xx: Make max expected current configurable
Maciej Purski
m.purski at samsung.com
Mon Oct 9 01:08:42 PDT 2017
On 10/08/2017 11:47 AM, Jonathan Cameron wrote:
> On Wed, 04 Oct 2017 09:11:31 +0200
> Maciej Purski <m.purski at samsung.com> wrote:
>
>> On 10/01/2017 12:29 PM, Jonathan Cameron wrote:
>>> On Thu, 28 Sep 2017 14:50:12 +0200
>>> Maciej Purski <m.purski at samsung.com> wrote:
>>>
>>>> Max expected current is used for calculating calibration register value,
>>>> Current LSB and Power LSB according to equations found in ina datasheet.
>>>> Max expected current is now implicitly set to default value,
>>>> which is 2^15, thanks to which Current LSB is equal to 1 mA and
>>>> Power LSB is equal to 20000 uW or 25000 uW depending on ina model.
>>>>
>>>> Make max expected current configurable, just like it's already done
>>>> with shunt resistance: from device tree, platform_data or later
>>>> from sysfs. On each max_expected_current change, calculate new values
>>>> for Current LSB and Power LSB. According to datasheet Current LSB should
>>>> be calculated by dividing max expected current by 2^15, as values read
>>>> from device registers are in this case 16-bit integers. Power LSB
>>>> is calculated by multiplying Current LSB by a factor, which is defined
>>>> in ina documentation.
>>>
>>> One odd bit of casting inline. Also this is new userspace ABI.
>>> It needs documenting in
>>>
>>> Documentation/ABI/testing/sysfs-bus-iio* as appropriate.
>>> I'm also unclear on one element about this - is it a value used only
>>> for calibration or are we talking about the actual 'range' of the device?
>>>
>>
>> This is used for calibration. The device measures directly only voltage.
>> However, it has also current and power registers. Their values are
>> calculated by the device using the calibration register which is calculated
>> using max expected current. So I guess that it's not what you mean
>> by the actual 'range' of the device.
>>
>>> The interpretation of this value isn't clear against the more general
>>> ABI.
>>>
>>> In particular it is it in raw units (adc counts) or mA? Docs say
>>> that but the naming of the attribute doesn't make this clear.
>>>
>>
>> It's in mA. I can make it clear in the attribute name.
>>
>>> Also I'm unconvinced this isn't better represented using the
>>> range specifications available for any IIO attribute on the raw
>>> value in combination with adjusting the scale value.
>>> Note not many drivers yet provide ranges on their raw outputs
>>> but we do have core support for it. I've been meaning to start
>>> pushing this out into drivers, but been busy since we introduced
>>> the core support. The dpot-dac driver does use it for examplel
>>>
>>
>>
>> I'm not sure if what I'm about to add is similar to what is done
>> in the mentioned dpot-dac driver. It seems that the callback read_avail
>> returns information on raw values which can be obtained from the device.
>> What I need is an adjustable value, which is then used by the device internally
>> in order to calculate current with the requested precision. Max expected current
>> is also used for calculating the scale value.
>> Tell me if I'm wrong. Maybe I misunderstood the 'range' concept in IIO and
>> your solution fits in here.
>>
>
> I think I answered this in the other branch of the thread.
> _calibscale is what you want here as it's internal to the device.
>
> It's not one often used for ADCs but quite a few other types of
> device provide some front end analog adjustment (whilst it is digital
> here, it is applied within the device - so we don't need to care).
>
> Jonathan
Thank you for your explanation. Calibscale seems suitable for me in this case,
but what do you think I should do then with SCALE attribute? Should I get rid of
it for current and use only calibscale? Or should I use both calibscale and
scale attributes and for current they will be the same value?
I should mention that currenst_lsb value is also used for calculating power_lsb
as they have a fixed ratio (20 or 25) given in the documentation.
Thanks,
Maciej
>
>> Best regards,
>>
>> Maciej
>>> This moves the burden of calculating the 1lsb value to userspace,
>>> but importantly it would give us a consistent ABI where this fits
>>> in with existing elements (largely buy not introducing any new
>>> ones :).
>>>
>>> Thanks,
>>>
>>> Jonathan
>>>>
>>>> Signed-off-by: Maciej Purski <m.purski at samsung.com>
>>>> ---
>>>> drivers/iio/adc/ina2xx-adc.c | 110 ++++++++++++++++++++++++++++++-----
>>>> include/linux/platform_data/ina2xx.h | 2 +
>>>> 2 files changed, 98 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
>>>> index f387b97..883fede 100644
>>>> --- a/drivers/iio/adc/ina2xx-adc.c
>>>> +++ b/drivers/iio/adc/ina2xx-adc.c
>>>> @@ -56,6 +56,7 @@
>>>> #define INA226_DEFAULT_IT 1110
>>>>
>>>> #define INA2XX_RSHUNT_DEFAULT 10000
>>>> +#define INA2XX_MAX_EXPECTED_A_DEFAULT (1 << 15) /* current_lsb = 1 mA */
>>>>
>>>> /*
>>>> * bit masks for reading the settings in the configuration register
>>>> @@ -114,7 +115,7 @@ struct ina2xx_config {
>>>> int shunt_div;
>>>> int bus_voltage_shift;
>>>> int bus_voltage_lsb; /* uV */
>>>> - int power_lsb; /* uW */
>>>> + int power_lsb_factor;
>>>> enum ina2xx_ids chip_id;
>>>> };
>>>>
>>>> @@ -123,7 +124,10 @@ 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; /* uOhms */
>>>> + unsigned int max_expected_current; /* mA */
>>>> + int current_lsb; /* uA */
>>>> + int power_lsb; /* uW */
>>>> int avg;
>>>> int int_time_vbus; /* Bus voltage integration time uS */
>>>> int int_time_vshunt; /* Shunt voltage integration time uS */
>>>> @@ -137,7 +141,7 @@ static const struct ina2xx_config ina2xx_config[] = {
>>>> .shunt_div = 100,
>>>> .bus_voltage_shift = 3,
>>>> .bus_voltage_lsb = 4000,
>>>> - .power_lsb = 20000,
>>>> + .power_lsb_factor = 20,
>>>> .chip_id = ina219,
>>>> },
>>>> [ina226] = {
>>>> @@ -146,7 +150,7 @@ static const struct ina2xx_config ina2xx_config[] = {
>>>> .shunt_div = 400,
>>>> .bus_voltage_shift = 0,
>>>> .bus_voltage_lsb = 1250,
>>>> - .power_lsb = 25000,
>>>> + .power_lsb_factor = 25,
>>>> .chip_id = ina226,
>>>> },
>>>> };
>>>> @@ -210,14 +214,15 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
>>>>
>>>> case INA2XX_POWER:
>>>> /* processed (mW) = raw*lsb (uW) / 1000 */
>>>> - *val = chip->config->power_lsb;
>>>> + *val = chip->power_lsb;
>>>> *val2 = 1000;
>>>> return IIO_VAL_FRACTIONAL;
>>>>
>>>> case INA2XX_CURRENT:
>>>> - /* processed (mA) = raw (mA) */
>>>> - *val = 1;
>>>> - return IIO_VAL_INT;
>>>> + /* processed (mA) = raw*lsb (uA) / 1000 */
>>>> + *val = chip->current_lsb;
>>>> + *val2 = 1000;
>>>> + return IIO_VAL_FRACTIONAL;
>>>> }
>>>> }
>>>>
>>>> @@ -434,24 +439,47 @@ 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.
>>>> + * Calculate calibration value according to equation 1 in ina226 datasheet
>>>> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.
>>>> + * Current LSB is in uA and RShunt is in uOhms, so RShunt should be
>>>> + * converted to mOhms in order to keep the scale.
>>>> * 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.
>>>> + * if the user updates RShunt or max expected current after driver
>>>> + * init, e.g upon reading an EEPROM/Probe-type value.
>>>> */
>>>> static int ina2xx_set_calibration(struct ina2xx_chip_info *chip)
>>>> {
>>>> + unsigned int rshunt = DIV_ROUND_CLOSEST(chip->shunt_resistor, 1000);
>>>> u16 regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
>>>> - chip->shunt_resistor);
>>>> + chip->current_lsb * rshunt);
>>>>
>>>> return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
>>>> }
>>>>
>>>> +/*
>>>> + * Set max_expected_current (mA) and calculate current_lsb (uA),
>>>> + * according to equation 2 in ina226 datasheet. Power LSB is calculated
>>>> + * by multiplying Current LSB by a given factor, which may vary depending
>>>> + * on ina version.
>>>> + */
>>>> +static int set_max_expected_current(struct ina2xx_chip_info *chip,
>>>> + unsigned int val)
>>>> +{
>>>> + if (val <= 0 || val > chip->config->calibration_factor)
>>>> + return -EINVAL;
>>>> +
>>>> + chip->max_expected_current = val;
>>>> + chip->current_lsb = DIV_ROUND_CLOSEST(chip->max_expected_current * 1000,
>>>> + 1 << 15);
>>>> + chip->power_lsb = chip->current_lsb * chip->config->power_lsb_factor;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int set_shunt_resistor(struct ina2xx_chip_info *chip, unsigned int val)
>>>> {
>>>> +
>>>> if (val <= 0 || val > chip->config->calibration_factor)
>>>> return -EINVAL;
>>>>
>>>> @@ -493,6 +521,39 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
>>>> return len;
>>>> }
>>>>
>>>> +static ssize_t ina2xx_max_expected_current_show(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + char *buf)
>>>> +{
>>>> + struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
>>>> +
>>>> + return sprintf(buf, "%d\n", chip->max_expected_current);
>>>> +}
>>>> +
>>>> +static ssize_t ina2xx_max_expected_current_store(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf, size_t len)
>>>> +{
>>>> + struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
>>>> + unsigned long val;
>>>> + int ret;
>>>> +
>>>> + ret = kstrtoul((const char *) buf, 10, &val);
>>>
>>> Odd bit of casting given that's what it already is...
>>>
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ret = set_max_expected_current(chip, val);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + /* Update the Calibration register */
>>>> + ret = ina2xx_set_calibration(chip);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return len;
>>>> +}
>>>> +
>>>> #define INA219_CHAN(_type, _index, _address) { \
>>>> .type = (_type), \
>>>> .address = (_address), \
>>>> @@ -755,10 +816,15 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,
>>>> ina2xx_shunt_resistor_show,
>>>> ina2xx_shunt_resistor_store, 0);
>>>>
>>>> +static IIO_DEVICE_ATTR(in_max_expected_current, S_IRUGO | S_IWUSR,
>>>> + ina2xx_max_expected_current_show,
>>>> + ina2xx_max_expected_current_store, 0);
>>>> +
>>>> static struct attribute *ina219_attributes[] = {
>>>> &iio_dev_attr_in_allow_async_readout.dev_attr.attr,
>>>> &iio_const_attr_ina219_integration_time_available.dev_attr.attr,
>>>> &iio_dev_attr_in_shunt_resistor.dev_attr.attr,
>>>> + &iio_dev_attr_in_max_expected_current.dev_attr.attr,
>>>> NULL,
>>>> };
>>>>
>>>> @@ -766,6 +832,7 @@ static struct attribute *ina226_attributes[] = {
>>>> &iio_dev_attr_in_allow_async_readout.dev_attr.attr,
>>>> &iio_const_attr_ina226_integration_time_available.dev_attr.attr,
>>>> &iio_dev_attr_in_shunt_resistor.dev_attr.attr,
>>>> + &iio_dev_attr_in_max_expected_current.dev_attr.attr,
>>>> NULL,
>>>> };
>>>>
>>>> @@ -851,6 +918,21 @@ static int ina2xx_probe(struct i2c_client *client,
>>>> if (ret)
>>>> return ret;
>>>>
>>>> + if (of_property_read_u32(client->dev.of_node,
>>>> + "max-expected-current", &val) < 0) {
>>>> + struct ina2xx_platform_data *pdata =
>>>> + dev_get_platdata(&client->dev);
>>>> +
>>>> + if (pdata && pdata->max_mA != 0)
>>>> + val = pdata->max_mA;
>>>> + else
>>>> + val = INA2XX_MAX_EXPECTED_A_DEFAULT;
>>>> + }
>>>> +
>>>> + ret = set_max_expected_current(chip, val);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> /* Patch the current config register with default. */
>>>> val = chip->config->config_default;
>>>>
>>>> diff --git a/include/linux/platform_data/ina2xx.h b/include/linux/platform_data/ina2xx.h
>>>> index 9abc0ca..f02b1d8 100644
>>>> --- a/include/linux/platform_data/ina2xx.h
>>>> +++ b/include/linux/platform_data/ina2xx.h
>>>> @@ -13,7 +13,9 @@
>>>> /**
>>>> * struct ina2xx_platform_data - ina2xx info
>>>> * @shunt_uohms shunt resistance in microohms
>>>> + * @max_mA max expected current in mA
>>>> */
>>>> struct ina2xx_platform_data {
>>>> long shunt_uohms;
>>>> + int max_mA;
>>>> };
>>>
>>>
>>>
>>>
>
>
>
>
More information about the linux-arm-kernel
mailing list