On Sat, 16 Jan 2021 23:15:56 -0800
Jyoti Bhayana [off-list ref] wrote:

Hi Jonathan,

Can you clarify one thing ? If we go with option 2 which is using
IIO_AVAIL_RANGE for min,step,high using IIO_VAL_INT then how will it
ensure the possible floating value for step as we are using int type?

As you've identified we can't go with IIO_VAL_INT if we are doing that
approach, would need to be IIO_VAL_INT_PLUS_MICRO (possibly NANO)

That is why it comes unstuck if we need to use both val and val2 to
do 64 bit integers...

For the  min and max we just set the val2 value to 0.

Jonathan

Thanks,
Jyoti

On Sat, Jan 16, 2021 at 11:33 AM Jonathan Cameron [off-list ref] wrote:

quoted

On Mon, 11 Jan 2021 13:17:51 -0800
Jyoti Bhayana [off-list ref] wrote:
 

quoted

Hi Jonathan,

I know it is a bit confusing. Let me try to explain it with some
examples to hopefully clarify some things here.
SCMI Platform talks to the native/actual sensor, gets the raw values
from the native sensor and applies the scale and then sends those
values to the SCMI agent and the SCMI IIO driver.
Since the sensor readings which SCMI IIO driver gets are integer, to
convert them to float , we need to apply scale to these sensor values
which is the unit_exponent(power-of-10 multiplier in two’s-complement
format) specified in the SCMI specification

Native Sensor -> SCMI platform->SCMI Agent->SCMI IIO Driver

So if Native Sensor gets the sensor value
32767 and the scale the SCMI Platform is using is 0.002392.
SCMI platform does the calculation of 32767 * 0.002392 = 78.378664
and send the sensor value as 78378664 and the scale as .000001 to the
SCMI agent and SCMI IIO driver

so for SCMI IIO driver the sensor reading = 78378664 and scale = .000001
and  the sensor value is sensor_reading * scale = 78378664 *  .000001
=  78.378664
and the resolution which the SCMI Platform sends to the SCMI agent is 0.002392.
In the SCMI IIO driver, scale which is .000001 is applied to the min
range/max range and the actual sensor values.
sensor resolution which is  0.002392 is just passed to the userspace
layer so that they know the Native sensor resolution/scale
being applied by the SCMI platform.  

That was pretty much where I'd gotten to.
Whilst the reasoning might be different it is equivalent to a sensor
providing info on expected noise by giving a 'valid resolution'.
In that case as well you have a sensor providing a number that looks to have
more precision than it actually does.

Anyhow, that similarity doesn't really matter here.

I'll also add that a design that applies scale in two places is inherently
less than ideal.   A cleaner design would have maintained the separation
between scale and raw value all the way up the stack.  That would result
in 0 loss of information and also be a cleaner interface.
Ah well, we live with what we have :)
 

quoted

Regarding your comments in the previous email, when you mentioned
"what we actually
need is non standard ABI for resolution"? Does that mean that it is ok
to have sensor resolution
as the IIO attribute shown below?

static IIO_DEVICE_ATTR(sensor_resolution, 0444, scmi_iio_get_sensor_resolution,
                     NULL, 0);  

