Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1627073

Re: [PATCH] ASoC: codec: wm9860: avoid maybe-uninitialized warning

Show all headers | View raw

Hi Arnd,

On Wed, Apr 19, 2017 at 8:04 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> The new PLL configuration code triggers a harmless warning:
>
> sound/soc/codecs/wm8960.c: In function 'wm8960_configure_clocking':
> sound/soc/codecs/wm8960.c:735:3: error: 'best_freq_out' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    wm8960_set_pll(codec, freq_in, best_freq_out);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> sound/soc/codecs/wm8960.c:699:12: note: 'best_freq_out' was declared here
>
> I think the warning was introduced by Daniel's bugfix. I've come up
> with a way to simplify the code in a way that is more readable to
> both humans and to gcc, which gets us rid of the warning.

I've struggled with this kind of warning when reworking wm8960
bitclock computation.

Anyhow, for this exact patch the warning didn't showed up.

I used:

gcc version 6.2.1 20161016 (Linaro GCC 6.2-2016.11)

My next patch:

https://patchwork.kernel.org/patch/9666921/ [1]

which is under review, had the issue you mention (in v1)

https://lkml.org/lkml/2017/4/5/246

but I initialized best_freq_out with 0 to get rid of the problem.

>
> Fixes: 84fdc00d519f ("ASoC: codec: wm9860: Refactor PLL out freq search")
> Fixes: 303e8954af8d ("ASoC: codec: wm8960: Stop when a matching PLL freq is found")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  sound/soc/codecs/wm8960.c | 25 ++++++++-----------------
>  1 file changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> index ace69da97cb8..ba0f80504c6b 100644
> --- a/sound/soc/codecs/wm8960.c
> +++ b/sound/soc/codecs/wm8960.c
> @@ -696,14 +696,12 @@ int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in,
>  {
>         struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
>         int sysclk, bclk, lrclk, freq_out;
> -       int diff, best_freq_out;
> +       int diff;
>         int i, j, k;
>
>         bclk = wm8960->bclk;
>         lrclk = wm8960->lrclk;
>
> -       *bclk_idx = -1;
> -
>         for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) {
>                 if (sysclk_divs[i] == -1)
>                         continue;
> @@ -720,22 +718,14 @@ int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in,
>                                         *sysclk_idx = i;
>                                         *dac_idx = j;
>                                         *bclk_idx = k;
> -                                       best_freq_out = freq_out;
> -                                       break;
> +                                       return freq_out;
>                                 }
>                         }
> -                       if (k != ARRAY_SIZE(bclk_divs))
> -                               break;
>                 }
> -               if (j != ARRAY_SIZE(dac_divs))
> -                       break;
>         }
> -
> -       if (*bclk_idx != -1)
> -               wm8960_set_pll(codec, freq_in, best_freq_out);

I think the compiler is tricked into thinking that best_freq_out might
be uninitialized. Notice
that each time *bclk_idx is assigned a value (which is always >=0) we
also initialize best_freq_out.

> -
> -       return *bclk_idx;
> +       return -EINVAL;
>  }
> +
>  static int wm8960_configure_clocking(struct snd_soc_codec *codec)
>  {
>         struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
> @@ -783,11 +773,12 @@ static int wm8960_configure_clocking(struct snd_soc_codec *codec)
>                 }
>         }
>
> -       ret = wm8960_configure_pll(codec, freq_in, &i, &j, &k);
> -       if (ret < 0) {
> +       freq_out = wm8960_configure_pll(codec, freq_in, &i, &j, &k);
> +       if (freq_out < 0) {
>                 dev_err(codec->dev, "failed to configure clock via PLL\n");
> -               return -EINVAL;
> +               return freq_out;
>         }
> +       wm8960_set_pll(codec, freq_in, freq_out);
>
>  configure_clock:
>         /* configure sysclk clock */

Your idea looks good, will need to rework my follow up patch on it:

https://patchwork.kernel.org/patch/9666921/

thanks,
Daniel.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] ASoC: codec: wm9860: avoid maybe-uninitialized warning Arnd Bergmann <arnd@arndb.de> - 2017-04-19 19:10 +0200
  Re: [PATCH] ASoC: codec: wm9860: avoid maybe-uninitialized warning Daniel Baluta <daniel.baluta@gmail.com> - 2017-04-20 08:50 +0200
    Re: [PATCH] ASoC: codec: wm9860: avoid maybe-uninitialized warning Arnd Bergmann <arnd@arndb.de> - 2017-04-20 09:50 +0200
      Re: [PATCH] ASoC: codec: wm9860: avoid maybe-uninitialized warning Daniel Baluta <daniel.baluta@gmail.com> - 2017-04-20 10:30 +0200
      Re: [PATCH] ASoC: codec: wm9860: avoid maybe-uninitialized warning Mark Brown <broonie@kernel.org> - 2017-04-20 21:50 +0200
  Re: [PATCH] ASoC: codec: wm9860: avoid maybe-uninitialized warning Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2017-04-20 11:10 +0200

csiph-web