On 18/2/24 02:57, Jonathan Cameron wrote:
On Sun, 18 Feb 2024 01:09:33 +1030
Subhajit Ghosh <subhajit.ghosh@xxxxxxxxxxxxxx> wrote:
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?
Both are useful - so thanks for this email.
Preference for a formal tag though as that goes in the git commit and we have
a convenient record that both says you tested it + that we should make sure
to cc you on related changes as you may well be in a position to test those
as well!
Thanks,
Jonathan
Regards,
Subhajit Ghosh
base-commit: 841c35169323cd833294798e58b9bf63fa4fa1de
Thank you Jonathan for explaining the above.
I forgot to mention that the above test is run in parallel with continuous raw reads
from another script and event monitoring.
As I understand that you have already applied this patch but still,
Tested-by: Subhajit Ghosh <subhajit.ghosh@xxxxxxxxxxxxxx>
Regards,
Subhajit Ghosh
