Re: [PATCH] iio: gts-helper: Fix division loop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux