| Message ID | Z1_rRXqdhxhL6wBw@mva-rohm (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] iio: gts: Simplify available scale table build | expand |
On Mon, 16 Dec 2024 10:56:37 +0200 Matti Vaittinen <mazziesaccount@gmail.com> wrote: > Make available scale building more clear. This hurts the performance > quite a bit by looping throgh the scales many times instead of doing > everything in one loop. It however simplifies logic by: > - decoupling the gain and scale allocations & computations > - keeping the temporary 'per_time_gains' table inside the > per_time_scales computation function. > - separating building the 'all scales' table in own function and doing > it based on the already computed per-time scales. > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> Looks good to me, but I want to leave it on list a while before applying. Ideal if it gets some tested-by or other tags before I pick it up. As always, this is fiddly code, so the more eyes the better! Jonathan > --- > Revision history: > v2: > - Add a few comments > - Use more descriptive variable name > > This is still only tested using the kunit tests. All further testing is > still highly appreciated! > --- > drivers/iio/industrialio-gts-helper.c | 272 ++++++++++++++++---------- > 1 file changed, 174 insertions(+), 98 deletions(-) > > diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c > index 291c0fc332c9..65697be58a10 100644 > --- a/drivers/iio/industrialio-gts-helper.c > +++ b/drivers/iio/industrialio-gts-helper.c > @@ -160,16 +160,123 @@ static void iio_gts_purge_avail_scale_table(struct iio_gts *gts) > gts->num_avail_all_scales = 0; > } > > +static int scale_eq(int *sc1, int *sc2) > +{ > + return sc1[0] == sc2[0] && sc1[1] == sc2[1]; > +} > + > +static int scale_smaller(int *sc1, int *sc2) > +{ > + if (sc1[0] != sc2[0]) > + return sc1[0] < sc2[0]; > + > + /* If integer parts are equal, fixp parts */ > + return sc1[1] < sc2[1]; > +} > + > +/* > + * Do a single table listing all the unique scales that any combination of > + * supported gains and times can provide. > + */ > +static int do_combined_scaletable(struct iio_gts *gts, > + size_t all_scales_tbl_bytes) > +{ > + int t_idx, i, new_idx; > + int **scales = gts->per_time_avail_scale_tables; > + int *all_scales = kcalloc(gts->num_itime, all_scales_tbl_bytes, > + GFP_KERNEL); > + > + if (!all_scales) > + return -ENOMEM; > + /* > + * Create table containing all of the supported scales by looping > + * through all of the per-time scales and copying the unique scales > + * into one sorted table. > + * > + * We assume all the gains for same integration time were unique. > + * It is likely the first time table had greatest time multiplier as > + * the times are in the order of preference and greater times are > + * usually preferred. Hence we start from the last table which is likely > + * to have the smallest total gains. > + */ > + t_idx = gts->num_itime - 1; > + memcpy(all_scales, scales[t_idx], all_scales_tbl_bytes); > + new_idx = gts->num_hwgain * 2; > + > + while (t_idx-- > 0) { > + for (i = 0; i < gts->num_hwgain ; i++) { > + int *candidate = &scales[t_idx][i * 2]; > + int chk; > + > + if (scale_smaller(candidate, &all_scales[new_idx - 2])) { > + all_scales[new_idx] = candidate[0]; > + all_scales[new_idx + 1] = candidate[1]; > + new_idx += 2; > + > + continue; > + } > + for (chk = 0; chk < new_idx; chk += 2) > + if (!scale_smaller(candidate, &all_scales[chk])) > + break; > + > + if (scale_eq(candidate, &all_scales[chk])) > + continue; > + > + memmove(&all_scales[chk + 2], &all_scales[chk], > + (new_idx - chk) * sizeof(int)); > + all_scales[chk] = candidate[0]; > + all_scales[chk + 1] = candidate[1]; > + new_idx += 2; > + } > + } > + > + gts->num_avail_all_scales = new_idx / 2; > + gts->avail_all_scales_table = all_scales; > + > + return 0; > +} > + > +static void iio_gts_free_int_table_array(int **arr, int num_tables) > +{ > + int i; > + > + for (i = 0; i < num_tables; i++) > + kfree(arr[i]); > + > + kfree(arr); > +} > + > +static int iio_gts_alloc_int_table_array(int ***arr, int num_tables, int num_table_items) > +{ > + int i, **tmp; > + > + tmp = kcalloc(num_tables, sizeof(**arr), GFP_KERNEL); > + if (!tmp) > + return -ENOMEM; > + > + for (i = 0; i < num_tables; i++) { > + tmp[i] = kcalloc(num_table_items, sizeof(int), GFP_KERNEL); > + if (!tmp[i]) > + goto err_free; > + } > + > + *arr = tmp; > + > + return 0; > +err_free: > + iio_gts_free_int_table_array(tmp, i); > + > + return -ENOMEM; > +} > + > static int iio_gts_gain_cmp(const void *a, const void *b) > { > return *(int *)a - *(int *)b; > } > > -static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) > +static int fill_and_sort_scaletables(struct iio_gts *gts, int **gains, int **scales) > { > - int i, j, new_idx, time_idx, ret = 0; > - int *all_gains; > - size_t gain_bytes; > + int i, j, ret; > > for (i = 0; i < gts->num_itime; i++) { > /* > @@ -189,71 +296,69 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) > } > } > > - gain_bytes = array_size(gts->num_hwgain, sizeof(int)); > - all_gains = kcalloc(gts->num_itime, gain_bytes, GFP_KERNEL); > - if (!all_gains) > - return -ENOMEM; > + return 0; > +} > + > +static void compute_per_time_gains(struct iio_gts *gts, int **gains) > +{ > + int i, j; > + > + for (i = 0; i < gts->num_itime; i++) { > + for (j = 0; j < gts->num_hwgain; j++) > + gains[i][j] = gts->hwgain_table[j].gain * > + gts->itime_table[i].mul; > + } > +} > + > +static int compute_per_time_tables(struct iio_gts *gts, int **scales) > +{ > + int **per_time_gains; > + int ret; > > /* > - * We assume all the gains for same integration time were unique. > - * It is likely the first time table had greatest time multiplier as > - * the times are in the order of preference and greater times are > - * usually preferred. Hence we start from the last table which is likely > - * to have the smallest total gains. > + * Create a temporary array of the 'total gains' for each integration > + * time. > */ > - time_idx = gts->num_itime - 1; > - memcpy(all_gains, gains[time_idx], gain_bytes); > - new_idx = gts->num_hwgain; > + ret = iio_gts_alloc_int_table_array(&per_time_gains, gts->num_itime, > + gts->num_hwgain); > + if (ret) > + return ret; > > - while (time_idx-- > 0) { > - for (j = 0; j < gts->num_hwgain; j++) { > - int candidate = gains[time_idx][j]; > - int chk; > + compute_per_time_gains(gts, per_time_gains); > > - if (candidate > all_gains[new_idx - 1]) { > - all_gains[new_idx] = candidate; > - new_idx++; > + /* Convert the gains to scales and populate the scale tables */ > + ret = fill_and_sort_scaletables(gts, per_time_gains, scales); > > - continue; > - } > - for (chk = 0; chk < new_idx; chk++) > - if (candidate <= all_gains[chk]) > - break; > + iio_gts_free_int_table_array(per_time_gains, gts->num_itime); > > - if (candidate == all_gains[chk]) > - continue; > + return ret; > +} > > - memmove(&all_gains[chk + 1], &all_gains[chk], > - (new_idx - chk) * sizeof(int)); > - all_gains[chk] = candidate; > - new_idx++; > - } > - } > +/* > + * Create a table of supported scales for each supported integration time. > + * This can be used as available_scales by drivers which don't allow scale > + * setting to change the integration time to display correct set of scales > + * depending on the used integration time. > + */ > +static int **create_per_time_scales(struct iio_gts *gts) > +{ > + int **per_time_scales, ret; > > - gts->avail_all_scales_table = kcalloc(new_idx, 2 * sizeof(int), > - GFP_KERNEL); > - if (!gts->avail_all_scales_table) { > - ret = -ENOMEM; > - goto free_out; > - } > - gts->num_avail_all_scales = new_idx; > + ret = iio_gts_alloc_int_table_array(&per_time_scales, gts->num_itime, > + gts->num_hwgain * 2); > + if (ret) > + return ERR_PTR(ret); > > - for (i = 0; i < gts->num_avail_all_scales; i++) { > - ret = iio_gts_total_gain_to_scale(gts, all_gains[i], > - >s->avail_all_scales_table[i * 2], > - >s->avail_all_scales_table[i * 2 + 1]); > + ret = compute_per_time_tables(gts, per_time_scales); > + if (ret) > + goto err_out; > > - if (ret) { > - kfree(gts->avail_all_scales_table); > - gts->num_avail_all_scales = 0; > - goto free_out; > - } > - } > + return per_time_scales; > > -free_out: > - kfree(all_gains); > +err_out: > + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); > > - return ret; > + return ERR_PTR(ret); > } > > /** > @@ -275,55 +380,26 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) > */ > static int iio_gts_build_avail_scale_table(struct iio_gts *gts) > { > - int **per_time_gains, **per_time_scales, i, j, ret = -ENOMEM; > - > - per_time_gains = kcalloc(gts->num_itime, sizeof(*per_time_gains), GFP_KERNEL); > - if (!per_time_gains) > - return ret; > - > - per_time_scales = kcalloc(gts->num_itime, sizeof(*per_time_scales), GFP_KERNEL); > - if (!per_time_scales) > - goto free_gains; > + int ret, all_scales_tbl_bytes; > + int **per_time_scales; > > - for (i = 0; i < gts->num_itime; i++) { > - per_time_scales[i] = kcalloc(gts->num_hwgain, 2 * sizeof(int), > - GFP_KERNEL); > - if (!per_time_scales[i]) > - goto err_free_out; > - > - per_time_gains[i] = kcalloc(gts->num_hwgain, sizeof(int), > - GFP_KERNEL); > - if (!per_time_gains[i]) { > - kfree(per_time_scales[i]); > - goto err_free_out; > - } > - > - for (j = 0; j < gts->num_hwgain; j++) > - per_time_gains[i][j] = gts->hwgain_table[j].gain * > - gts->itime_table[i].mul; > - } > + if (unlikely(check_mul_overflow(gts->num_hwgain, 2 * sizeof(int), > + &all_scales_tbl_bytes))) > + return -EOVERFLOW; > > - ret = gain_to_scaletables(gts, per_time_gains, per_time_scales); > - if (ret) > - goto err_free_out; > + per_time_scales = create_per_time_scales(gts); > + if (IS_ERR(per_time_scales)) > + return PTR_ERR(per_time_scales); > > - for (i = 0; i < gts->num_itime; i++) > - kfree(per_time_gains[i]); > - kfree(per_time_gains); > gts->per_time_avail_scale_tables = per_time_scales; > > - return 0; > - > -err_free_out: > - for (i--; i >= 0; i--) { > - kfree(per_time_scales[i]); > - kfree(per_time_gains[i]); > + ret = do_combined_scaletable(gts, all_scales_tbl_bytes); > + if (ret) { > + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); > + return ret; > } > - kfree(per_time_scales); > -free_gains: > - kfree(per_time_gains); > > - return ret; > + return 0; > } > > static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
On 20/12/2024 21:21, Jonathan Cameron wrote: > On Mon, 16 Dec 2024 10:56:37 +0200 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> Make available scale building more clear. This hurts the performance >> quite a bit by looping throgh the scales many times instead of doing >> everything in one loop. It however simplifies logic by: >> - decoupling the gain and scale allocations & computations >> - keeping the temporary 'per_time_gains' table inside the >> per_time_scales computation function. >> - separating building the 'all scales' table in own function and doing >> it based on the already computed per-time scales. >> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > Looks good to me, but I want to leave it on list a while before applying. > Ideal if it gets some tested-by or other tags before I pick it up. > As always, this is fiddly code, so the more eyes the better! Please, let it wait until the Christmas has passed. I got information we might be getting some testing before the year changes :) Yours, -- Matti
On 22/12/2024 11:24, Matti Vaittinen wrote: > On 20/12/2024 21:21, Jonathan Cameron wrote: >> On Mon, 16 Dec 2024 10:56:37 +0200 >> Matti Vaittinen <mazziesaccount@gmail.com> wrote: >> >>> Make available scale building more clear. This hurts the performance >>> quite a bit by looping throgh the scales many times instead of doing >>> everything in one loop. It however simplifies logic by: >>> - decoupling the gain and scale allocations & computations >>> - keeping the temporary 'per_time_gains' table inside the >>> per_time_scales computation function. >>> - separating building the 'all scales' table in own function and doing >>> it based on the already computed per-time scales. >>> >>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> >> Looks good to me, but I want to leave it on list a while before applying. >> Ideal if it gets some tested-by or other tags before I pick it up. >> As always, this is fiddly code, so the more eyes the better! > > Please, let it wait until the Christmas has passed. I got information we > might be getting some testing before the year changes :) Well, the year changed and no tested-by tags emerged. I suppose my sources weren't right at this time. Yours, -- Matti
On 11/1/25 00:56, Matti Vaittinen wrote: > On 22/12/2024 11:24, Matti Vaittinen wrote: >> On 20/12/2024 21:21, Jonathan Cameron wrote: >>> On Mon, 16 Dec 2024 10:56:37 +0200 >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: >>> >>>> Make available scale building more clear. This hurts the performance >>>> quite a bit by looping throgh the scales many times instead of doing >>>> everything in one loop. It however simplifies logic by: >>>> - decoupling the gain and scale allocations & computations >>>> - keeping the temporary 'per_time_gains' table inside the >>>> per_time_scales computation function. >>>> - separating building the 'all scales' table in own function and doing >>>> it based on the already computed per-time scales. >>>> >>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> >>> Looks good to me, but I want to leave it on list a while before applying. >>> Ideal if it gets some tested-by or other tags before I pick it up. >>> As always, this is fiddly code, so the more eyes the better! >> >> Please, let it wait until the Christmas has passed. I got information we might be getting some testing before the year changes :) > > Well, the year changed and no tested-by tags emerged. I suppose my sources weren't right at this time. > > Yours, > -- Matti Hi Matti, Hope you had a good Christmas and new year. After my US trip, it took me some time to come to terms that I have to work for a living! The code works fine. I tested it with apds9306 driver with stm32mp157-dk2 board. Tested-by: subhajit.ghosh@tweaklogic.com Just want to report something else which may not be related to this. When I tried to cross-compile with linux-gnueabi-gcc version 12.2.0 with Linux kernel 6.1.28, I got the following errors: CC [M] /home/subhajit/opensource_contributions/apds9306/apds9306_backport/./drivers/iio/industrialio-gts-helper.o /tmp/ccn9UpwF.s: Assembler messages: /tmp/ccn9UpwF.s:22: Error: junk at end of line, first unrecognized character is `I' ... ... I had to remove the double quotes from the macros for all symbol exports: EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, "IIO_GTS_HELPER"); to EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, IIO_GTS_HELPER); However the x86_64 native build of the mainline kernel 6.13.0-rc6 on my laptop went just fine with this patch. Regards, Subhajit Ghosh
On Sat, 11 Jan 2025 14:47:28 +1030 Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote: > On 11/1/25 00:56, Matti Vaittinen wrote: > > On 22/12/2024 11:24, Matti Vaittinen wrote: > >> On 20/12/2024 21:21, Jonathan Cameron wrote: > >>> On Mon, 16 Dec 2024 10:56:37 +0200 > >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >>> > >>>> Make available scale building more clear. This hurts the performance > >>>> quite a bit by looping throgh the scales many times instead of doing > >>>> everything in one loop. It however simplifies logic by: > >>>> - decoupling the gain and scale allocations & computations > >>>> - keeping the temporary 'per_time_gains' table inside the > >>>> per_time_scales computation function. > >>>> - separating building the 'all scales' table in own function and doing > >>>> it based on the already computed per-time scales. > >>>> > >>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > >>> Looks good to me, but I want to leave it on list a while before applying. > >>> Ideal if it gets some tested-by or other tags before I pick it up. > >>> As always, this is fiddly code, so the more eyes the better! > >> > >> Please, let it wait until the Christmas has passed. I got information we might be getting some testing before the year changes :) > > > > Well, the year changed and no tested-by tags emerged. I suppose my sources weren't right at this time. > > > > Yours, > > -- Matti > Hi Matti, > > Hope you had a good Christmas and new year. After my US trip, it took me some time > to come to terms that I have to work for a living! > > The code works fine. I tested it with apds9306 driver with stm32mp157-dk2 board. > > Tested-by: subhajit.ghosh@tweaklogic.com > > Just want to report something else which may not be related to this. > When I tried to cross-compile with linux-gnueabi-gcc version 12.2.0 with Linux kernel 6.1.28, I got the following errors: > CC [M] /home/subhajit/opensource_contributions/apds9306/apds9306_backport/./drivers/iio/industrialio-gts-helper.o > /tmp/ccn9UpwF.s: Assembler messages: > /tmp/ccn9UpwF.s:22: Error: junk at end of line, first unrecognized character is `I' > ... > ... > > I had to remove the double quotes from the macros for all symbol exports: > EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, "IIO_GTS_HELPER"); > to > EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, IIO_GTS_HELPER); > > However the x86_64 native build of the mainline kernel 6.13.0-rc6 on my laptop went just fine with this patch. > > Regards, > Subhajit Ghosh Hi Subhajit, You've run into a tree wide change wrt to those quote that went in just after rc1 of this cycle. The error message is less than helpful and we've spent all cycle fixing these up :( Anyhow this is expected if backporting. Jonathan
On Sat, 11 Jan 2025 12:42:24 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Sat, 11 Jan 2025 14:47:28 +1030 > Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote: > > > On 11/1/25 00:56, Matti Vaittinen wrote: > > > On 22/12/2024 11:24, Matti Vaittinen wrote: > > >> On 20/12/2024 21:21, Jonathan Cameron wrote: > > >>> On Mon, 16 Dec 2024 10:56:37 +0200 > > >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > >>> > > >>>> Make available scale building more clear. This hurts the performance > > >>>> quite a bit by looping throgh the scales many times instead of doing > > >>>> everything in one loop. It however simplifies logic by: > > >>>> - decoupling the gain and scale allocations & computations > > >>>> - keeping the temporary 'per_time_gains' table inside the > > >>>> per_time_scales computation function. > > >>>> - separating building the 'all scales' table in own function and doing > > >>>> it based on the already computed per-time scales. > > >>>> > > >>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > >>> Looks good to me, but I want to leave it on list a while before applying. > > >>> Ideal if it gets some tested-by or other tags before I pick it up. > > >>> As always, this is fiddly code, so the more eyes the better! > > >> > > >> Please, let it wait until the Christmas has passed. I got information we might be getting some testing before the year changes :) > > > > > > Well, the year changed and no tested-by tags emerged. I suppose my sources weren't right at this time. > > > > > > Yours, > > > -- Matti > > Hi Matti, > > > > Hope you had a good Christmas and new year. After my US trip, it took me some time > > to come to terms that I have to work for a living! > > > > The code works fine. I tested it with apds9306 driver with stm32mp157-dk2 board. > > > > Tested-by: subhajit.ghosh@tweaklogic.com > > > > Just want to report something else which may not be related to this. > > When I tried to cross-compile with linux-gnueabi-gcc version 12.2.0 with Linux kernel 6.1.28, I got the following errors: > > CC [M] /home/subhajit/opensource_contributions/apds9306/apds9306_backport/./drivers/iio/industrialio-gts-helper.o > > /tmp/ccn9UpwF.s: Assembler messages: > > /tmp/ccn9UpwF.s:22: Error: junk at end of line, first unrecognized character is `I' > > ... > > ... > > > > I had to remove the double quotes from the macros for all symbol exports: > > EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, "IIO_GTS_HELPER"); > > to > > EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, IIO_GTS_HELPER); > > > > However the x86_64 native build of the mainline kernel 6.13.0-rc6 on my laptop went just fine with this patch. > > > > Regards, > > Subhajit Ghosh > > Hi Subhajit, > > You've run into a tree wide change wrt to those quote that went in just after rc1 of this cycle. > The error message is less than helpful and we've spent all cycle fixing these up :( > > Anyhow this is expected if backporting. > > Jonathan > Applied to the testing branch of iio.git. I'll be rebasing on rc1 once it is available and then push this out as normal togreg branch. Thanks Jonathan > >
diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c index 291c0fc332c9..65697be58a10 100644 --- a/drivers/iio/industrialio-gts-helper.c +++ b/drivers/iio/industrialio-gts-helper.c @@ -160,16 +160,123 @@ static void iio_gts_purge_avail_scale_table(struct iio_gts *gts) gts->num_avail_all_scales = 0; } +static int scale_eq(int *sc1, int *sc2) +{ + return sc1[0] == sc2[0] && sc1[1] == sc2[1]; +} + +static int scale_smaller(int *sc1, int *sc2) +{ + if (sc1[0] != sc2[0]) + return sc1[0] < sc2[0]; + + /* If integer parts are equal, fixp parts */ + return sc1[1] < sc2[1]; +} + +/* + * Do a single table listing all the unique scales that any combination of + * supported gains and times can provide. + */ +static int do_combined_scaletable(struct iio_gts *gts, + size_t all_scales_tbl_bytes) +{ + int t_idx, i, new_idx; + int **scales = gts->per_time_avail_scale_tables; + int *all_scales = kcalloc(gts->num_itime, all_scales_tbl_bytes, + GFP_KERNEL); + + if (!all_scales) + return -ENOMEM; + /* + * Create table containing all of the supported scales by looping + * through all of the per-time scales and copying the unique scales + * into one sorted table. + * + * We assume all the gains for same integration time were unique. + * It is likely the first time table had greatest time multiplier as + * the times are in the order of preference and greater times are + * usually preferred. Hence we start from the last table which is likely + * to have the smallest total gains. + */ + t_idx = gts->num_itime - 1; + memcpy(all_scales, scales[t_idx], all_scales_tbl_bytes); + new_idx = gts->num_hwgain * 2; + + while (t_idx-- > 0) { + for (i = 0; i < gts->num_hwgain ; i++) { + int *candidate = &scales[t_idx][i * 2]; + int chk; + + if (scale_smaller(candidate, &all_scales[new_idx - 2])) { + all_scales[new_idx] = candidate[0]; + all_scales[new_idx + 1] = candidate[1]; + new_idx += 2; + + continue; + } + for (chk = 0; chk < new_idx; chk += 2) + if (!scale_smaller(candidate, &all_scales[chk])) + break; + + if (scale_eq(candidate, &all_scales[chk])) + continue; + + memmove(&all_scales[chk + 2], &all_scales[chk], + (new_idx - chk) * sizeof(int)); + all_scales[chk] = candidate[0]; + all_scales[chk + 1] = candidate[1]; + new_idx += 2; + } + } + + gts->num_avail_all_scales = new_idx / 2; + gts->avail_all_scales_table = all_scales; + + return 0; +} + +static void iio_gts_free_int_table_array(int **arr, int num_tables) +{ + int i; + + for (i = 0; i < num_tables; i++) + kfree(arr[i]); + + kfree(arr); +} + +static int iio_gts_alloc_int_table_array(int ***arr, int num_tables, int num_table_items) +{ + int i, **tmp; + + tmp = kcalloc(num_tables, sizeof(**arr), GFP_KERNEL); + if (!tmp) + return -ENOMEM; + + for (i = 0; i < num_tables; i++) { + tmp[i] = kcalloc(num_table_items, sizeof(int), GFP_KERNEL); + if (!tmp[i]) + goto err_free; + } + + *arr = tmp; + + return 0; +err_free: + iio_gts_free_int_table_array(tmp, i); + + return -ENOMEM; +} + static int iio_gts_gain_cmp(const void *a, const void *b) { return *(int *)a - *(int *)b; } -static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) +static int fill_and_sort_scaletables(struct iio_gts *gts, int **gains, int **scales) { - int i, j, new_idx, time_idx, ret = 0; - int *all_gains; - size_t gain_bytes; + int i, j, ret; for (i = 0; i < gts->num_itime; i++) { /* @@ -189,71 +296,69 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) } } - gain_bytes = array_size(gts->num_hwgain, sizeof(int)); - all_gains = kcalloc(gts->num_itime, gain_bytes, GFP_KERNEL); - if (!all_gains) - return -ENOMEM; + return 0; +} + +static void compute_per_time_gains(struct iio_gts *gts, int **gains) +{ + int i, j; + + for (i = 0; i < gts->num_itime; i++) { + for (j = 0; j < gts->num_hwgain; j++) + gains[i][j] = gts->hwgain_table[j].gain * + gts->itime_table[i].mul; + } +} + +static int compute_per_time_tables(struct iio_gts *gts, int **scales) +{ + int **per_time_gains; + int ret; /* - * We assume all the gains for same integration time were unique. - * It is likely the first time table had greatest time multiplier as - * the times are in the order of preference and greater times are - * usually preferred. Hence we start from the last table which is likely - * to have the smallest total gains. + * Create a temporary array of the 'total gains' for each integration + * time. */ - time_idx = gts->num_itime - 1; - memcpy(all_gains, gains[time_idx], gain_bytes); - new_idx = gts->num_hwgain; + ret = iio_gts_alloc_int_table_array(&per_time_gains, gts->num_itime, + gts->num_hwgain); + if (ret) + return ret; - while (time_idx-- > 0) { - for (j = 0; j < gts->num_hwgain; j++) { - int candidate = gains[time_idx][j]; - int chk; + compute_per_time_gains(gts, per_time_gains); - if (candidate > all_gains[new_idx - 1]) { - all_gains[new_idx] = candidate; - new_idx++; + /* Convert the gains to scales and populate the scale tables */ + ret = fill_and_sort_scaletables(gts, per_time_gains, scales); - continue; - } - for (chk = 0; chk < new_idx; chk++) - if (candidate <= all_gains[chk]) - break; + iio_gts_free_int_table_array(per_time_gains, gts->num_itime); - if (candidate == all_gains[chk]) - continue; + return ret; +} - memmove(&all_gains[chk + 1], &all_gains[chk], - (new_idx - chk) * sizeof(int)); - all_gains[chk] = candidate; - new_idx++; - } - } +/* + * Create a table of supported scales for each supported integration time. + * This can be used as available_scales by drivers which don't allow scale + * setting to change the integration time to display correct set of scales + * depending on the used integration time. + */ +static int **create_per_time_scales(struct iio_gts *gts) +{ + int **per_time_scales, ret; - gts->avail_all_scales_table = kcalloc(new_idx, 2 * sizeof(int), - GFP_KERNEL); - if (!gts->avail_all_scales_table) { - ret = -ENOMEM; - goto free_out; - } - gts->num_avail_all_scales = new_idx; + ret = iio_gts_alloc_int_table_array(&per_time_scales, gts->num_itime, + gts->num_hwgain * 2); + if (ret) + return ERR_PTR(ret); - for (i = 0; i < gts->num_avail_all_scales; i++) { - ret = iio_gts_total_gain_to_scale(gts, all_gains[i], - >s->avail_all_scales_table[i * 2], - >s->avail_all_scales_table[i * 2 + 1]); + ret = compute_per_time_tables(gts, per_time_scales); + if (ret) + goto err_out; - if (ret) { - kfree(gts->avail_all_scales_table); - gts->num_avail_all_scales = 0; - goto free_out; - } - } + return per_time_scales; -free_out: - kfree(all_gains); +err_out: + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); - return ret; + return ERR_PTR(ret); } /** @@ -275,55 +380,26 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) */ static int iio_gts_build_avail_scale_table(struct iio_gts *gts) { - int **per_time_gains, **per_time_scales, i, j, ret = -ENOMEM; - - per_time_gains = kcalloc(gts->num_itime, sizeof(*per_time_gains), GFP_KERNEL); - if (!per_time_gains) - return ret; - - per_time_scales = kcalloc(gts->num_itime, sizeof(*per_time_scales), GFP_KERNEL); - if (!per_time_scales) - goto free_gains; + int ret, all_scales_tbl_bytes; + int **per_time_scales; - for (i = 0; i < gts->num_itime; i++) { - per_time_scales[i] = kcalloc(gts->num_hwgain, 2 * sizeof(int), - GFP_KERNEL); - if (!per_time_scales[i]) - goto err_free_out; - - per_time_gains[i] = kcalloc(gts->num_hwgain, sizeof(int), - GFP_KERNEL); - if (!per_time_gains[i]) { - kfree(per_time_scales[i]); - goto err_free_out; - } - - for (j = 0; j < gts->num_hwgain; j++) - per_time_gains[i][j] = gts->hwgain_table[j].gain * - gts->itime_table[i].mul; - } + if (unlikely(check_mul_overflow(gts->num_hwgain, 2 * sizeof(int), + &all_scales_tbl_bytes))) + return -EOVERFLOW; - ret = gain_to_scaletables(gts, per_time_gains, per_time_scales); - if (ret) - goto err_free_out; + per_time_scales = create_per_time_scales(gts); + if (IS_ERR(per_time_scales)) + return PTR_ERR(per_time_scales); - for (i = 0; i < gts->num_itime; i++) - kfree(per_time_gains[i]); - kfree(per_time_gains); gts->per_time_avail_scale_tables = per_time_scales; - return 0; - -err_free_out: - for (i--; i >= 0; i--) { - kfree(per_time_scales[i]); - kfree(per_time_gains[i]); + ret = do_combined_scaletable(gts, all_scales_tbl_bytes); + if (ret) { + iio_gts_free_int_table_array(per_time_scales, gts->num_itime); + return ret; } - kfree(per_time_scales); -free_gains: - kfree(per_time_gains); - return ret; + return 0; } static void iio_gts_us_to_int_micro(int *time_us, int *int_micro_times,
Make available scale building more clear. This hurts the performance quite a bit by looping throgh the scales many times instead of doing everything in one loop. It however simplifies logic by: - decoupling the gain and scale allocations & computations - keeping the temporary 'per_time_gains' table inside the per_time_scales computation function. - separating building the 'all scales' table in own function and doing it based on the already computed per-time scales. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- Revision history: v2: - Add a few comments - Use more descriptive variable name This is still only tested using the kunit tests. All further testing is still highly appreciated! --- drivers/iio/industrialio-gts-helper.c | 272 ++++++++++++++++---------- 1 file changed, 174 insertions(+), 98 deletions(-)