* [PATCH 0/2] Relax bitclk computation when using PLL @ 2017-04-21 13:07 Daniel Baluta 2017-04-21 13:07 ` [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning Daniel Baluta 2017-04-21 13:07 ` [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL Daniel Baluta 0 siblings, 2 replies; 8+ messages in thread From: Daniel Baluta @ 2017-04-21 13:07 UTC (permalink / raw) To: broonie, tiwai, ckeepax, arnd, lgirdwood Cc: alsa-devel, patches, Daniel Baluta, linux-kernel Using strict bitclk requirements we cannot support all promised rates and formats. For this reason we relax bitclk computation by choosing the best available bitclk. First patch in the series is based on Arnd's patch: http://mailman.alsa-project.org/pipermail/alsa-devel/2017-April/119899.html Second one does the actual bitclk relaxation. Daniel Baluta (2): ASoC: codec: wm9860: avoid maybe-uninitialized warning ASoC: codec: wm8960: Relax bit clock computation when using PLL sound/soc/codecs/wm8960.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning 2017-04-21 13:07 [PATCH 0/2] Relax bitclk computation when using PLL Daniel Baluta @ 2017-04-21 13:07 ` Daniel Baluta 2017-04-21 14:46 ` Arnd Bergmann 2017-04-21 13:07 ` [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL Daniel Baluta 1 sibling, 1 reply; 8+ messages in thread From: Daniel Baluta @ 2017-04-21 13:07 UTC (permalink / raw) To: broonie, tiwai, ckeepax, arnd, lgirdwood Cc: alsa-devel, patches, Daniel Baluta, linux-kernel 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 Fixes: 84fdc00d519f ("ASoC: codec: wm9860: Refactor PLL out freq search") Fixes: 303e8954af8d ("ASoC: codec: wm8960: Stop when a matching PLL freq is found") Suggested-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> --- Arnd, I agree that your code was more both humans and gcc anyhow for consistency with wm8960_configure_sysclk function I preferred to keep the "if(..) break" statements. sound/soc/codecs/wm8960.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c index ace69da..8c87153 100644 --- a/sound/soc/codecs/wm8960.c +++ b/sound/soc/codecs/wm8960.c @@ -702,7 +702,7 @@ int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in, bclk = wm8960->bclk; lrclk = wm8960->lrclk; - *bclk_idx = -1; + best_freq_out = -EINVAL; for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) { if (sysclk_divs[i] == -1) @@ -731,10 +731,7 @@ int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in, break; } - if (*bclk_idx != -1) - wm8960_set_pll(codec, freq_in, best_freq_out); - - return *bclk_idx; + return best_freq_out; } static int wm8960_configure_clocking(struct snd_soc_codec *codec) { @@ -783,11 +780,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 */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning 2017-04-21 13:07 ` [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning Daniel Baluta @ 2017-04-21 14:46 ` Arnd Bergmann 2017-04-24 13:15 ` Daniel Baluta 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2017-04-21 14:46 UTC (permalink / raw) To: Daniel Baluta Cc: alsa-devel, Linux Kernel Mailing List, patches, tiwai, Liam Girdwood, Mark Brown, Charles Keepax On Fri, Apr 21, 2017 at 3:07 PM, Daniel Baluta <daniel.baluta@nxp.com> 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 > > Fixes: 84fdc00d519f ("ASoC: codec: wm9860: Refactor PLL out freq search") > Fixes: 303e8954af8d ("ASoC: codec: wm8960: Stop when a matching PLL freq is found") > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > --- > Arnd, > > I agree that your code was more both humans and gcc anyhow > for consistency with wm8960_configure_sysclk function I preferred > to keep the "if(..) break" statements. How about changing both functions the same way then? Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning 2017-04-21 14:46 ` Arnd Bergmann @ 2017-04-24 13:15 ` Daniel Baluta 2017-04-24 15:27 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Daniel Baluta @ 2017-04-24 13:15 UTC (permalink / raw) To: Arnd Bergmann Cc: alsa-devel, patches, Liam Girdwood, Linux Kernel Mailing List, Mark Brown, Takashi Iwai, Daniel Baluta, Charles Keepax On Fri, Apr 21, 2017 at 5:46 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Fri, Apr 21, 2017 at 3:07 PM, Daniel Baluta <daniel.baluta@nxp.com> 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 >> >> Fixes: 84fdc00d519f ("ASoC: codec: wm9860: Refactor PLL out freq search") >> Fixes: 303e8954af8d ("ASoC: codec: wm8960: Stop when a matching PLL freq is found") >> Suggested-by: Arnd Bergmann <arnd@arndb.de> >> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> >> --- >> Arnd, >> >> I agree that your code was more both humans and gcc anyhow >> for consistency with wm8960_configure_sysclk function I preferred >> to keep the "if(..) break" statements. > > How about changing both functions the same way then? I've tried but I couldn't find any solution. For clarity here is how the code actually looks like. The git diff is a little bit misleading. Here is how wm8960_configure_pll code looks like: https://pastebin.com/naGdVNQz static int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in, » » » int *sysclk_idx, int *dac_idx, int *bclk_idx) { » struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec); » int sysclk, bclk, lrclk, freq_out; » int diff, closest, best_freq_out; » int i, j, k; » bclk = wm8960->bclk; » lrclk = wm8960->lrclk; » closest = freq_in; » best_freq_out = -EINVAL; » *sysclk_idx = *dac_idx = *bclk_idx = -1; » for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) { » » if (sysclk_divs[i] == -1) » » » continue; » » for (j = 0; j < ARRAY_SIZE(dac_divs); ++j) { » » » sysclk = lrclk * dac_divs[j]; » » » freq_out = sysclk * sysclk_divs[i]; » » » for (k = 0; k < ARRAY_SIZE(bclk_divs); ++k) { » » » » if (!is_pll_freq_available(freq_in, freq_out)) » » » » » continue; » » » » diff = sysclk - bclk * bclk_divs[k] / 10; » » » » if (diff == 0) { » » » » » *sysclk_idx = i; » » » » » *dac_idx = j; » » » » » *bclk_idx = k; » » » » » best_freq_out = freq_out; » » » » » break; » » » » } » » » » if (diff > 0 && closest > diff) { » » » » » *sysclk_idx = i; » » » » » *dac_idx = j; » » » » » *bclk_idx = k; » » » » » closest = diff; » » » » » best_freq_out = freq_out; » » » » } » » » } » » » if (k != ARRAY_SIZE(bclk_divs)) » » » » break; » » } » » if (j != ARRAY_SIZE(dac_divs)) » » » break; » } » return best_freq_out; } In my opinion this is a compiler false positive. Any clue on how to rework this would be welcomed :). I couldn't find any decent solution. Daniel. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning 2017-04-24 13:15 ` Daniel Baluta @ 2017-04-24 15:27 ` Arnd Bergmann 2017-04-25 10:17 ` Daniel Baluta 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2017-04-24 15:27 UTC (permalink / raw) To: Daniel Baluta Cc: alsa-devel, patches, Liam Girdwood, Linux Kernel Mailing List, Mark Brown, Takashi Iwai, Daniel Baluta, Charles Keepax On Mon, Apr 24, 2017 at 3:15 PM, Daniel Baluta <daniel.baluta@gmail.com> wrote: > On Fri, Apr 21, 2017 at 5:46 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> On Fri, Apr 21, 2017 at 3:07 PM, Daniel Baluta <daniel.baluta@nxp.com> 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 >>> >>> Fixes: 84fdc00d519f ("ASoC: codec: wm9860: Refactor PLL out freq search") >>> Fixes: 303e8954af8d ("ASoC: codec: wm8960: Stop when a matching PLL freq is found") >>> Suggested-by: Arnd Bergmann <arnd@arndb.de> >>> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> >>> --- >>> Arnd, >>> >>> I agree that your code was more both humans and gcc anyhow >>> for consistency with wm8960_configure_sysclk function I preferred >>> to keep the "if(..) break" statements. >> >> How about changing both functions the same way then? > > I've tried but I couldn't find any solution. For clarity here is how > the code actually looks like. > > The git diff is a little bit misleading. Here is how wm8960_configure_pll code > looks like: > > https://pastebin.com/naGdVNQz > > static > int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in, > » » » int *sysclk_idx, int *dac_idx, int *bclk_idx) > { > » struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec); > » int sysclk, bclk, lrclk, freq_out; > » int diff, closest, best_freq_out; > » int i, j, k; > > » bclk = wm8960->bclk; > » lrclk = wm8960->lrclk; > » closest = freq_in; > > » best_freq_out = -EINVAL; > » *sysclk_idx = *dac_idx = *bclk_idx = -1; > > » for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) { > » » if (sysclk_divs[i] == -1) > » » » continue; > » » for (j = 0; j < ARRAY_SIZE(dac_divs); ++j) { > » » » sysclk = lrclk * dac_divs[j]; > » » » freq_out = sysclk * sysclk_divs[i]; > > » » » for (k = 0; k < ARRAY_SIZE(bclk_divs); ++k) { > » » » » if (!is_pll_freq_available(freq_in, freq_out)) > » » » » » continue; > > » » » » diff = sysclk - bclk * bclk_divs[k] / 10; > » » » » if (diff == 0) { > » » » » » *sysclk_idx = i; > » » » » » *dac_idx = j; > » » » » » *bclk_idx = k; > » » » » » best_freq_out = freq_out; > » » » » » break; > » » » » } > » » » » if (diff > 0 && closest > diff) { > » » » » » *sysclk_idx = i; > » » » » » *dac_idx = j; > » » » » » *bclk_idx = k; > » » » » » closest = diff; > » » » » » best_freq_out = freq_out; > » » » » } > » » » } > » » » if (k != ARRAY_SIZE(bclk_divs)) > » » » » break; > » » } > » » if (j != ARRAY_SIZE(dac_divs)) > » » » break; > » } > > » return best_freq_out; > } > > In my opinion this is a compiler false positive. Any clue on how to rework this > would be welcomed :). I couldn't find any decent solution. Actually I think in this case the compiler is supposed to warn if best_freq_out is not initialized, as we would never set it in case is_pll_freq_available() returns false for all inputs or sysclk_divs[] is -1 for all fields. I'd leave the initialization then, and only replace the breaks with a goto (not tested): > » for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) { > » » if (sysclk_divs[i] == -1) > » » » continue; > » » for (j = 0; j < ARRAY_SIZE(dac_divs); ++j) { > » » » sysclk = lrclk * dac_divs[j]; > » » » freq_out = sysclk * sysclk_divs[i]; > > » » » for (k = 0; k < ARRAY_SIZE(bclk_divs); ++k) { > » » » » if (!is_pll_freq_available(freq_in, freq_out)) > » » » » » continue; > > » » » » diff = sysclk - bclk * bclk_divs[k] / 10; > » » » » if (diff == 0) { > » » » » » *sysclk_idx = i; > » » » » » *dac_idx = j; > » » » » » *bclk_idx = k; > » » » » » best_freq_out = freq_out; > » » » » » goto out; > » » » » } > » » » » if (diff > 0 && closest > diff) { > » » » » » *sysclk_idx = i; > » » » » » *dac_idx = j; > » » » » » *bclk_idx = k; > » » » » » closest = diff; > » » » » » best_freq_out = freq_out; > » » » » } > » » » } > » » } > » } >out: > » return best_freq_out; > } Arnd _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning 2017-04-24 15:27 ` Arnd Bergmann @ 2017-04-25 10:17 ` Daniel Baluta 0 siblings, 0 replies; 8+ messages in thread From: Daniel Baluta @ 2017-04-25 10:17 UTC (permalink / raw) To: Arnd Bergmann Cc: alsa-devel, patches, Liam Girdwood, Linux Kernel Mailing List, Mark Brown, Takashi Iwai, Daniel Baluta, Charles Keepax On Mon, Apr 24, 2017 at 6:27 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Mon, Apr 24, 2017 at 3:15 PM, Daniel Baluta <daniel.baluta@gmail.com> wrote: >> On Fri, Apr 21, 2017 at 5:46 PM, Arnd Bergmann <arnd@arndb.de> wrote: >>> On Fri, Apr 21, 2017 at 3:07 PM, Daniel Baluta <daniel.baluta@nxp.com> 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 >>>> >>>> Fixes: 84fdc00d519f ("ASoC: codec: wm9860: Refactor PLL out freq search") >>>> Fixes: 303e8954af8d ("ASoC: codec: wm8960: Stop when a matching PLL freq is found") >>>> Suggested-by: Arnd Bergmann <arnd@arndb.de> >>>> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> >>>> --- >>>> Arnd, >>>> >>>> I agree that your code was more both humans and gcc anyhow >>>> for consistency with wm8960_configure_sysclk function I preferred >>>> to keep the "if(..) break" statements. >>> >>> How about changing both functions the same way then? >> >> I've tried but I couldn't find any solution. For clarity here is how >> the code actually looks like. >> >> The git diff is a little bit misleading. Here is how wm8960_configure_pll code >> looks like: >> >> https://pastebin.com/naGdVNQz >> >> static >> int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in, >> » » » int *sysclk_idx, int *dac_idx, int *bclk_idx) >> { >> » struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec); >> » int sysclk, bclk, lrclk, freq_out; >> » int diff, closest, best_freq_out; >> » int i, j, k; >> >> » bclk = wm8960->bclk; >> » lrclk = wm8960->lrclk; >> » closest = freq_in; >> >> » best_freq_out = -EINVAL; >> » *sysclk_idx = *dac_idx = *bclk_idx = -1; >> >> » for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) { >> » » if (sysclk_divs[i] == -1) >> » » » continue; >> » » for (j = 0; j < ARRAY_SIZE(dac_divs); ++j) { >> » » » sysclk = lrclk * dac_divs[j]; >> » » » freq_out = sysclk * sysclk_divs[i]; >> >> » » » for (k = 0; k < ARRAY_SIZE(bclk_divs); ++k) { >> » » » » if (!is_pll_freq_available(freq_in, freq_out)) >> » » » » » continue; >> >> » » » » diff = sysclk - bclk * bclk_divs[k] / 10; >> » » » » if (diff == 0) { >> » » » » » *sysclk_idx = i; >> » » » » » *dac_idx = j; >> » » » » » *bclk_idx = k; >> » » » » » best_freq_out = freq_out; >> » » » » » break; >> » » » » } >> » » » » if (diff > 0 && closest > diff) { >> » » » » » *sysclk_idx = i; >> » » » » » *dac_idx = j; >> » » » » » *bclk_idx = k; >> » » » » » closest = diff; >> » » » » » best_freq_out = freq_out; >> » » » » } >> » » » } >> » » » if (k != ARRAY_SIZE(bclk_divs)) >> » » » » break; >> » » } >> » » if (j != ARRAY_SIZE(dac_divs)) >> » » » break; >> » } >> >> » return best_freq_out; >> } >> >> In my opinion this is a compiler false positive. Any clue on how to rework this >> would be welcomed :). I couldn't find any decent solution. > > Actually I think in this case the compiler is supposed to warn if > best_freq_out is not initialized, as we would never set it > in case is_pll_freq_available() returns false for all inputs or > sysclk_divs[] is -1 for all fields. > I'd leave the initialization then, and only replace the breaks > with a goto (not tested): > >> » for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) { >> » » if (sysclk_divs[i] == -1) >> » » » continue; >> » » for (j = 0; j < ARRAY_SIZE(dac_divs); ++j) { >> » » » sysclk = lrclk * dac_divs[j]; >> » » » freq_out = sysclk * sysclk_divs[i]; >> >> » » » for (k = 0; k < ARRAY_SIZE(bclk_divs); ++k) { >> » » » » if (!is_pll_freq_available(freq_in, freq_out)) >> » » » » » continue; >> >> » » » » diff = sysclk - bclk * bclk_divs[k] / 10; >> » » » » if (diff == 0) { >> » » » » » *sysclk_idx = i; >> » » » » » *dac_idx = j; >> » » » » » *bclk_idx = k; >> » » » » » best_freq_out = freq_out; >> » » » » » goto out; >> » » » » } >> » » » » if (diff > 0 && closest > diff) { >> » » » » » *sysclk_idx = i; >> » » » » » *dac_idx = j; >> » » » » » *bclk_idx = k; >> » » » » » closest = diff; >> » » » » » best_freq_out = freq_out; >> » » » » } >> » » » } >> » » } >> » } >>out: >> » return best_freq_out; >> } Sure, this looks reasonable. I will send v2. Daniel. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL 2017-04-21 13:07 [PATCH 0/2] Relax bitclk computation when using PLL Daniel Baluta 2017-04-21 13:07 ` [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning Daniel Baluta @ 2017-04-21 13:07 ` Daniel Baluta 2017-04-21 14:44 ` Arnd Bergmann 1 sibling, 1 reply; 8+ messages in thread From: Daniel Baluta @ 2017-04-21 13:07 UTC (permalink / raw) To: broonie, tiwai, ckeepax, arnd, lgirdwood Cc: patches, alsa-devel, linux-kernel, Daniel Baluta Bitclk is derived from sysclk using bclk_divs. Sysclk can be derived in two ways: (1) directly from MLCK (2) MCLK via PLL Commit 3c01b9ee2ab9d0d ("ASoC: codec: wm8960: Relax bit clock computation") relaxed bitclk computation when sysclk is directly derived from MCLK. Lets do the same thing when sysclk is derived via PLL. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> --- Here, I forced the following harmless initialization: *sysclk_idx = *dac_idx = *bclk_idx = -1; otherwise I would trigger a gcc false positive warning: sound/soc/codecs/wm8960.c: In function 'wm8960_configure_clocking': sound/soc/codecs/wm8960.c:810:46: warning: 'j' may be used uninitialized in this function [-Wmaybe-uninitialized] snd_soc_update_bits(codec, WM8960_CLOCK1, 0x7 << 6, j << 6); ~~^~~~ sound/soc/codecs/wm8960.c:806:44: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized] snd_soc_update_bits(codec, WM8960_CLOCK1, 3 << 1, i << 1); ~~^~~~ sound/soc/codecs/wm8960.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c index 8c87153..60700d5 100644 --- a/sound/soc/codecs/wm8960.c +++ b/sound/soc/codecs/wm8960.c @@ -679,6 +679,10 @@ int wm8960_configure_sysclk(struct wm8960_priv *wm8960, int mclk, * - freq_out = sysclk * sysclk_divs * - 10 * sysclk = bclk * bclk_divs * + * If we cannot find an exact match for (sysclk, lrclk, bclk) + * triplet, we relax the bclk such that bclk is chosen as the + * closest available frequency greater than expected bclk. + * * @codec: codec structure * @freq_in: input frequency used to derive freq out via PLL * @sysclk_idx: sysclk_divs index for found sysclk @@ -696,13 +700,15 @@ 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, closest, best_freq_out; int i, j, k; bclk = wm8960->bclk; lrclk = wm8960->lrclk; + closest = freq_in; best_freq_out = -EINVAL; + *sysclk_idx = *dac_idx = *bclk_idx = -1; for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) { if (sysclk_divs[i] == -1) @@ -723,6 +729,13 @@ int wm8960_configure_pll(struct snd_soc_codec *codec, int freq_in, best_freq_out = freq_out; break; } + if (diff > 0 && closest > diff) { + *sysclk_idx = i; + *dac_idx = j; + *bclk_idx = k; + closest = diff; + best_freq_out = freq_out; + } } if (k != ARRAY_SIZE(bclk_divs)) break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL 2017-04-21 13:07 ` [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL Daniel Baluta @ 2017-04-21 14:44 ` Arnd Bergmann 0 siblings, 0 replies; 8+ messages in thread From: Arnd Bergmann @ 2017-04-21 14:44 UTC (permalink / raw) To: Daniel Baluta Cc: alsa-devel, Linux Kernel Mailing List, patches, tiwai, Liam Girdwood, Mark Brown, Charles Keepax On Fri, Apr 21, 2017 at 3:07 PM, Daniel Baluta <daniel.baluta@nxp.com> wrote: > Bitclk is derived from sysclk using bclk_divs. > Sysclk can be derived in two ways: > (1) directly from MLCK > (2) MCLK via PLL > > Commit 3c01b9ee2ab9d0d ("ASoC: codec: wm8960: Relax bit clock > computation") > relaxed bitclk computation when sysclk is directly derived from MCLK. > > Lets do the same thing when sysclk is derived via PLL. > > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> > --- > Here, I forced the following harmless initialization: > > *sysclk_idx = *dac_idx = *bclk_idx = -1; > > otherwise I would trigger a gcc false positive warning: > > sound/soc/codecs/wm8960.c: In function 'wm8960_configure_clocking': > sound/soc/codecs/wm8960.c:810:46: warning: 'j' may be used uninitialized > in this function [-Wmaybe-uninitialized] > snd_soc_update_bits(codec, WM8960_CLOCK1, 0x7 << 6, j << 6); > ~~^~~~ > sound/soc/codecs/wm8960.c:806:44: warning: 'i' may be used uninitialized > in this function [-Wmaybe-uninitialized] > snd_soc_update_bits(codec, WM8960_CLOCK1, 3 << 1, i << 1); > ~~^~~~ I saw the same warning earlier, but it was gone after the rework I posted the other day. Please try if that works for you as well, I think that would be better than a bogus initialization. Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-25 10:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-21 13:07 [PATCH 0/2] Relax bitclk computation when using PLL Daniel Baluta 2017-04-21 13:07 ` [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning Daniel Baluta 2017-04-21 14:46 ` Arnd Bergmann 2017-04-24 13:15 ` Daniel Baluta 2017-04-24 15:27 ` Arnd Bergmann 2017-04-25 10:17 ` Daniel Baluta 2017-04-21 13:07 ` [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL Daniel Baluta 2017-04-21 14:44 ` Arnd Bergmann
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).