linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: nau8824: use 64-bit arithmetic instead of 32-bit
@ 2018-07-05 13:06 Gustavo A. R. Silva
  2018-07-06  2:09 ` [alsa-devel] " Trent Piepho
  0 siblings, 1 reply; 2+ messages in thread
From: Gustavo A. R. Silva @ 2018-07-05 13:06 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, Gustavo A. R. Silva

Add suffix ULL to constant 256 in order to give the compiler complete
information about the proper arithmetic to use.

Notice that such constant is used in a context that expects an
expression of type u64 (64 bits, unsigned) and the following
expression is currently being evaluated using 32-bit arithmetic:

256 * fs * 2 * mclk_src_scaling[i].param

Addresses-Coverity-ID: 1432039 ("Unintentional integer overflow")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 sound/soc/codecs/nau8824.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c
index 6bd1445..468d514 100644
--- a/sound/soc/codecs/nau8824.c
+++ b/sound/soc/codecs/nau8824.c
@@ -1274,7 +1274,7 @@ static int nau8824_calc_fll_param(unsigned int fll_in,
 	fvco_max = 0;
 	fvco_sel = ARRAY_SIZE(mclk_src_scaling);
 	for (i = 0; i < ARRAY_SIZE(mclk_src_scaling); i++) {
-		fvco = 256 * fs * 2 * mclk_src_scaling[i].param;
+		fvco = 256ULL * fs * 2 * mclk_src_scaling[i].param;
 		if (fvco > NAU_FVCO_MIN && fvco < NAU_FVCO_MAX &&
 			fvco_max < fvco) {
 			fvco_max = fvco;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [alsa-devel] [PATCH] ASoC: nau8824: use 64-bit arithmetic instead of 32-bit
  2018-07-05 13:06 [PATCH] ASoC: nau8824: use 64-bit arithmetic instead of 32-bit Gustavo A. R. Silva
@ 2018-07-06  2:09 ` Trent Piepho
  0 siblings, 0 replies; 2+ messages in thread
From: Trent Piepho @ 2018-07-06  2:09 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	alsa-devel, linux-kernel

On Thu, Jul 5, 2018 at 6:06 AM, Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> Add suffix ULL to constant 256 in order to give the compiler complete
> information about the proper arithmetic to use.
>
> Notice that such constant is used in a context that expects an
> expression of type u64 (64 bits, unsigned) and the following
> expression is currently being evaluated using 32-bit arithmetic:
>
> 256 * fs * 2 * mclk_src_scaling[i].param
>
> Addresses-Coverity-ID: 1432039 ("Unintentional integer overflow")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  sound/soc/codecs/nau8824.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c
> index 6bd1445..468d514 100644
> --- a/sound/soc/codecs/nau8824.c
> +++ b/sound/soc/codecs/nau8824.c
> @@ -1274,7 +1274,7 @@ static int nau8824_calc_fll_param(unsigned int fll_in,
>         fvco_max = 0;
>         fvco_sel = ARRAY_SIZE(mclk_src_scaling);
>         for (i = 0; i < ARRAY_SIZE(mclk_src_scaling); i++) {
> -               fvco = 256 * fs * 2 * mclk_src_scaling[i].param;
> +               fvco = 256ULL * fs * 2 * mclk_src_scaling[i].param;

This would be better written as fvco = (256 * 2 *
mclk_src_scaling[i].param) * (u64)fs;

There no reason the entire expression must use 64 bit multiplies,
since the left side
above is know to fit in 32-bits.

The code could be more efficient if mclk_src_scaling had been sorted
by scaling ratio
instead of register value.  In fact, most of the math in this loop
could be precomputed by the
compiler if one instead though of min and max fs range for a given scaler.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-07-06  2:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-05 13:06 [PATCH] ASoC: nau8824: use 64-bit arithmetic instead of 32-bit Gustavo A. R. Silva
2018-07-06  2:09 ` [alsa-devel] " Trent Piepho

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).