[PATCH 2/2] iio: gts-helpers: fix integration time units

[PATCH 2/2] iio: gts-helpers: fix integration time units

[Matti Vaittinen]

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

The IIO ABI mandates expressing integration times in seconds. The GTS
helper errorneously uses micro seconds in integration_times_available.
Fix this by converting the lists to IIO_VAL_INT_PLUS_MICRO.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/iio/industrialio-gts-helper.c | 43 ++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
index 8bb68975b259..2ab8d2dce019 100644
--- a/drivers/iio/industrialio-gts-helper.c
+++ b/drivers/iio/industrialio-gts-helper.c
@@ -337,6 +337,17 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
 	return ret;
 }
 
+static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
+				    int num_times)
+{
+	int i;
+
+	for (i = 0; i < num_times; i++) {
+		int_micro_times[i * 2] = time_us[i] / 1000000;
+		int_micro_times[i * 2 + 1] = time_us[i] % 1000000;
+	}
+}
+
 /**
  * iio_gts_build_avail_time_table - build table of available integration times
  * @gts:	Gain time scale descriptor
@@ -351,7 +362,7 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
  */
 static int iio_gts_build_avail_time_table(struct iio_gts *gts)
 {
-	int *times, i, j, idx = 0;
+	int *times, i, j, idx = 0, *int_micro_times;
 
 	if (!gts->num_itime)
 		return 0;
@@ -360,6 +371,7 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts)
 	if (!times)
 		return -ENOMEM;
 
+
 	/* Sort times from all tables to one and remove duplicates */
 	for (i = gts->num_itime - 1; i >= 0; i--) {
 		int new = gts->itime_table[i].time_us;
@@ -378,13 +390,24 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts)
 			}
 		}
 	}
-	gts->avail_time_tables = times;
-	/*
-	 * This is just to survive a unlikely corner-case where times in the
-	 * given time table were not unique. Else we could just trust the
-	 * gts->num_itime.
-	 */
-	gts->num_avail_time_tables = idx;
+
+	/* create a list of times formatted as list of IIO_VAL_INT_PLUS_MICRO */
+	int_micro_times = kcalloc(idx, sizeof(int) * 2, GFP_KERNEL);
+	if (int_micro_times) {
+		/*
+		 * This is just to survive a unlikely corner-case where times in
+		 * the given time table were not unique. Else we could just
+		 * trust the gts->num_itime.
+		 */
+		gts->num_avail_time_tables = idx;
+		iio_gts_us_to_int_micro(times, int_micro_times, idx);
+	}
+
+	gts->avail_time_tables = int_micro_times;
+	kfree(times);
+
+	if (!int_micro_times)
+		return -ENOMEM;
 
 	return 0;
 }
@@ -683,8 +706,8 @@ int iio_gts_avail_times(struct iio_gts *gts,  const int **vals, int *type,
 		return -EINVAL;
 
 	*vals = gts->avail_time_tables;
-	*type = IIO_VAL_INT;
-	*length = gts->num_avail_time_tables;
+	*type = IIO_VAL_INT_PLUS_MICRO;
+	*length = gts->num_avail_time_tables * 2;
 
 	return IIO_AVAIL_LIST;
 }
-- 
2.39.2


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

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

Re: [PATCH 2/2] iio: gts-helpers: fix integration time units

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <eeacd192c259e885850b5a2dd8b776bccfc44fa8.1681722914.git.mazziesaccount@gmail.com>
References: <eeacd192c259e885850b5a2dd8b776bccfc44fa8.1681722914.git.mazziesaccount@gmail.com>
TO: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>, Matti Vaittinen <mazziesaccount@gmail.com>
TO: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>, Matti Vaittinen <mazziesaccount@gmail.com>
CC: Jonathan Cameron <jic23@kernel.org>
CC: "Lars-Peter Clausen" <lars@metafoo.de>
CC: linux-iio@vger.kernel.org
CC: linux-kernel@vger.kernel.org

