[IIO] Proposal for sysfs attributes

[IIO] Proposal for sysfs attributes

[Manuel Stahl]

[-- Attachment #1.1: Type: text/plain, Size: 2288 bytes --]

Hi all,

there were some sysfs userspace questions on linux-iio where we would 
like to get some more comments on.

First thing, there are some attributes describing the layout of a ring 
buffer that can be read out via a char device. The buffer contains 
several entries with each containing so called scan elements. All 
entries have the same layout. It is described in a directory 
scan_elements (see below) that contains three attributes per scan element.

> OK, no packed buffers for now, but we should implement variable
 > sample sizes for standard types. Indeed we already have this for
 > the timestamp, which is always 64 bit.
>
> To be compatible with future extensions we could have:
>    |- /sys/bus/iio/device0/buffer0/scan_elements/
>       |- accel_x_en    (0 or 1)
>       |- accel_x_type  (i.e. s14/16, see *)
>       |- accel_x_index (position inside the buffer entry)
>
> * s14/16 means signed 14 bits, stored in 16 bits, right aligned.
 >   If it's left aligned we can just modify the scale attribute and
 >   give the 16 bit interpretation in <channel>_raw.

Is the 's14/16' self explaining or should we use some other format?
Is 'type' a good postfix for the attribute?


Next question: how strictly we want to resamble the hwmon ABI?

To sum up the discussion:
Hwmon has files with the postfix _input to read values scaled to
reasonable units for fixed point representation. So the units are
sometimes scaled down to 'millis' e.g. millivolt, millidegree Celsius.
We agreed that we want to keep this, whenever we use the _input postfix.
For IIO there is also the postfix triple _raw, _scale and _offset,
where the final value is calculated by (_raw + _offset) * _scale.
Floating point values are allowed for any of these files.

The question in place was:
Do we want to resamble the 'milli' units or should we stick to
standard SI units (radians, kelvin, etc.) as floating point math is
necessary anyway.

Regards,
-- 
Dipl.-Inf. Manuel Stahl
Fraunhofer-Institut für Integrierte Schaltungen IIS
- Leistungsoptimierte Systeme -
Nordostpark 93                Telefon  +49 (0)911/58061-6419
90411 Nürnberg                Fax      +49 (0)911/58061-6398
http://www.iis.fraunhofer.de  manuel.stahl@iis.fraunhofer.de

[-- Attachment #1.2: manuel_stahl.vcf --]
[-- Type: text/x-vcard, Size: 170 bytes --]

begin:vcard
fn:Manuel Stahl
n:Stahl;Manuel
email;internet:manuel.stahl@iis.fraunhofer.de
tel;work:+49 911 58061-6419
x-mozilla-html:FALSE
version:2.1
end:vcard


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 6148 bytes --]

Re: [IIO] Proposal for sysfs attributes

[Jonathan Cameron]

On 09/06/10 09:41, Manuel Stahl wrote:
> Hi all,
> 
> there were some sysfs userspace questions on linux-iio where we would like to get some more comments on.
> 
> First thing, there are some attributes describing the layout of a
> ring buffer that can be read out via a char device. The buffer
> contains several entries with each containing so called scan
> elements. All entries have the same layout. It is described in a
> directory scan_elements (see below) that contains three attributes
> per scan element.
> 
>> OK, no packed buffers for now, but we should implement variable
>> sample sizes for standard types. Indeed we already have this for
>> the timestamp, which is always 64 bit.
>>
>> To be compatible with future extensions we could have:
>>    |- /sys/bus/iio/device0/buffer0/scan_elements/
>>       |- accel_x_en    (0 or 1)
>>       |- accel_x_type  (i.e. s14/16, see *)
>>       |- accel_x_index (position inside the buffer entry)
>>
>> * s14/16 means signed 14 bits, stored in 16 bits, right aligned.
>>   If it's left aligned we can just modify the scale attribute and
>>   give the 16 bit interpretation in <channel>_raw.
> 
> Is the 's14/16' self explaining or should we use some other format?
> Is 'type' a good postfix for the attribute?

One additional point here.  What we currently have that the enable
parameters are currently [m]_accel_x_en etc with m being an indicator
of the position of a parameter within the buffer.  Note not all channels
are enabled at a time.  For all devices we have so far, the ordering
is fixed, so this naming is constant.  Basically we roll the accel_x_index
attribute into the naming of the enable attribute.  Manuel, could you summarize
what you have against that approach? (I'm not particularly tied to it, but
it is in place and it does work).
> 
> 
> Next question: how strictly we want to resamble the hwmon ABI?
> 
> To sum up the discussion:
> Hwmon has files with the postfix _input to read values scaled to
> reasonable units for fixed point representation. So the units are
> sometimes scaled down to 'millis' e.g. millivolt, millidegree Celsius.
> We agreed that we want to keep this, whenever we use the _input postfix.
> For IIO there is also the postfix triple _raw, _scale and _offset,
> where the final value is calculated by (_raw + _offset) * _scale.
> Floating point values are allowed for any of these files.
(other than _raw obviously!)
> 
> The question in place was:
> Do we want to resemble the 'milli' units or should we stick to
> standard SI units (radians, kelvin, etc.) as floating point math is
> necessary anyway.
I've cc'd a few people who have contributed to previous abi discussions
for IIO in ways that make me think they may have opinions on this.

Re: [IIO] Proposal for sysfs attributes

[Alan Cox]

> > Do we want to resemble the 'milli' units or should we stick to
> > standard SI units (radians, kelvin, etc.) as floating point math is
> > necessary anyway.
> I've cc'd a few people who have contributed to previous abi discussions
> for IIO in ways that make me think they may have opinions on this.

FP isn't needed you can let the drivers do fixed point quite happily -
the Intel compass driver intentionally does this.

Using milli is asking for problems. There are lots of units where "milli"
is actually quite big (millifarad, millihenry) and others where its
stupendously small (eV for example). It works for hwmon because hwmon
covers such a narrow range of devices and units.

So I'd favour FP because I think there are three choices

1.	Encode the value, define the scaling factor in the ABI (not
	future proof)

2.	Encode the value and scaling factor as two fields in the ABI
	(more future proof)

3.	As 2 but use the normal encoding for this which involves
	putting a "." in the right place and using one field (ie FP)

Drivers are simply going to end up doing this with sysfs values

	snprintf(buf, sizeof(buf), "%d.%02d", v/100, v%100);

or similar - or in many cases

	snprintf(buf, sizeof(buf), "%d0000000000", v);

neither of which is a big cost.

For a ring buffer you probably need to encode the scaling factor and
field width in some sort of interface description query and the values in
the bitstream.

Which raises an interesting question. The natural way to deal with sysfs
is decimal fixed point internally. If drivers are going to do both
sysfs and rings then really you want to be able to encode powers of 10
shifts not just bitshifts.

Alan

Re: [IIO] Proposal for sysfs attributes

[Jonathan Cameron]

Hi Alan,

>>> Do we want to resemble the 'milli' units or should we stick to
>>> standard SI units (radians, kelvin, etc.) as floating point math is
>>> necessary anyway.
>> I've cc'd a few people who have contributed to previous abi discussions
>> for IIO in ways that make me think they may have opinions on this.
> 
> FP isn't needed you can let the drivers do fixed point quite happily -
> the Intel compass driver intentionally does this.
True.
> 
> Using milli is asking for problems. There are lots of units where "milli"
> is actually quite big (millifarad, millihenry) and others where its
> stupendously small (eV for example). It works for hwmon because hwmon
> covers such a narrow range of devices and units.
Agreed, but as has been suggested before, we want to minimize the number
of interfaces in kernel.  I have no issue at all in using SI units for
things that haven't turned up before (and IIO does).  For the processed
inputs (like your compass driver or light sensors with nasty conversion
functions) we have _input parameters as per hwmon.  Given it was their
interface first we want to keep to their units where they apply.
The question Manuel was raising is do we extend this (for consistency)
to the _raw and _scale combination (as per your option 2).
> 
> So I'd favour FP because I think there are three choices
> 
> 1.	Encode the value, define the scaling factor in the ABI (not
> 	future proof)
> 
> 2.	Encode the value and scaling factor as two fields in the ABI
> 	(more future proof)
That's what we are currently doing.  Mainly for exactly the reason
you suggest below.  We want to use the same logic for the buffered
and direct (e.g. sysfs attribute) reads. Basically we need to export
the scale factor anyway so why not use it for both and keep the driver
as simple as possible.  Userspace can almost always just cache the scale
value anyway (it won't change in any current device unless userspace
tells it to do so).  I guess we might have an autoranging device to
deal with in the future.  Perhaps we just mandate that such a device
gives _input style outputs.  For buffer route on that it is going to
require an extension to the interface anyway.
> 
> 3.	As 2 but use the normal encoding for this which involves
> 	putting a "." in the right place and using one field (ie FP)
> 
> Drivers are simply going to end up doing this with sysfs values
> 
> 	snprintf(buf, sizeof(buf), "%d.%02d", v/100, v%100);
> 
> or similar - or in many cases
> 
> 	snprintf(buf, sizeof(buf), "%d0000000000", v);
> 
> neither of which is a big cost.
> 
> For a ring buffer you probably need to encode the scaling factor and
> field width in some sort of interface description query and the values in
> the bitstream.
Agreed.  Bitstream doesn't change format (without userspace telling it to
do so), hence sysfs attrs are perfect for doing this.
> 
> Which raises an interesting question. The natural way to deal with sysfs
> is decimal fixed point internally. If drivers are going to do both
> sysfs and rings then really you want to be able to encode powers of 10
> shifts not just bitshifts.
I'm afraid I have lost you here.  Why restrict to powers of 10?  The buffered
route is going to ship whatever the chip actually produces. We don't want to
touch them at all in kernel.  The scale factor could then be pretty much anything.
Typically for sysfs route we don't care how much we have to do to the data
so we could do your option 3, but given it is trivial for userspace to deal
with option 2 I'm more inclined to keep things consistent across the two interfaces
and go with that.

