Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1631179 > unrolled thread

Re: [PATCH 2/2] iio: adc: Allow setting Shunt Voltage PGA gain and Bus Voltage range

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] iio: adc: Allow setting Shunt Voltage PGA gain and  Bus Voltage range Jonathan Cameron <jic23@kernel.org> - 2017-04-26 08:30 +0200
    Re: [PATCH 2/2] iio: adc: Allow setting Shunt Voltage PGA gain and  Bus Voltage range Jonathan Cameron <jic23@kernel.org> - 2017-04-26 09:10 +0200
      Re: [PATCH 2/2] iio: adc: Allow setting Shunt Voltage PGA gain and Bus Voltage range Stefan Bruens <stefan.bruens@rwth-aachen.de> - 2017-04-29 22:40 +0200
        Re: [PATCH 2/2] iio: adc: Allow setting Shunt Voltage PGA gain and  Bus Voltage range Jonathan Cameron <jic23@kernel.org> - 2017-04-30 18:30 +0200

#1631179 — Re: [PATCH 2/2] iio: adc: Allow setting Shunt Voltage PGA gain and Bus Voltage range

On 17/04/17 23:08, Stefan Bruens wrote:
> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
>> On 12/04/17 04:01, Stefan Brüns wrote:
>>> Reducing shunt and bus voltage range improves the accuracy, so allow
>>> altering the default settings.
>>>
>>> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
>>
>> Hi Stefan,
>>
>> There is new userspace ABI in here, so starting point is to document that.
>>
>> That would allow the discussion of whether it is the right ABI to begin.
>>
>> In particular can one of these at least be rolled into the standard
>> scale attributes that are already supported by the driver?
>> It looks to me like they both probably can - perhaps in conjunction with
>> use of the _available callback to notify userspace the range available from
>> _raw thus allowing easy computation of the range you are providing.
>>
>> Keeping new ABI to a minimum makes life a lot easier for userspace tooling!
>>
>> I particularly love the way it's described in the datasheet as a gain
>> for the shunt voltage but a range for the bus voltage - despite being the
>> same PGA (at least in the symbolic representation).
> 
> Unfortunately, correct use of raw and scale is somewhat underdocumented. I 
> would expect the raw values to reflect the value read from the device, 
> unaltered.
The raw value will indeed give you that.  The _available callbacks provide
the range of a particular value.  They are rather undocumented though and
rather new which is indeed less than ideal.

Correct use of raw and scale themselves is pretty well covered by the ABI
docs.
Documentation/ABI/testing/sysfs-bus-iio*

> 
> For the INA226, all value registers are 16 bit, while for the INA219 the 
> voltage register is 13bit (msb aligned, lowest 3 bits from the register are 
> masked), the other 3 registers are 16 bit as well.
> 
> The INA219 incorporates the bus range and shunt voltage gain in the register 
> value, i.e. the shunt voltage value 0x0100 always corresponds to 256 * 10uV, 
> irrespective of the PGA setting.
Ah. I hadn't realised that.  In that case the standard approach for this
is to use calibscale which reflects changes that are within the hardware
but not in the resulting _raw values (i.e. devices that apply the scale
internally only)
> 
> I think its a bad idea to incorporate the gain settings into the scale 
> attribute:
> 1. Raw values would no longer be raw values
I think this one is me being unclear in my original response.
> 2. Scale for the INA219 would be settable, but readonly for the INA226
That's not really a problem...

> 3. If the device has a gain setting, it should be exposed as such, and names 
> should correspond to the datasheet
Not necessarily. The aim here is to produce a single unified (and normally
minimal) ABI for all devices.  If a particular datasheet takes one particular
naming they user should not be obliged to get out said datasheet to find
out what it means.  Some of the early parts we supported did everything in
terms of scaling in the datasheets. They got in first and so we are stuck
with that ABI if we can possibly use it.
> 4. Any user of the gain settings had to be made aware of the possibility to 
> change it, no matter how it is exposed. Making it part of the scale, and thus 
> changing the meaning of the raw values, would be breaking the existing ABI.
The raw values should indeed not change.  That was a missunderstanding on my
part.  Usually when a device has a PGA it is not compensated for in the
output. So normally it's up to the driver to 'apply' the effective gain to
the incoming reading.  When that isn't the case, it can be considered some
sort of internal trim - hence the use of calibscale for this case.

