* [PATCH 0/2] Relax bitclk computation when using PLL
@ 2017-04-21 13:07 ` Daniel Baluta
0 siblings, 0 replies; 24+ 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
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] 24+ messages in thread
* [PATCH 0/2] Relax bitclk computation when using PLL
@ 2017-04-21 13:07 ` Daniel Baluta
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
2017-04-21 13:07 ` Daniel Baluta
@ 2017-04-21 13:07 ` Daniel Baluta
-1 siblings, 0 replies; 24+ 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
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] 24+ messages in thread
* [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
@ 2017-04-21 13:07 ` Daniel Baluta
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
2017-04-21 13:07 ` Daniel Baluta
@ 2017-04-21 14:46 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2017-04-21 14:46 UTC (permalink / raw)
To: Daniel Baluta
Cc: Mark Brown, tiwai, Charles Keepax, Liam Girdwood, patches,
alsa-devel, Linux Kernel Mailing List
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] 24+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
@ 2017-04-21 14:46 ` Arnd Bergmann
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
2017-04-21 14:46 ` Arnd Bergmann
@ 2017-04-24 13:15 ` Daniel Baluta
-1 siblings, 0 replies; 24+ messages in thread
From: Daniel Baluta @ 2017-04-24 13:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Baluta, alsa-devel, Linux Kernel Mailing List, patches,
Takashi Iwai, Liam Girdwood, Mark Brown, 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.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
@ 2017-04-24 13:15 ` Daniel Baluta
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
2017-04-24 13:15 ` Daniel Baluta
@ 2017-04-24 15:27 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2017-04-24 15:27 UTC (permalink / raw)
To: Daniel Baluta
Cc: Daniel Baluta, alsa-devel, Linux Kernel Mailing List, patches,
Takashi Iwai, Liam Girdwood, Mark Brown, 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
@ 2017-04-24 15:27 ` Arnd Bergmann
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
2017-04-24 15:27 ` Arnd Bergmann
@ 2017-04-25 10:17 ` Daniel Baluta
-1 siblings, 0 replies; 24+ messages in thread
From: Daniel Baluta @ 2017-04-25 10:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Baluta, alsa-devel, Linux Kernel Mailing List, patches,
Takashi Iwai, Liam Girdwood, Mark Brown, 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.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] ASoC: codec: wm9860: avoid maybe-uninitialized warning
@ 2017-04-25 10:17 ` Daniel Baluta
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
2017-04-21 13:07 ` Daniel Baluta
@ 2017-04-21 13:07 ` Daniel Baluta
-1 siblings, 0 replies; 24+ 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] 24+ messages in thread
* [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-21 13:07 ` Daniel Baluta
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
2017-04-21 13:07 ` Daniel Baluta
@ 2017-04-21 14:44 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2017-04-21 14:44 UTC (permalink / raw)
To: Daniel Baluta
Cc: Mark Brown, tiwai, Charles Keepax, Liam Girdwood, patches,
alsa-devel, Linux Kernel Mailing List
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] 24+ messages in thread
* Re: [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-21 14:44 ` Arnd Bergmann
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* [PATCH 0/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-06 11:51 Daniel Baluta
2017-04-06 11:51 ` Daniel Baluta
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Baluta @ 2017-04-06 11:51 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, broonie, perex, tiwai, linux-kernel, patches, ckeepax,
shengjiu.wang, viorel.suman, mihai.serban
This is a follow up of patch series sent here:
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-April/119517.html
where only patch 1/2 was merged.
In this new patch series, we fix a problem with patch already merged and
then we relax bit clock computation when using PLL the same as we did
in patch 2/2 of previous series.
Daniel Baluta (2):
ASoC: codec: wm8960: Stop when a matching PLL freq is found
ASoC: codec: wm8960: Relax bit clock computation when using PLL
sound/soc/codecs/wm8960.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
2017-04-06 11:51 [PATCH 0/2] " Daniel Baluta
@ 2017-04-06 11:51 ` Daniel Baluta
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Baluta @ 2017-04-06 11:51 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, broonie, perex, tiwai, linux-kernel, patches, ckeepax,
shengjiu.wang, viorel.suman, mihai.serban
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>
Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
sound/soc/codecs/wm8960.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index ace69da..8ab9fa2 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,14 @@ 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 = 0;
int i, j, k;
bclk = wm8960->bclk;
lrclk = wm8960->lrclk;
+ closest = freq_in;
- *bclk_idx = -1;
+ *bclk_idx = *dac_idx = *sysclk_idx = -1;
for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) {
if (sysclk_divs[i] == -1)
@@ -723,6 +728,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] 24+ messages in thread
* [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-06 11:51 ` Daniel Baluta
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Baluta @ 2017-04-06 11:51 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, broonie, perex, tiwai, linux-kernel, patches, ckeepax,
shengjiu.wang, viorel.suman, mihai.serban
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>
Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
sound/soc/codecs/wm8960.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index ace69da..8ab9fa2 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,14 @@ 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 = 0;
int i, j, k;
bclk = wm8960->bclk;
lrclk = wm8960->lrclk;
+ closest = freq_in;
- *bclk_idx = -1;
+ *bclk_idx = *dac_idx = *sysclk_idx = -1;
for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) {
if (sysclk_divs[i] == -1)
@@ -723,6 +728,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] 24+ messages in thread
* [PATCH 0/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-04 16:45 Daniel Baluta
2017-04-04 16:45 ` Daniel Baluta
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Baluta @ 2017-04-04 16:45 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, broonie, perex, tiwai, linux-kernel, patches, ckeepax,
shengjiu.wang, viorel.suman, mihai.serban
This is a follow up of commit 3c01b9ee2ab ("ASoC: codec: wm8960: Relax bit clock computation")
where we relaxed bitclk when sysclk was derived from MCLK.
Now, we do the same thing for sysclk derived using PLL.
Daniel Baluta (2):
ASoC: codec: wm9860: Refactor PLL out freq search
ASoC: codec: wm8960: Relax bit clock computation when using PLL
sound/soc/codecs/wm8960.c | 105 +++++++++++++++++++++++++++++++++-------------
1 file changed, 76 insertions(+), 29 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
2017-04-04 16:45 [PATCH 0/2] " Daniel Baluta
@ 2017-04-04 16:45 ` Daniel Baluta
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Baluta @ 2017-04-04 16:45 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, broonie, perex, tiwai, linux-kernel, patches, ckeepax,
shengjiu.wang, viorel.suman, mihai.serban
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>
---
sound/soc/codecs/wm8960.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 36c8454..e8cb764 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,11 +700,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, closest, best_freq_out;
int i, j, k;
bclk = wm8960->bclk;
lrclk = wm8960->lrclk;
+ closest = freq_in;
*bclk_idx = -1;
@@ -723,6 +728,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;
+ }
}
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-04 16:45 ` Daniel Baluta
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Baluta @ 2017-04-04 16:45 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, broonie, perex, tiwai, linux-kernel, patches, ckeepax,
shengjiu.wang, viorel.suman, mihai.serban
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>
---
sound/soc/codecs/wm8960.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 36c8454..e8cb764 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,11 +700,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, closest, best_freq_out;
int i, j, k;
bclk = wm8960->bclk;
lrclk = wm8960->lrclk;
+ closest = freq_in;
*bclk_idx = -1;
@@ -723,6 +728,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;
+ }
}
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
2017-04-04 16:45 ` Daniel Baluta
@ 2017-04-05 9:02 ` Charles Keepax
-1 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2017-04-05 9:02 UTC (permalink / raw)
To: Daniel Baluta
Cc: alsa-devel, lgirdwood, broonie, perex, tiwai, linux-kernel,
patches, shengjiu.wang, viorel.suman, mihai.serban
On Tue, Apr 04, 2017 at 07:45:14PM +0300, Daniel Baluta 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>
> ---
Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Thanks,
Charles
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-05 9:02 ` Charles Keepax
0 siblings, 0 replies; 24+ messages in thread
From: Charles Keepax @ 2017-04-05 9:02 UTC (permalink / raw)
To: Daniel Baluta
Cc: alsa-devel, lgirdwood, shengjiu.wang, patches, linux-kernel,
tiwai, broonie, viorel.suman, mihai.serban
On Tue, Apr 04, 2017 at 07:45:14PM +0300, Daniel Baluta 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>
> ---
Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Thanks,
Charles
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
2017-04-04 16:45 ` Daniel Baluta
@ 2017-04-05 10:17 ` kbuild test robot
-1 siblings, 0 replies; 24+ messages in thread
From: kbuild test robot @ 2017-04-05 10:17 UTC (permalink / raw)
To: Daniel Baluta
Cc: kbuild-all, alsa-devel, lgirdwood, broonie, perex, tiwai,
linux-kernel, patches, ckeepax, shengjiu.wang, viorel.suman,
mihai.serban
[-- Attachment #1: Type: text/plain, Size: 3773 bytes --]
Hi Daniel,
[auto build test WARNING on asoc/for-next]
[also build test WARNING on next-20170405]
[cannot apply to v4.11-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Baluta/ASoC-codec-wm8960-Relax-bit-clock-computation-when-using-PLL/20170405-144647
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: i386-randconfig-s0-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
sound/soc/codecs/wm8960.c: In function 'wm8960_configure_clocking':
>> sound/soc/codecs/wm8960.c:743:3: warning: 'best_freq_out' may be used uninitialized in this function [-Wmaybe-uninitialized]
wm8960_set_pll(codec, freq_in, best_freq_out);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/wm8960.c:703:21: note: 'best_freq_out' was declared here
int diff, closest, best_freq_out;
^~~~~~~~~~~~~
sound/soc/codecs/wm8960.c:806:56: 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:802:54: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized]
snd_soc_update_bits(codec, WM8960_CLOCK1, 3 << 1, i << 1);
~~^~~~
vim +/best_freq_out +743 sound/soc/codecs/wm8960.c
6b662dee Daniel Baluta 2017-04-04 727 *bclk_idx = k;
6b662dee Daniel Baluta 2017-04-04 728 best_freq_out = freq_out;
6b662dee Daniel Baluta 2017-04-04 729 break;
6b662dee Daniel Baluta 2017-04-04 730 }
16c42f46 Daniel Baluta 2017-04-04 731 if (diff > 0 && closest > diff) {
16c42f46 Daniel Baluta 2017-04-04 732 *sysclk_idx = i;
16c42f46 Daniel Baluta 2017-04-04 733 *dac_idx = j;
16c42f46 Daniel Baluta 2017-04-04 734 *bclk_idx = k;
16c42f46 Daniel Baluta 2017-04-04 735 closest = diff;
16c42f46 Daniel Baluta 2017-04-04 736 best_freq_out = freq_out;
16c42f46 Daniel Baluta 2017-04-04 737 }
6b662dee Daniel Baluta 2017-04-04 738 }
6b662dee Daniel Baluta 2017-04-04 739 }
6b662dee Daniel Baluta 2017-04-04 740 }
6b662dee Daniel Baluta 2017-04-04 741
6b662dee Daniel Baluta 2017-04-04 742 if (*bclk_idx != -1)
6b662dee Daniel Baluta 2017-04-04 @743 wm8960_set_pll(codec, freq_in, best_freq_out);
6b662dee Daniel Baluta 2017-04-04 744
6b662dee Daniel Baluta 2017-04-04 745 return *bclk_idx;
6b662dee Daniel Baluta 2017-04-04 746 }
3176bf2d Zidan Wang 2015-08-11 747 static int wm8960_configure_clocking(struct snd_soc_codec *codec)
0e50b51a Zidan Wang 2015-05-12 748 {
0e50b51a Zidan Wang 2015-05-12 749 struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
6b662dee Daniel Baluta 2017-04-04 750 int freq_out, freq_in;
0e50b51a Zidan Wang 2015-05-12 751 u16 iface1 = snd_soc_read(codec, WM8960_IFACE1);
:::::: The code at line 743 was first introduced by commit
:::::: 6b662deec0da0ad4f9dce8112d01828ed72b5a4c ASoC: codec: wm9860: Refactor PLL out freq search
:::::: TO: Daniel Baluta <daniel.baluta@nxp.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33329 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] ASoC: codec: wm8960: Relax bit clock computation when using PLL
@ 2017-04-05 10:17 ` kbuild test robot
0 siblings, 0 replies; 24+ messages in thread
From: kbuild test robot @ 2017-04-05 10:17 UTC (permalink / raw)
To: Daniel Baluta
Cc: alsa-devel, lgirdwood, linux-kernel, shengjiu.wang, patches,
tiwai, broonie, kbuild-all, viorel.suman, mihai.serban, ckeepax
[-- Attachment #1: Type: text/plain, Size: 3773 bytes --]
Hi Daniel,
[auto build test WARNING on asoc/for-next]
[also build test WARNING on next-20170405]
[cannot apply to v4.11-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Baluta/ASoC-codec-wm8960-Relax-bit-clock-computation-when-using-PLL/20170405-144647
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: i386-randconfig-s0-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
sound/soc/codecs/wm8960.c: In function 'wm8960_configure_clocking':
>> sound/soc/codecs/wm8960.c:743:3: warning: 'best_freq_out' may be used uninitialized in this function [-Wmaybe-uninitialized]
wm8960_set_pll(codec, freq_in, best_freq_out);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/wm8960.c:703:21: note: 'best_freq_out' was declared here
int diff, closest, best_freq_out;
^~~~~~~~~~~~~
sound/soc/codecs/wm8960.c:806:56: 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:802:54: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized]
snd_soc_update_bits(codec, WM8960_CLOCK1, 3 << 1, i << 1);
~~^~~~
vim +/best_freq_out +743 sound/soc/codecs/wm8960.c
6b662dee Daniel Baluta 2017-04-04 727 *bclk_idx = k;
6b662dee Daniel Baluta 2017-04-04 728 best_freq_out = freq_out;
6b662dee Daniel Baluta 2017-04-04 729 break;
6b662dee Daniel Baluta 2017-04-04 730 }
16c42f46 Daniel Baluta 2017-04-04 731 if (diff > 0 && closest > diff) {
16c42f46 Daniel Baluta 2017-04-04 732 *sysclk_idx = i;
16c42f46 Daniel Baluta 2017-04-04 733 *dac_idx = j;
16c42f46 Daniel Baluta 2017-04-04 734 *bclk_idx = k;
16c42f46 Daniel Baluta 2017-04-04 735 closest = diff;
16c42f46 Daniel Baluta 2017-04-04 736 best_freq_out = freq_out;
16c42f46 Daniel Baluta 2017-04-04 737 }
6b662dee Daniel Baluta 2017-04-04 738 }
6b662dee Daniel Baluta 2017-04-04 739 }
6b662dee Daniel Baluta 2017-04-04 740 }
6b662dee Daniel Baluta 2017-04-04 741
6b662dee Daniel Baluta 2017-04-04 742 if (*bclk_idx != -1)
6b662dee Daniel Baluta 2017-04-04 @743 wm8960_set_pll(codec, freq_in, best_freq_out);
6b662dee Daniel Baluta 2017-04-04 744
6b662dee Daniel Baluta 2017-04-04 745 return *bclk_idx;
6b662dee Daniel Baluta 2017-04-04 746 }
3176bf2d Zidan Wang 2015-08-11 747 static int wm8960_configure_clocking(struct snd_soc_codec *codec)
0e50b51a Zidan Wang 2015-05-12 748 {
0e50b51a Zidan Wang 2015-05-12 749 struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
6b662dee Daniel Baluta 2017-04-04 750 int freq_out, freq_in;
0e50b51a Zidan Wang 2015-05-12 751 u16 iface1 = snd_soc_read(codec, WM8960_IFACE1);
:::::: The code at line 743 was first introduced by commit
:::::: 6b662deec0da0ad4f9dce8112d01828ed72b5a4c ASoC: codec: wm9860: Refactor PLL out freq search
:::::: TO: Daniel Baluta <daniel.baluta@nxp.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33329 bytes --]
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-04-25 10:17 UTC | newest]
Thread overview: 24+ 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 ` 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:46 ` Arnd Bergmann
2017-04-21 14:46 ` Arnd Bergmann
2017-04-24 13:15 ` [alsa-devel] " Daniel Baluta
2017-04-24 13:15 ` Daniel Baluta
2017-04-24 15:27 ` [alsa-devel] " Arnd Bergmann
2017-04-24 15:27 ` Arnd Bergmann
2017-04-25 10:17 ` [alsa-devel] " Daniel Baluta
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 13:07 ` Daniel Baluta
2017-04-21 14:44 ` Arnd Bergmann
2017-04-21 14:44 ` Arnd Bergmann
-- strict thread matches above, loose matches on Subject: below --
2017-04-06 11:51 [PATCH 0/2] " Daniel Baluta
2017-04-06 11:51 ` [PATCH 2/2] " Daniel Baluta
2017-04-06 11:51 ` Daniel Baluta
2017-04-04 16:45 [PATCH 0/2] " Daniel Baluta
2017-04-04 16:45 ` [PATCH 2/2] " Daniel Baluta
2017-04-04 16:45 ` Daniel Baluta
2017-04-05 9:02 ` Charles Keepax
2017-04-05 9:02 ` Charles Keepax
2017-04-05 10:17 ` kbuild test robot
2017-04-05 10:17 ` kbuild test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.