Thanks,

Jonathan

Re: [IIO] Proposal for sysfs attributes

[Alan Cox]

> I'm afraid I have lost you here.  Why restrict to powers of 10?  The buffered
> route is going to ship whatever the chip actually produces.

Then I don't see the point of IIO over a million random misc API's of the
week. Giving 50,000 interfaces the same name doesn't actually achieve
anything or solve any real problems.

The job of the OS is to provide an *abstraction*. If IIO won't do that I
don't see the point of IIO.

The current situation is

	Random gadget added with own API
	Qt sensors module is written for said gadget

IIO is offering no improvement if it won't abstract in a controlled
fashion.

Re: [IIO] Proposal for sysfs attributes

[Jonathan Cameron]

Hi Alan,
>> I'm afraid I have lost you here.  Why restrict to powers of 10?  The buffered
>> route is going to ship whatever the chip actually produces.
> 
> Then I don't see the point of IIO over a million random misc API's of the
> week. Giving 50,000 interfaces the same name doesn't actually achieve
> anything or solve any real problems.
> 
> The job of the OS is to provide an *abstraction*. If IIO won't do that I
> don't see the point of IIO.
Sorry, I think we are talking at cross purposes here. IIO does provide an abstraction.
Otherwise as you say it would be pointless.

The values in the buffer are raw data off the chip. It tends to be already in the most
compact form and we guarantee no loss of accuracy this way.  How accurate you make
further processing is in the hands of the userspace code.

