Messages in this thread |  | | Date | Fri, 19 Jan 2024 22:26:58 +1030 | | Subject | Re: [PATCH] iio: gts-helper: Fix division loop | | From | Subhajit Ghosh <> |
| |
On 8/1/24 02:52, Jonathan Cameron wrote: > On Thu, 4 Jan 2024 11:34:28 +0200 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> The loop based 64bit division may run for a long time when dividend is a >> lot bigger than the divider. Replace the division loop by the >> div64_u64() which implementation may be significantly faster. >> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> >> Fixes: 38416c28e168 ("iio: light: Add gain-time-scale helpers") > > Hmm. Fix or not perf improvement? I'm going to take the middle ground > and leave the fixes tag, but not rush this in. > > So applied to the togreg branch of iio.git and for now just pushed out > as testing for 0-day etc to take a look before I rebase that tree after > rc1. > > > >> --- >> >> I've implemented also a fixup series for supporting rounding of >> gains/scales: >> https://lore.kernel.org/lkml/37d3aa193e69577353d314e94463a08d488ddd8d.1701780964.git.mazziesaccount@gmail.com/ >> >> That series does also remove the offending loop. >> >> We don't currently have any in-tree users of GTS helpers which would >> need the rounding support so pushing the rounding is not urgent (and I >> haven't heard of Subjahit whose driver required the rounding). Hence, we >> may want to only take this loop fix in for now (?) and reconsider >> rounding when someone need that. >> >> Jonathan, what's your take on this? > Agreed - let us wait for the rounding to have a user, but makes sense > to tidy this corner up in the meantime. > > Thanks, > > Jonathan > >> >> drivers/iio/industrialio-gts-helper.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c >> index 7653261d2dc2..abcab2d38589 100644 >> --- a/drivers/iio/industrialio-gts-helper.c >> +++ b/drivers/iio/industrialio-gts-helper.c >> @@ -34,7 +34,7 @@ >> static int iio_gts_get_gain(const u64 max, const u64 scale) >> { >> u64 full = max; >> - int tmp = 1; >> + int tmp = 0; >> >> if (scale > full || !scale) >> return -EINVAL; >> @@ -48,8 +48,7 @@ static int iio_gts_get_gain(const u64 max, const u64 scale) >> tmp++; >> } >> >> - while (full > scale * (u64)tmp) >> - tmp++; >> + tmp += div64_u64(full, scale); >> >> return tmp; >> } >> >> base-commit: 2cc14f52aeb78ce3f29677c2de1f06c0e91471ab > > Hi Matti,
Your fix works beautifully with the latest version of apds9306 driver which I am working on. All available scale values can be set without any errors. Thank you.
Moving to a new city with a new full time job with the assumption of getting more time for my list of opensource projects and contributions proved to be utterly wrong!
Regards, Subhajit Ghosh
|  |