Hi Matti,

kernel test robot noticed the following build warnings:

[auto build test WARNING on c86b0e73f0bebbb0245ef2bac4cf269d61ff828c]

url:    https://github.com/intel-lab-lkp/linux/commits/Matti-Vaittinen/iio-bu27034-Fix-integration-time/20230417-172240
base:   c86b0e73f0bebbb0245ef2bac4cf269d61ff828c
patch link:    https://lore.kernel.org/r/eeacd192c259e885850b5a2dd8b776bccfc44fa8.1681722914.git.mazziesaccount%40gmail.com
patch subject: [PATCH 2/2] iio: gts-helpers: fix integration time units
:::::: branch date: 6 days ago
:::::: commit date: 6 days ago
config: parisc-randconfig-m031-20230421 (https://download.01.org/0day-ci/archive/20230423/202304231128.2wtp08sj-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304231128.2wtp08sj-lkp@intel.com/

New smatch warnings:
drivers/iio/industrialio-gts-helper.c:395 iio_gts_build_avail_time_table() warn: double check that we're allocating correct size: 4 vs 8

Old smatch warnings:
drivers/iio/industrialio-gts-helper.c:246 gain_to_scaletables() warn: double check that we're allocating correct size: 4 vs 8
drivers/iio/industrialio-gts-helper.c:269 gain_to_scaletables() error: uninitialized symbol 'ret'.
drivers/iio/industrialio-gts-helper.c:302 iio_gts_build_avail_scale_table() warn: double check that we're allocating correct size: 4 vs 8

vim +395 drivers/iio/industrialio-gts-helper.c

0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  350  
38416c28e16890 Matti Vaittinen 2023-03-31  351  /**
38416c28e16890 Matti Vaittinen 2023-03-31  352   * iio_gts_build_avail_time_table - build table of available integration times
38416c28e16890 Matti Vaittinen 2023-03-31  353   * @gts:	Gain time scale descriptor
38416c28e16890 Matti Vaittinen 2023-03-31  354   *
38416c28e16890 Matti Vaittinen 2023-03-31  355   * Build the table which can represent the available times to be returned
38416c28e16890 Matti Vaittinen 2023-03-31  356   * to users using the read_avail-callback.
38416c28e16890 Matti Vaittinen 2023-03-31  357   *
38416c28e16890 Matti Vaittinen 2023-03-31  358   * NOTE: Space allocated for the tables must be freed using
38416c28e16890 Matti Vaittinen 2023-03-31  359   * iio_gts_purge_avail_time_table() when the tables are no longer needed.
38416c28e16890 Matti Vaittinen 2023-03-31  360   *
38416c28e16890 Matti Vaittinen 2023-03-31  361   * Return: 0 on success.
38416c28e16890 Matti Vaittinen 2023-03-31  362   */
38416c28e16890 Matti Vaittinen 2023-03-31  363  static int iio_gts_build_avail_time_table(struct iio_gts *gts)
38416c28e16890 Matti Vaittinen 2023-03-31  364  {
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  365  	int *times, i, j, idx = 0, *int_micro_times;
38416c28e16890 Matti Vaittinen 2023-03-31  366  
38416c28e16890 Matti Vaittinen 2023-03-31  367  	if (!gts->num_itime)
38416c28e16890 Matti Vaittinen 2023-03-31  368  		return 0;
38416c28e16890 Matti Vaittinen 2023-03-31  369  
38416c28e16890 Matti Vaittinen 2023-03-31  370  	times = kcalloc(gts->num_itime, sizeof(int), GFP_KERNEL);
38416c28e16890 Matti Vaittinen 2023-03-31  371  	if (!times)
38416c28e16890 Matti Vaittinen 2023-03-31  372  		return -ENOMEM;
38416c28e16890 Matti Vaittinen 2023-03-31  373  
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  374  
38416c28e16890 Matti Vaittinen 2023-03-31  375  	/* Sort times from all tables to one and remove duplicates */
38416c28e16890 Matti Vaittinen 2023-03-31  376  	for (i = gts->num_itime - 1; i >= 0; i--) {
38416c28e16890 Matti Vaittinen 2023-03-31  377  		int new = gts->itime_table[i].time_us;
38416c28e16890 Matti Vaittinen 2023-03-31  378  
38416c28e16890 Matti Vaittinen 2023-03-31  379  		if (times[idx] < new) {
38416c28e16890 Matti Vaittinen 2023-03-31  380  			times[idx++] = new;
38416c28e16890 Matti Vaittinen 2023-03-31  381  			continue;
38416c28e16890 Matti Vaittinen 2023-03-31  382  		}
38416c28e16890 Matti Vaittinen 2023-03-31  383  
38416c28e16890 Matti Vaittinen 2023-03-31  384  		for (j = 0; j <= idx; j++) {
38416c28e16890 Matti Vaittinen 2023-03-31  385  			if (times[j] > new) {
38416c28e16890 Matti Vaittinen 2023-03-31  386  				memmove(&times[j + 1], &times[j],
38416c28e16890 Matti Vaittinen 2023-03-31  387  					(idx - j) * sizeof(int));
38416c28e16890 Matti Vaittinen 2023-03-31  388  				times[j] = new;
38416c28e16890 Matti Vaittinen 2023-03-31  389  				idx++;
38416c28e16890 Matti Vaittinen 2023-03-31  390  			}
38416c28e16890 Matti Vaittinen 2023-03-31  391  		}
38416c28e16890 Matti Vaittinen 2023-03-31  392  	}
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  393  
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  394  	/* create a list of times formatted as list of IIO_VAL_INT_PLUS_MICRO */
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17 @395  	int_micro_times = kcalloc(idx, sizeof(int) * 2, GFP_KERNEL);
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  396  	if (int_micro_times) {
38416c28e16890 Matti Vaittinen 2023-03-31  397  		/*
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  398  		 * This is just to survive a unlikely corner-case where times in
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  399  		 * the given time table were not unique. Else we could just
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  400  		 * trust the gts->num_itime.
38416c28e16890 Matti Vaittinen 2023-03-31  401  		 */
38416c28e16890 Matti Vaittinen 2023-03-31  402  		gts->num_avail_time_tables = idx;
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  403  		iio_gts_us_to_int_micro(times, int_micro_times, idx);
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  404  	}
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  405  
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  406  	gts->avail_time_tables = int_micro_times;
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  407  	kfree(times);
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  408  
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  409  	if (!int_micro_times)
0cfd72b41d5fe0 Matti Vaittinen 2023-04-17  410  		return -ENOMEM;
38416c28e16890 Matti Vaittinen 2023-03-31  411  
38416c28e16890 Matti Vaittinen 2023-03-31  412  	return 0;
38416c28e16890 Matti Vaittinen 2023-03-31  413  }
38416c28e16890 Matti Vaittinen 2023-03-31  414  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 2/2] iio: gts-helpers: fix integration time units

