[PATCH] iio: kx022a: document new chip_info structure members

[PATCH] iio: kx022a: document new chip_info structure members

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

The kx022a driver supports a few different HW variants. A chip-info
structure is used to describe sensor specific details. Support for
sensors with different measurement g-ranges was added recently,
introducing sensor specific scale arrays.

The members of the chip-info structure have been documented using
kerneldoc. The newly added members omitted the documentation. It is nice
to have all the entries documented for the sake of the consistency.
Furthermore, the scale table format may not be self explatonary, nor how
the amount of scales is informed.

Add documentation to scale table entries to maintain consistency and to
make it more obvious how the scales should be represented.

Suggested-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
Wording is difficult. Especially when not working on ones native
language. So, I am glad is someone evaluates whether using the 'NANO'
to describe 0.000 000 001 is correct - or if term like 'ppb' would make
more sense...
---
 drivers/iio/accel/kionix-kx022a.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
index 142652ff4b22..82c4ced7426d 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -137,6 +137,11 @@ struct kx022a_data;
  *
  * @name:			name of the device
  * @regmap_config:		pointer to register map configuration
+ * scale_table:			Array of two integer tables containing
+ *				supported scales. Each scale is represented
+ *				a 2 value array. First value being full
+ *				integers, second being NANOs.
+ * scale_table_size:		Amount of values in tables.
  * @channels:			pointer to iio_chan_spec array
  * @num_channels:		number of iio_chan_spec channels
  * @fifo_length:		number of 16-bit samples in a full buffer
-- 
2.47.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] iio: kx022a: document new chip_info structure members

[Mehdi Djait]

Hi Matti,

Thank you for the patch!

On Mon, Dec 02, 2024 at 01:47:42PM +0200, Matti Vaittinen wrote:
> The kx022a driver supports a few different HW variants. A chip-info
> structure is used to describe sensor specific details. Support for
> sensors with different measurement g-ranges was added recently,
> introducing sensor specific scale arrays.
> 
> The members of the chip-info structure have been documented using
> kerneldoc. The newly added members omitted the documentation. It is nice
> to have all the entries documented for the sake of the consistency.
> Furthermore, the scale table format may not be self explatonary, nor how
> the amount of scales is informed.
> 
> Add documentation to scale table entries to maintain consistency and to
> make it more obvious how the scales should be represented.
> 
> Suggested-by: Mehdi Djait <mehdi.djait@linux.intel.com>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> Wording is difficult. Especially when not working on ones native
> language. So, I am glad is someone evaluates whether using the 'NANO'
> to describe 0.000 000 001 is correct - or if term like 'ppb' would make
> more sense...
> ---
>  drivers/iio/accel/kionix-kx022a.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
> index 142652ff4b22..82c4ced7426d 100644
> --- a/drivers/iio/accel/kionix-kx022a.h
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -137,6 +137,11 @@ struct kx022a_data;
>   *
>   * @name:			name of the device
>   * @regmap_config:		pointer to register map configuration
> + * scale_table:			Array of two integer tables containing
> + *				supported scales. Each scale is represented
> + *				a 2 value array. First value being full
> + *				integers, second being NANOs.

How about: 

Array of tables containing two scaling factors for the supported
acceleration measurement ranges. First value is the integer part and
second value is the fractional part in nano units.

> + * scale_table_size:		Amount of values in tables.
>   * @channels:			pointer to iio_chan_spec array
>   * @num_channels:		number of iio_chan_spec channels
>   * @fifo_length:		number of 16-bit samples in a full buffer
> -- 
> 2.47.0
> 

--
Kind Regards
Mehdi Djait

Re: [PATCH] iio: kx022a: document new chip_info structure members

[Matti Vaittinen]

On 02/12/2024 15:41, Mehdi Djait wrote:
> Hi Matti,