The abstraction comes in combination with the sysfs attributes that describe
what that format raw format is and tell userspace how to convert it to a standard
format. The combination of the two (and a suitable userspace library, perhaps provided
by us, perhaps by someone else), give you the nice consistent interface you desire.
Manuel is working on such a library and hence has been finding all the inconsistencies
and missing information in the current abi.

The second question Manuel raised was about exactly how we describe this data stream.
The current abi has proven cumbersome and hasn't covered all cases, so he was seeking
to propose a slight adjustment to fix this.

For sysfs attributes, (accel_x_raw + accel_x_offset) * accel_x_scale will always give
you a value in m/s^2.  accel_x_input is also in m/s^2 if that is provided instead.
With accelerometers and many other sensor, accel_x_offset is often 0 and hence we
don't bother providing it.  The offset and scale are fixed values unless userspace
has intervened to change them (e.g. in multirange sensors).  Thus they need only be
read once at start up.  If it proves necessary we can use the sysfs select stuff
to allow userspace to be notified of a change caused by other userspace code.

With the buffer and Manuel's proposal:  The data corresponding to accel_x_raw is
extracted in binary form using the type attributes (of it and all previous elements
in the capture).  This can then be converted in exactly the same way as the above
sysfs approach.  Hence my favouring of what I understood by your option 2.
If what you are interested in doing is brute force logging, or an algorithm that
can be performed in the raw data space, then you don't want to do this conversion at all.
The cost of converting this data may seem minor to you, but take into account that
some of the devices within the scope of IIO produce a 'lot' of data.

