The GTS helpers do flooring of scale when calculating available scales. This results available-scales to be reported smaller than they should when the division in scale computation resulted remainder greater than half of the divider. (decimal part of result > 0.5) Furthermore, when gains are computed based on scale, the gain resulting from the scale computation is also floored. As a consequence the floored scales reported by available scales may not match the gains that can be set. The related discussion can be found from: https://lore.kernel.org/all/84d7c283-e8e5-4c98-835c-fe3f6ff94f4b@xxxxxxxxx/ Do rounding when computing scales and gains. Fixes: 38416c28e168 ("iio: light: Add gain-time-scale helpers") Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx> --- Subjahit, is there any chance you test this patch with your driver? Can you drop the: if (val2 % 10) val2 += 1; from scale setting and do you see written and read scales matching? I did run a few Kunit tests on this change - but I'm still a bit jumpy on it... Reviewing/testing is highly appreciated! Just in case someone is interested in seeing the Kunit tests, they're somewhat unpolished & crude and can emit noisy debug prints - but can anyways be found from: https://github.com/M-Vaittinen/linux/commits/iio-gts-helpers-test-v6.6 --- drivers/iio/industrialio-gts-helper.c | 58 +++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c index 7653261d2dc2..7dc144ac10c8 100644 --- a/drivers/iio/industrialio-gts-helper.c +++ b/drivers/iio/industrialio-gts-helper.c @@ -18,6 +18,32 @@ #include <linux/iio/iio-gts-helper.h> #include <linux/iio/types.h> +static int iio_gts_get_gain_32(u64 full, unsigned int scale) +{ + unsigned int full32 = (unsigned int) full; + unsigned int rem; + int result; + + if (full == (u64)full32) { + unsigned int rem; + + result = full32 / scale; + rem = full32 - scale * result; + if (rem >= scale / 2) + result++; + + return result; + } + + rem = do_div(full, scale); + if ((u64)rem >= scale / 2) + result = full + 1; + else + result = full; + + return result; +} + /** * iio_gts_get_gain - Convert scale to total gain * @@ -28,30 +54,42 @@ * scale is 64 100 000 000. * @scale: Linearized scale to compute the gain for. * - * Return: (floored) gain corresponding to the scale. -EINVAL if scale + * Return: (rounded) gain corresponding to the scale. -EINVAL if scale * is invalid. */ static int iio_gts_get_gain(const u64 max, const u64 scale) { - u64 full = max; + u64 full = max, half_div; + unsigned int scale32 = (unsigned int) scale; int tmp = 1; - if (scale > full || !scale) + if (scale / 2 > full || !scale) return -EINVAL; + /* + * The loop-based implementation below will potentially run _long_ + * if we have a small scale and large 'max' - which may be needed when + * GTS is used for channels returning specific units. Luckily we can + * avoid the loop when scale is small and fits in 32 bits. + */ + if ((u64)scale32 == scale) + return iio_gts_get_gain_32(full, scale32); + if (U64_MAX - full < scale) { /* Risk of overflow */ - if (full - scale < scale) + if (full - scale / 2 < scale) return 1; full -= scale; tmp++; } - while (full > scale * (u64)tmp) + half_div = scale >> 2; + + while (full + half_div >= scale * (u64)tmp) tmp++; - return tmp; + return tmp - 1; } /** @@ -133,6 +171,7 @@ static int iio_gts_linearize(int scale_whole, int scale_nano, * Convert the total gain value to scale. NOTE: This does not separate gain * generated by HW-gain or integration time. It is up to caller to decide what * part of the total gain is due to integration time and what due to HW-gain. + * Computed gain is rounded to nearest integer. * * Return: 0 on success. Negative errno on failure. */ @@ -140,10 +179,13 @@ int iio_gts_total_gain_to_scale(struct iio_gts *gts, int total_gain, int *scale_int, int *scale_nano) { u64 tmp; + int rem; tmp = gts->max_scale; - do_div(tmp, total_gain); + rem = do_div(tmp, total_gain); + if (total_gain > 1 && rem >= total_gain / 2) + tmp += 1ULL; return iio_gts_delinearize(tmp, NANO, scale_int, scale_nano); } @@ -192,7 +234,7 @@ static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales) sort(gains[i], gts->num_hwgain, sizeof(int), iio_gts_gain_cmp, NULL); - /* Convert gains to scales */ + /* Convert gains to scales. */ for (j = 0; j < gts->num_hwgain; j++) { ret = iio_gts_total_gain_to_scale(gts, gains[i][j], &scales[i][2 * j], base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa -- 2.41.0 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =]
Attachment:
signature.asc
Description: PGP signature
