On 17/2/24 00:28, Jonathan Cameron wrote:
On Mon, 12 Feb 2024 13:20:09 +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")
---
This is a resend. Only change is the base which is now the v6.8-rc4 and
not the v6.8-rc1
Given I'm not rushing this in, it is going via my togreg tree, so the
rebase wasn't really helpful (thankfully didn't stop it applying).
Would have been fine to send a ping response to the first posting of it.
I was leaving some time for David or Subhajit to have time to take
another look, but guess they are either happy with this or busy.
Applied to the togreg branch of iio.git and pushed out as testing for
all the normal reasons.
Jonathan
This change was earlier applied and reverted as it confusingly lacked of
the removal of the overflow check (which is only needed when we do
looping "while (full > scale * (u64)tmp)". As this loop got removed, the
check got also obsolete and leaving it to the code caused some
confusion.
So, I marked this as a v2, where v1 is the reverted change discussed
here:
https://lore.kernel.org/linux-iio/ZZZ7pJBGkTdFFqiY@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Revision history:
v1 => v2:
- Drop the obsolete overflow check
- Rebased on top of the v6.8-rc4
iio: gts: loop fix fix
---
drivers/iio/industrialio-gts-helper.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
index 7653261d2dc2..b51eb6cb766f 100644
--- a/drivers/iio/industrialio-gts-helper.c
+++ b/drivers/iio/industrialio-gts-helper.c
@@ -34,24 +34,11 @@
static int iio_gts_get_gain(const u64 max, const u64 scale)
{
u64 full = max;
- int tmp = 1;
if (scale > full || !scale)
return -EINVAL;
- if (U64_MAX - full < scale) {
- /* Risk of overflow */
- if (full - scale < scale)
- return 1;
-
- full -= scale;
- tmp++;
- }
-
- while (full > scale * (u64)tmp)
- tmp++;
-
- return tmp;
+ return div64_u64(full, scale);
}
/**
Hi Matti and Jonathan,
I somehow missed testing this patch earlier. The above patch works fine with apds9306 v7 driver(which work in progress!).
There are no errors.
My test script is simple:
#!/bin/bash
D=0
S=`cat /sys/bus/iio/devices/iio:device${D}/in_illuminance_scale_available`
for s in $S; do
echo $s
echo $s > /sys/bus/iio/devices/iio:device${D}/in_illuminance_scale
sleep 5
done
One question - if I test a patch like this, do I put a "Tested-by" tag or just mention that I have tested it?
Regards,
Subhajit Ghosh
base-commit: 841c35169323cd833294798e58b9bf63fa4fa1de