Obviously we are currently limiting the _raw approach to devices with linear conversions
to SI units. What we do about high speed devices with more complex conversion functions
is still and open question.  Right now no one has suggested adding any.  We'll deal
with that when it becomes relevant.

> 
> The current situation is
> 
> 	Random gadget added with own API
> 	Qt sensors module is written for said gadget
> 
> IIO is offering no improvement if it won't abstract in a controlled
> fashion.
I agree that would be the case, but the whole point is to provide this
abstraction!

Jonathan

Re: [IIO] Proposal for sysfs attributes

[Jonathan Cameron]

Hi Manuel,

Unfortunately this thread wandered off in a different direction, so let us
draw it back to the actual questions it brings up.
>> there were some sysfs userspace questions on linux-iio where we would like to get some more comments on.
>>
>> First thing, there are some attributes describing the layout of a
>> ring buffer that can be read out via a char device. The buffer
>> contains several entries with each containing so called scan
>> elements. All entries have the same layout. It is described in a
>> directory scan_elements (see below) that contains three attributes
>> per scan element.
>>
>>> OK, no packed buffers for now, but we should implement variable
>>> sample sizes for standard types. Indeed we already have this for
>>> the timestamp, which is always 64 bit.
>>>
>>> To be compatible with future extensions we could have:
>>>    |- /sys/bus/iio/device0/buffer0/scan_elements/
>>>       |- accel_x_en    (0 or 1)
>>>       |- accel_x_type  (i.e. s14/16, see *)
>>>       |- accel_x_index (position inside the buffer entry)
>>>
>>> * s14/16 means signed 14 bits, stored in 16 bits, right aligned.
>>>   If it's left aligned we can just modify the scale attribute and
>>>   give the 16 bit interpretation in <channel>_raw.
>>
>> Is the 's14/16' self explaining or should we use some other format?
>> Is 'type' a good postfix for the attribute?
> 
> One additional point here.  What we currently have that the enable
> parameters are currently [m]_accel_x_en etc with m being an indicator
> of the position of a parameter within the buffer.  Note not all channels
> are enabled at a time.  For all devices we have so far, the ordering
> is fixed, so this naming is constant.  Basically we roll the accel_x_index
> attribute into the naming of the enable attribute.  Manuel, could you summarize
> what you have against that approach? (I'm not particularly tied to it, but
> it is in place and it does work).
Any response to this question Manuel?  Basically this question asks whether
we want to explicitly tell userspace what the ordering is, or instead provide
it with sufficient info to trivially figure it out itself.  I'm inclined to leave
this job to user space.

Anyway, I'd like to see the addition of you type field asap (as I need it for some
stuff I'm doing today ;)  So do you want to propose the relevant abi docs patch
or shall I?  I'll start adding the parameter to my test driver set asap.
>>
>>
>> Next question: how strictly we want to resamble the hwmon ABI?
>>
>> To sum up the discussion:
>> Hwmon has files with the postfix _input to read values scaled to
>> reasonable units for fixed point representation. So the units are
>> sometimes scaled down to 'millis' e.g. millivolt, millidegree Celsius.
>> We agreed that we want to keep this, whenever we use the _input postfix.
>> For IIO there is also the postfix triple _raw, _scale and _offset,
>> where the final value is calculated by (_raw + _offset) * _scale.
>> Floating point values are allowed for any of these files.
> (other than _raw obviously!)
>>
>> The question in place was:
>> Do we want to resemble the 'milli' units or should we stick to
>> standard SI units (radians, kelvin, etc.) as floating point math is
>> necessary anyway.
> I've cc'd a few people who have contributed to previous abi discussions
> for IIO in ways that make me think they may have opinions on this.
> 
To express my personal view, I think we have to keep to the units of hwmon.
If we do so for the 'input' attributes and not the combined raw + scale version
things will just be rather confusing.  Ultimately I don't really care, as long
as we pick one option or the other!

A nice as kelvin sounds I don't think that is worth doing.  Lets just stick to
the Celcius choice of hwmon.  If you really want kelvin in a given device, feel
free to use the _offset parameter to just say it is offset appropriately from
Celsius!

Jonathan