[Jonathan Cameron]

On Mon, 17 Apr 2023 12:20:18 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> The IIO ABI mandates expressing integration times in seconds. The GTS
> helper errorneously uses micro seconds in integration_times_available.
> Fix this by converting the lists to IIO_VAL_INT_PLUS_MICRO.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
>  drivers/iio/industrialio-gts-helper.c | 43 ++++++++++++++++++++-------
>  1 file changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
> index 8bb68975b259..2ab8d2dce019 100644
> --- a/drivers/iio/industrialio-gts-helper.c
> +++ b/drivers/iio/industrialio-gts-helper.c
> @@ -337,6 +337,17 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
>  	return ret;
>  }
>  
> +static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
> +				    int num_times)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_times; i++) {
> +		int_micro_times[i * 2] = time_us[i] / 1000000;
> +		int_micro_times[i * 2 + 1] = time_us[i] % 1000000;
> +	}
> +}
> +
>  /**
>   * iio_gts_build_avail_time_table - build table of available integration times
>   * @gts:	Gain time scale descriptor
> @@ -351,7 +362,7 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
>   */
>  static int iio_gts_build_avail_time_table(struct iio_gts *gts)
>  {
> -	int *times, i, j, idx = 0;
> +	int *times, i, j, idx = 0, *int_micro_times;
>  
>  	if (!gts->num_itime)
>  		return 0;
> @@ -360,6 +371,7 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts)
>  	if (!times)
>  		return -ENOMEM;
>  
> +

