On 22/1/24 17:20, Matti Vaittinen wrote:
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@xxxxxxxxx> 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@xxxxxxxxx>
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@xxxxxxxxx/
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@xxxxxxxxx/
You are most welcome. I did not check the above rounding fix pointed out by the link. I will be happy to check it and let you know by the end of this month.
I checked this division fix.
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!
Thank you Matti.
Yours,
-- Matti
Regards,
Subhajit Ghosh
