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
> >
>