Sorry for the delay in replying, I'm travelling at the moment and reviewing
with jet lag isn't much fun!

Thanks,

Jonathan
> 
> Kind regards,
> 
> Stefan
> 

[toc] | [next] | [standalone]

#1631194

On 26/04/17 07:19, Jonathan Cameron wrote:
> On 17/04/17 23:08, Stefan Bruens wrote:
>> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
>>> On 12/04/17 04:01, Stefan Brüns wrote:
>>>> Reducing shunt and bus voltage range improves the accuracy, so allow
>>>> altering the default settings.
>>>>
>>>> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
>>>
>>> Hi Stefan,
>>>
>>> There is new userspace ABI in here, so starting point is to document that.
>>>
>>> That would allow the discussion of whether it is the right ABI to begin.
>>>
>>> In particular can one of these at least be rolled into the standard
>>> scale attributes that are already supported by the driver?
>>> It looks to me like they both probably can - perhaps in conjunction with
>>> use of the _available callback to notify userspace the range available from
>>> _raw thus allowing easy computation of the range you are providing.
>>>
>>> Keeping new ABI to a minimum makes life a lot easier for userspace tooling!
>>>
>>> I particularly love the way it's described in the datasheet as a gain
>>> for the shunt voltage but a range for the bus voltage - despite being the
>>> same PGA (at least in the symbolic representation).
>>
>> Unfortunately, correct use of raw and scale is somewhat underdocumented. I 
>> would expect the raw values to reflect the value read from the device, 
>> unaltered.
> The raw value will indeed give you that.  The _available callbacks provide
> the range of a particular value.  They are rather undocumented though and
> rather new which is indeed less than ideal.
> 
> Correct use of raw and scale themselves is pretty well covered by the ABI
> docs.
> Documentation/ABI/testing/sysfs-bus-iio*
> 
>>
>> For the INA226, all value registers are 16 bit, while for the INA219 the 
>> voltage register is 13bit (msb aligned, lowest 3 bits from the register are 
>> masked), the other 3 registers are 16 bit as well.
>>
>> The INA219 incorporates the bus range and shunt voltage gain in the register 
>> value, i.e. the shunt voltage value 0x0100 always corresponds to 256 * 10uV, 
>> irrespective of the PGA setting.
> Ah. I hadn't realised that.  In that case the standard approach for this
> is to use calibscale which reflects changes that are within the hardware
> but not in the resulting _raw values (i.e. devices that apply the scale
> internally only)
>>
>> I think its a bad idea to incorporate the gain settings into the scale 
>> attribute:
>> 1. Raw values would no longer be raw values
> I think this one is me being unclear in my original response.
>> 2. Scale for the INA219 would be settable, but readonly for the INA226
> That's not really a problem...
> 
>> 3. If the device has a gain setting, it should be exposed as such, and names 
>> should correspond to the datasheet
> Not necessarily. The aim here is to produce a single unified (and normally
> minimal) ABI for all devices.  If a particular datasheet takes one particular
> naming they user should not be obliged to get out said datasheet to find
> out what it means.  Some of the early parts we supported did everything in
> terms of scaling in the datasheets. They got in first and so we are stuck
> with that ABI if we can possibly use it.
>> 4. Any user of the gain settings had to be made aware of the possibility to 
>> change it, no matter how it is exposed. Making it part of the scale, and thus 
>> changing the meaning of the raw values, would be breaking the existing ABI.
> The raw values should indeed not change.  That was a missunderstanding on my
> part.  Usually when a device has a PGA it is not compensated for in the
> output. So normally it's up to the driver to 'apply' the effective gain to
> the incoming reading.  When that isn't the case, it can be considered some
> sort of internal trim - hence the use of calibscale for this case.
Mulling this over, calibscale might not work either in this case. The datasheet
helpfully sometimes uses ranges and sometimes uses scale factors.
There is also obviously the calibration register kicking around which would
also be handled with calibscale if exposed to userspace (currently it isn't)

I'm out of time tonight so will think it bit more about this and get back to you
in the next few days...

Jonathan
> 
> Sorry for the delay in replying, I'm travelling at the moment and reviewing
> with jet lag isn't much fun!
> 
> Thanks,
> 
> Jonathan
>>
>> Kind regards,
>>
>> Stefan
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [next] | [standalone]

#1633340 — Re: [PATCH 2/2] iio: adc: Allow setting Shunt Voltage PGA gain and Bus Voltage range

On Mittwoch, 26. April 2017 08:59:47 CEST Jonathan Cameron wrote:
> On 26/04/17 07:19, Jonathan Cameron wrote:
> > On 17/04/17 23:08, Stefan Bruens wrote:
> >> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
[...]
> > 
> >> 4. Any user of the gain settings had to be made aware of the possibility
> >> to
> >> change it, no matter how it is exposed. Making it part of the scale, and
> >> thus changing the meaning of the raw values, would be breaking the
> >> existing ABI.> 
> > The raw values should indeed not change.  That was a missunderstanding on
> > my part.  Usually when a device has a PGA it is not compensated for in
> > the output. So normally it's up to the driver to 'apply' the effective
> > gain to the incoming reading.  When that isn't the case, it can be
> > considered some sort of internal trim - hence the use of calibscale for
> > this case.
> Mulling this over, calibscale might not work either in this case. The
> datasheet helpfully sometimes uses ranges and sometimes uses scale factors.
> There is also obviously the calibration register kicking around which would
> also be handled with calibscale if exposed to userspace (currently it isn't)
> 
> I'm out of time tonight so will think it bit more about this and get back to
> you in the next few days...

hardwaregain may be a viable option. For the shunt voltage, available values 
would be [0.125, 0.25, 0.5, 1.0], for the bus range we would have either [0.5, 
1.0] or [1.0, 2.0] for bus ranges [32V, 16V].

Does hardwaregain have the right semantics for shunt voltage gain and/or bus 
range?

Kind regards,

Stefan

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

[toc] | [prev] | [next] | [standalone]

#1633456

On 29/04/17 21:37, Stefan Bruens wrote:
> On Mittwoch, 26. April 2017 08:59:47 CEST Jonathan Cameron wrote:
>> On 26/04/17 07:19, Jonathan Cameron wrote:
>>> On 17/04/17 23:08, Stefan Bruens wrote:
>>>> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
> [...]
>>>
>>>> 4. Any user of the gain settings had to be made aware of the possibility
>>>> to
>>>> change it, no matter how it is exposed. Making it part of the scale, and
>>>> thus changing the meaning of the raw values, would be breaking the
>>>> existing ABI.> 
>>> The raw values should indeed not change.  That was a missunderstanding on
>>> my part.  Usually when a device has a PGA it is not compensated for in
>>> the output. So normally it's up to the driver to 'apply' the effective
>>> gain to the incoming reading.  When that isn't the case, it can be
>>> considered some sort of internal trim - hence the use of calibscale for
>>> this case.
>> Mulling this over, calibscale might not work either in this case. The
>> datasheet helpfully sometimes uses ranges and sometimes uses scale factors.
>> There is also obviously the calibration register kicking around which would
>> also be handled with calibscale if exposed to userspace (currently it isn't)
>>
>> I'm out of time tonight so will think it bit more about this and get back to
>> you in the next few days...
> 
> hardwaregain may be a viable option. For the shunt voltage, available values 
> would be [0.125, 0.25, 0.5, 1.0], for the bus range we would have either [0.5, 
> 1.0] or [1.0, 2.0] for bus ranges [32V, 16V].
> 
> Does hardwaregain have the right semantics for shunt voltage gain and/or bus 
> range?
Description we currently have in 
Documentation/ABI/testing/sysfs-bus-iio is:
		Hardware applied gain factor. If shared across all channels,
		<type>_hardwaregain is used.

Just thinking about the use cases, it is mostly used for cases where the
gain is not of the measurement being acquired, but rather of something related
(like the gain on time of flight sensors or pulse counters).

It also gets used for output devices and amplifiers though so kind of similar
as in those cases we felt calibrationscale was a bit of a stretch!

So yes, I can see that working.  Whether it is a better choice than 
simply allowing the range attributes (documented for this narrow
case to say they should only be used when the range is independent of
the scale) is an open question.  Given we have always preferred scales
to ranges if you think you can make hardwaregain fit well then lets
go with that, perhaps updating the docs to make this usecase explicit.

Looking back at the original emails we were actually thinking of
transistioning calibscale to hardwaregain in general as it covered
describing both uses, but it never happened...

J


> 
> Kind regards,
> 
> Stefan
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web