On 4/8/23 13:30, Jonathan Cameron wrote:
> On Sat, 25 Feb 2023 15:55:25 +0200
> Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
>
>> Values in the iio_chan_info_enum are crucial for understanding the
>> characteristics of an IIO channel and the data delivered via IIO channel.
>> Give a hand to developers who do their first set of IIO drivers.
>>
>> Add some documentation to these channel specifiers.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
>> ---
>> Please note that I did only add documentation for entries I am familiar
>> with. I did still add doc placeholders for all of the enum entries to
>> ease seeing which entries could still be documented. Hopefully this
>> encourages people to add missing pieces of documentation.
>
> Good to hear the optimism :)
>
> I'll add it to my activities for boring journeys (with good internet
> as probably need datasheets). Note I'm reviewing this on a train
> (having ignored it for a few weeks ;)
>
>> ---
>> include/linux/iio/types.h | 46 ++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> index 82faa98c719a..c8e3288ca24b 100644
>> --- a/include/linux/iio/types.h
>> +++ b/include/linux/iio/types.h
>> @@ -35,7 +35,51 @@ enum iio_available_type {
>> IIO_AVAIL_LIST,
>> IIO_AVAIL_RANGE,
>> };
>> -
>> +/**
>> + * enum iio_chan_info_enum - Information related to a IIO channel
>> + *
>> + * Many IIO channels have extra properties. Typically these properties can be
> "extra" glosses over the fact that some of these almost always exist.
> E.g. raw.
>
> IIO channels have a range of properties that may be read from userspace
> (via sysfs attributes) or from other drivers using the in kernel IIO consumer
> interfaces. These properties are read / written using the read_raw...
>
>
>> + * read / written by user using the read_raw or write_raw callbacks in the
>> + * struct iio_info.
>> + *
>> + * @IIO_CHAN_INFO_RAW: Raw channel data as provided by device. Scale
>> + * and offset are often required to convert these
>> + * values to meaningful units.
>
> to base units as defined in the IIO ABI (link)
This is just great. I like adding the link to ABI doc here! Thanks!
>> + * @IIO_CHAN_INFO_HARDWAREGAIN: Amplification applied by the hardware.
> Given how often this is done wrong I'd love to call out something like:
> "SCALE should be used for control if the HARDWAREGAIN directly affects the
> channel RAW measurement". Examples of HARDWAREGAIN include amplification of
> the light signal in a time of flight sensor."
So, let's try to tell people that HARDWAREGAIN is typically not the
thing they are interested in :) I think this is exactly the way these
docs should help both the driver authors as well as the poor sod
reviewing all the driver patches ;)
>> + * @IIO_CHAN_INFO_HYSTERESIS:
>> + * @IIO_CHAN_INFO_HYSTERESIS_RELATIVE:
>> + * @IIO_CHAN_INFO_INT_TIME: Integration time. Time during which the data is
>> + * accumulated by the device.
>
> Unit? (seconds I think).
Really...? Can you please double check this?
I believe the BU27034 uses micro seconds. I thought that was correct
approach - but if it is not then we can probably still change it w/o
breaking userland...
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