Grumble.

If nothing else comes up I'll tidy that stray line up when applying.

Note that these will need to wait for after rc1 now so my fixes branch
has moved on to include the code being fixed.

>  	/* Sort times from all tables to one and remove duplicates */
>  	for (i = gts->num_itime - 1; i >= 0; i--) {
>  		int new = gts->itime_table[i].time_us;
> @@ -378,13 +390,24 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts)
>  			}
>  		}
>  	}
> -	gts->avail_time_tables = times;
> -	/*
> -	 * This is just to survive a unlikely corner-case where times in the
> -	 * given time table were not unique. Else we could just trust the
> -	 * gts->num_itime.
> -	 */
> -	gts->num_avail_time_tables = idx;
> +
> +	/* create a list of times formatted as list of IIO_VAL_INT_PLUS_MICRO */
> +	int_micro_times = kcalloc(idx, sizeof(int) * 2, GFP_KERNEL);
> +	if (int_micro_times) {
> +		/*
> +		 * This is just to survive a unlikely corner-case where times in
> +		 * the given time table were not unique. Else we could just
> +		 * trust the gts->num_itime.
> +		 */
> +		gts->num_avail_time_tables = idx;
> +		iio_gts_us_to_int_micro(times, int_micro_times, idx);
> +	}
> +
> +	gts->avail_time_tables = int_micro_times;
> +	kfree(times);
> +
> +	if (!int_micro_times)
> +		return -ENOMEM;
>  
>  	return 0;
>  }
> @@ -683,8 +706,8 @@ int iio_gts_avail_times(struct iio_gts *gts,  const int **vals, int *type,
>  		return -EINVAL;
>  
>  	*vals = gts->avail_time_tables;
> -	*type = IIO_VAL_INT;
> -	*length = gts->num_avail_time_tables;
> +	*type = IIO_VAL_INT_PLUS_MICRO;
> +	*length = gts->num_avail_time_tables * 2;
>  
>  	return IIO_AVAIL_LIST;
>  }

Re: [PATCH 2/2] iio: gts-helpers: fix integration time units

[Vaittinen, Matti]

On 4/23/23 14:18, Jonathan Cameron wrote:
> On Mon, 17 Apr 2023 12:20:18 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> The IIO ABI mandates expressing integration times in seconds. The GTS
>> helper errorneously uses micro seconds in integration_times_available.
>> Fix this by converting the lists to IIO_VAL_INT_PLUS_MICRO.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>>   drivers/iio/industrialio-gts-helper.c | 43 ++++++++++++++++++++-------
>>   1 file changed, 33 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
>> index 8bb68975b259..2ab8d2dce019 100644
>> --- a/drivers/iio/industrialio-gts-helper.c
>> +++ b/drivers/iio/industrialio-gts-helper.c
>> @@ -337,6 +337,17 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
>>   	return ret;
>>   }
>>   
>> +static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
>> +				    int num_times)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < num_times; i++) {
>> +		int_micro_times[i * 2] = time_us[i] / 1000000;
>> +		int_micro_times[i * 2 + 1] = time_us[i] % 1000000;
>> +	}
>> +}
>> +
>>   /**
>>    * iio_gts_build_avail_time_table - build table of available integration times
>>    * @gts:	Gain time scale descriptor
>> @@ -351,7 +362,7 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
>>    */
>>   static int iio_gts_build_avail_time_table(struct iio_gts *gts)
>>   {
>> -	int *times, i, j, idx = 0;
>> +	int *times, i, j, idx = 0, *int_micro_times;
>>   
>>   	if (!gts->num_itime)
>>   		return 0;
>> @@ -360,6 +371,7 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts)
>>   	if (!times)
>>   		return -ENOMEM;
>>   
>> +
> 
> Grumble.