> On Mon, Dec 02, 2024 at 01:47:42PM +0200, Matti Vaittinen wrote:
>> The kx022a driver supports a few different HW variants. A chip-info
>> structure is used to describe sensor specific details. Support for
>> sensors with different measurement g-ranges was added recently,
>> introducing sensor specific scale arrays.
>>
>> The members of the chip-info structure have been documented using
>> kerneldoc. The newly added members omitted the documentation. It is nice
>> to have all the entries documented for the sake of the consistency.
>> Furthermore, the scale table format may not be self explatonary, nor how
>> the amount of scales is informed.
>>
>> Add documentation to scale table entries to maintain consistency and to
>> make it more obvious how the scales should be represented.
>>
>> Suggested-by: Mehdi Djait <mehdi.djait@linux.intel.com>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> Wording is difficult. Especially when not working on ones native
>> language. So, I am glad is someone evaluates whether using the 'NANO'
>> to describe 0.000 000 001 is correct - or if term like 'ppb' would make
>> more sense...
>> ---
>>   drivers/iio/accel/kionix-kx022a.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
>> index 142652ff4b22..82c4ced7426d 100644
>> --- a/drivers/iio/accel/kionix-kx022a.h
>> +++ b/drivers/iio/accel/kionix-kx022a.h
>> @@ -137,6 +137,11 @@ struct kx022a_data;
>>    *
>>    * @name:			name of the device
>>    * @regmap_config:		pointer to register map configuration
>> + * scale_table:			Array of two integer tables containing
>> + *				supported scales. Each scale is represented
>> + *				a 2 value array. First value being full
>> + *				integers, second being NANOs.
> 
> How about:
> 
> Array of tables containing two scaling factors for the supported
> acceleration measurement ranges. First value is the integer part and
> second value is the fractional part in nano units.
> 

Hi Mehdi. Thanks for the input. I definitely prefer your wording over 
what I wrote. Except maybe the note about each table containing two 
scaling factors. I think a table contains two integers, but only one 
scaling factor which is composed of those integers.

I am also still wondering if ppb (or even fully written "parts per 
billion") should be used instead of nano. In my ears the "nano" needs 
units, but I suppose the scale does not have any.

Yours,
	-- Matti

Re: [PATCH] iio: kx022a: document new chip_info structure members

[Mehdi Djait]

Hi Matti,

> > >   drivers/iio/accel/kionix-kx022a.h | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
> > > index 142652ff4b22..82c4ced7426d 100644
> > > --- a/drivers/iio/accel/kionix-kx022a.h
> > > +++ b/drivers/iio/accel/kionix-kx022a.h
> > > @@ -137,6 +137,11 @@ struct kx022a_data;
> > >    *
> > >    * @name:			name of the device
> > >    * @regmap_config:		pointer to register map configuration
> > > + * scale_table:			Array of two integer tables containing
> > > + *				supported scales. Each scale is represented
> > > + *				a 2 value array. First value being full
> > > + *				integers, second being NANOs.
> > 
> > How about:
> > 
> > Array of tables containing two scaling factors for the supported
> > acceleration measurement ranges. First value is the integer part and
> > second value is the fractional part in nano units.
> > 
> 
> Hi Mehdi. Thanks for the input. I definitely prefer your wording over what I
> wrote. Except maybe the note about each table containing two scaling
> factors. I think a table contains two integers, but only one scaling factor
> which is composed of those integers.
> 

that is true, it is just one scaling factor.

> I am also still wondering if ppb (or even fully written "parts per billion")
> should be used instead of nano. In my ears the "nano" needs units, but I
> suppose the scale does not have any.
> 

So how about:
Array of tables containing a scaling factor for the supported
acceleration measurement range. First value is the integer part and
second value is the nano fractional part.

or:
Array of tables containing a scaling factor for the supported
acceleration measurement range. First value is the integer part and
second value is the nano/ppb (parts per billion) fractional part.

> Yours,
> 	-- Matti

--
Kind Regards
Mehdi Djait