We could do something new (see later for why I don't think we need to)
Would need to clearly reflect what it applies to and I'm not sure resolution
is even an unambiguous name given sensor resolution is often described as 8bit
10bit etc.  E.g. this selection table from Maxim for ADCs.
https://www.maximintegrated.com/en/products/parametric/search.html?fam=prec_adc&tree=master&metaTitle=Precision%20ADCs%20(%20%205Msps)&hide=270
Of course sometimes it's also used for what you want here.

Hohum.  So we might be still be able to do this with standard ABI but we
are going to need to do some maths in the driver.
So if we were to express it via

in_accel_raw_avail for example we could use the [low step high] form.

low and high are straight forward as those are expressed directly from
axis_min_range and axis_max_range which I think are in the same units
as the _raw channel itself.

For resolution, we have it expressed as [res] x 10^res_exponent
and if we just put that in as the 'step' above it would have the wrong
exponent (as we'd expect to still have to apply your 0.00001 from above
example).

Hence we express it as [res] x 10^(res_exponent - exponent)

I'm going to slightly modify your example above because the two exponents
are the same so it's hard to tell if I have them right way around.
Hence let res = 0.00293 = 293 x 10^(-5)  (I just dropped the trailing 2)

scale = 10^(-6) exponent = -6

So step = 2392 x 10^(-5 + 6) = 2390
giving us

[min 2390 max] for _raw_available

Hence when userspace comes along and wants this in relevant base units (here
m/sec^2) it applies the x10^(-6) mutliplier from _scale we get out expected value
of 0.00239 m/sec^2

That should work for any case we see but the maths done in the driver will have
to cope with potential negative exponents for step.

One catch will be the 64 bit potential values for min and max :(
 

quoted

static struct attribute *scmi_iio_attributes[] = {
       &iio_dev_attr_sensor_resolution.dev_attr.attr,
       NULL,
};

and for the min/max range, I can use the read_avail callback?  

I would have said yes normally but if we are going to cope with
a potential floating point value for step as touched on above we
may have to do it by hand in the driver.  Not ideal but may
be only option :(
 

quoted

Also, for the min/max range, there were two options discussed in the
email thread:
option 1)  Add new IIO val Type IIO_VAL_INT_H32_L32, and modify the
iio_format_value to format the 64 bit int properly for the userspace
option 2) Ignore the H32 bits and use the existing IIO_VAL_INT as just
L32 bits should be sufficient for current sensor values.  

Ignore is a strong way of putting it.  We would definitely want to
shout about it if we do get anything set in H32.

If we are fairly sure that we aren't going to anything greater than
32 bits than we are fine.

It should be possible to work through the worst cases given
limits of say +-20g for accelerometers for example and the relatively
limited exponents (5 bits).  + sensible resolution.

If it's fairly safe I'd like to go for option 2. as it would ensure we
can do floating point for the step (which is then used to compute the
resolution value for android)

Thanks

Jonathan
 

quoted

Let me know which option you prefer for min/max range. and also please
confirm if it is ok to have an IIO attribute for resolution like
mentioned above.


Thanks,
Jyoti

Thank you so much

Jyoti



On Mon, Jan 11, 2021 at 4:34 AM Jonathan Cameron
[off-list ref] wrote:  

quoted

On Sun, 10 Jan 2021 22:44:44 -0800
Jyoti Bhayana [off-list ref] wrote:
 

quoted

Hi Jonathan,

In section 4.7.2.5.1 of the specification, the following exponent is
the scale value

uint32 axis_attributes_high
Bits[15:11] Exponent: The power-of-10 multiplier in two’s-complement
format that is applied to the sensor unit
specified by the SensorType field.

and the resolution is

uint32 axis_resolution
Bits[31:27] Exponent: The power-of-10 multiplier in two’s-complement format
that is applied to the Res field. Bits[26:0] Res: The resolution of
the sensor axis.

From code in scmi_protocol.h
/**
 * scmi_sensor_axis_info  - describes one sensor axes
 * @id: The axes ID.
 * @type: Axes type. Chosen amongst one of @enum scmi_sensor_class.
 * @scale: Power-of-10 multiplier applied to the axis unit.
 * @name: NULL-terminated string representing axes name as advertised by
 *  SCMI platform.
 * @extended_attrs: Flag to indicate the presence of additional extended
 *    attributes for this axes.
 * @resolution: Extended attribute representing the resolution of the axes.
 * Set to 0 if not reported by this axes.
 * @exponent: Extended attribute representing the power-of-10 multiplier that
 *      is applied to the resolution field. Set to 0 if not reported by
 *      this axes.
 * @attrs: Extended attributes representing minimum and maximum values
 *   measurable by this axes. Set to 0 if not reported by this sensor.
 */

struct scmi_sensor_axis_info {
unsigned int id;
unsigned int type;
int scale; //This is the scale used for min/max range
char name[SCMI_MAX_STR_SIZE];
bool extended_attrs;
unsigned int resolution;
int exponent; // This is the scale used in resolution
struct scmi_range_attrs attrs;
};

The scale above  is the Power-of-10 multiplier which is applied to the min range
and the max range value
but the resolution is equal to resolution and multiplied by
Power-of-10 multiplier
of exponent in the above struct.
So as can be seen above the value of the power of 10 multiplier used
for min/max range
can be different than the value of the power of 10 multiplier used for
the resolution.
Hence, if I have to use IIO_AVAIL_RANGE to specify min/max range and as well
as resolution, then I have to use the float format with the scale applied.

If there is another way which you know of and prefer, please let me know.  

I'll confess I've gotten a bit lost here.

So I think where we are is how to describe the range of the sensor and why we can't
use in_accel_x_raw_available to provide the

Understood that the resolution can have different scaling.  That is presumably
to allow for the case where a device is reporting values at a finer scale than
it's real resolution.  Resolution might take into account expected noise for
example.  So it should be decoupled from the scaling of both the actual measurements
and the axis high / low limits.

However, I'd read that as saying the axis high / low limits and the actual sensor
readings should be scaled by the exponent in axis_attributes_high.
So I think we are fine for the range, but my earlier assumption that resolution
was equivalent to scale in IIO (real world value for 1LSB) may be completely wrong
as resolution may be unconnected to how you convert to a real world value?

If nothing else I'd like to suggest the spec needs to be tightened a bit here
to say exactly how we convert a value coming in to real world units (maybe
I'm just missing it).

Anyhow, I suspect we've been looking at this the wrong way and what we actually
need is non standard ABI for resolution.

Jonathan



 

quoted

Thanks,
Jyoti




Thanks,
Jyoti

On Sat, Jan 9, 2021 at 11:01 AM Jonathan Cameron [off-list ref] wrote:  

quoted

On Wed,  6 Jan 2021 21:23:53 +0000
Jyoti Bhayana [off-list ref] wrote:
 

quoted

Hi Jonathan,

Instead of adding IIO_VAL_INT_H32_L32, I am thinking of adding IIO_VAL_FRACTIONAL_LONG
or IIO_VAL_FRACTIONAL_64 as the scale/exponent used for min/max range can be different
than the one used in resolution according to specification.  

That's somewhat 'odd'.  Given min/max are inherently values the sensor is supposed to
be able to return why give them different resolutions?  Can you point me at a specific
section of the spec?  The axis_min_range_low etc fields don't seem to have units specified
but I assumed they were in sensor units and so same scale factors?
 

quoted

I am planning to use read_avail for IIO_CHAN_INFO_PROCESSED using IIO_AVAIL_RANGE
and this new IIO_VAL_FRACTIONAL_64 for min range,max range and resolution.
Instead of two values used in IIO_VAL_FRACTIONAL, IIO_VAL_FRACTIONAL_64 will use 4 values
val_high,val_low,and val2_high and val2_low.  

I'm not keen on the changing that internal kernel interface unless we absolutely
have to.  read_avail() is called from consumer drivers and they won't know anything
about this new variant.
 

quoted

Let me know if that is an acceptable solution.  

Hmm. It isn't a standard use of the ABI given the value in the buffer is (I assume)
raw (needs scale applied).  However, it isn't excluded by the ABI docs.  Whether
a standard userspace is going to expect it is not clear to me.

I don't want to end up in a position where we end up with available being generally
added for processed when what most people care about is what the value range they
might get from a polled read is (rather than via a buffer).

So I'm not that keen on this solution but if we can find a way to avoid it.

Jonathan

 

quoted

Thanks,
Jyoti