Oh. I wonder how things like this tend to slip-in. Maybe I should change 
my password, it must be someone else has cracked my it and is typing 
these in at night while I am sleeping ^_^;

> If nothing else comes up I'll tidy that stray line up when applying.

Thanks!

> Note that these will need to wait for after rc1 now so my fixes branch
> has moved on to include the code being fixed.

Well, that's Ok. Please, let me know if you want me to rebase to rc1 and 
respin the series.

--Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH 2/2] iio: gts-helpers: fix integration time units

[Jonathan Cameron]

On Mon, 24 Apr 2023 05:09:36 +0000
"Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:

> On 4/23/23 14:18, Jonathan Cameron wrote:
> > On Mon, 17 Apr 2023 12:20:18 +0300
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> The IIO ABI mandates expressing integration times in seconds. The GTS
> >> helper errorneously uses micro seconds in integration_times_available.
> >> Fix this by converting the lists to IIO_VAL_INT_PLUS_MICRO.
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> >> ---
> >>   drivers/iio/industrialio-gts-helper.c | 43 ++++++++++++++++++++-------
> >>   1 file changed, 33 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
> >> index 8bb68975b259..2ab8d2dce019 100644
> >> --- a/drivers/iio/industrialio-gts-helper.c
> >> +++ b/drivers/iio/industrialio-gts-helper.c
> >> @@ -337,6 +337,17 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
> >>   	return ret;
> >>   }
> >>   
> >> +static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
> >> +				    int num_times)
> >> +{
> >> +	int i;
> >> +
> >> +	for (i = 0; i < num_times; i++) {
> >> +		int_micro_times[i * 2] = time_us[i] / 1000000;
> >> +		int_micro_times[i * 2 + 1] = time_us[i] % 1000000;
> >> +	}
> >> +}
> >> +
> >>   /**
> >>    * iio_gts_build_avail_time_table - build table of available integration times
> >>    * @gts:	Gain time scale descriptor
> >> @@ -351,7 +362,7 @@ static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
> >>    */
> >>   static int iio_gts_build_avail_time_table(struct iio_gts *gts)
> >>   {
> >> -	int *times, i, j, idx = 0;
> >> +	int *times, i, j, idx = 0, *int_micro_times;
> >>   
> >>   	if (!gts->num_itime)
> >>   		return 0;
> >> @@ -360,6 +371,7 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts)
> >>   	if (!times)
> >>   		return -ENOMEM;
> >>   
> >> +  
> > 
> > Grumble.  
> 
> Oh. I wonder how things like this tend to slip-in. Maybe I should change 
> my password, it must be someone else has cracked my it and is typing 
> these in at night while I am sleeping ^_^;
> 
> > If nothing else comes up I'll tidy that stray line up when applying.  
> 
> Thanks!
> 
> > Note that these will need to wait for after rc1 now so my fixes branch
> > has moved on to include the code being fixed.  
> 
> Well, that's Ok. Please, let me know if you want me to rebase to rc1 and 
> respin the series.
No need as not much changed around these (not I'm not at rc1 yet, but
char-misc-linus now includes these so I rebased on that)

Applied to the fixes-togreg branch of iio.git with fixes tags added now
the commits are upstream.

Thanks,

Jonathan

> 
> --Matti
>