Messages in this thread |  | | Date | Mon, 22 Jan 2024 08:50:44 +0200 | | Subject | Re: [PATCH] iio: gts-helper: Fix division loop | | From | Matti Vaittinen <> |
| |
On 1/19/24 13:56, Subhajit Ghosh wrote: > 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.
Thanks for testing Subhajit! Just to ensure we have no miscommunication - did you test just this division fix, or the rounding fix here: https://lore.kernel.org/lkml/37d3aa193e69577353d314e94463a08d488ddd8d.1701780964.git.mazziesaccount@gmail.com/
> 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!
Well, I can't blame you :) Being in a new work at new city sounds like you have a lot on your plate right now. Give it half a year and things will stabilize though :) Oh, and falsely assuming that "when XXX, I will have the time to do YYY" - been there done that :)
Good luck on the new work and city!
Yours, -- Matti
-- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
|  |