On Wed, Mar 15, 2017 at 05:33:06PM +0200, Daniel Baluta wrote:
> WM8960 derives bit clock from sysclock using BCLKDIV[3:0] of R8
> clocking register (See WM8960 datasheet, page 71).
>
> There are use cases, like this:
> aplay -Dhw:0,0 -r 48000 -c 1 -f S20_3LE -t raw audio48k20b_3LE1c.pcm
>
> where no BCLKDIV applied to sysclock can give us the exact requested
> bitclk, so driver fails to configure clocking and aplay fails to run.
>
> Fix this by relaxing bitclk computation, so that when no exact value
> can be derived from sysclk pick the closest value greater than
> expected bitclk.
>
> Suggested-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Daniel Baluta <daniel.baluta@xxxxxxx>
> ---
> sound/soc/codecs/wm8960.c | 39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> index cb2ff2d..1669b45 100644
> --- a/sound/soc/codecs/wm8960.c
> +++ b/sound/soc/codecs/wm8960.c
> @@ -611,6 +611,10 @@ static const int bclk_divs[] = {
> * - lrclk = sysclk / dac_divs
> * - 10 * bclk = sysclk / 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.
> + *
> * @wm8960_priv: wm8960 codec private data
> * @mclk: MCLK used to derive sysclk
> * @_i: sysclk_divs index for found sysclk
> @@ -620,14 +624,14 @@ static const int bclk_divs[] = {
> * Returns:
> * -1, in case no sysclk frequency available found
> * 0, in case an exact match is found. See @_i, @_j, @_k
> - *
> + * >0, in case a relaxed match is found. See @_i, @_j, @_k
> */
> int wm8960_configure_sysclk(struct wm8960_priv *wm8960, int mclk,
> int *_i, int *_j, int *_k)
> {
> int sysclk, bclk, lrclk;
> int i, j, k;
> - int diff;
> + int diff, closest = mclk;
Don't you need to initialise diff here too?
>
> bclk = wm8960->bclk;
> lrclk = wm8960->lrclk;
> @@ -648,6 +652,12 @@ int wm8960_configure_sysclk(struct wm8960_priv *wm8960, int mclk,
> *_k = k;
> break;
> }
> + if (diff > 0 && closest > diff) {
> + *_i = i;
> + *_j = j;
> + *_k = k;
> + closest = diff;
> + }
> }
> if (k != ARRAY_SIZE(bclk_divs))
> break;
> @@ -656,10 +666,16 @@ int wm8960_configure_sysclk(struct wm8960_priv *wm8960, int mclk,
> break;
> }
>
> + /* exact match */
> if (i != ARRAY_SIZE(sysclk_divs))
> return 0;
>
> - return -1;
> + /* no match */
> + if (closest == mclk)
> + return -1;
> +
> + /* relaxed match */
> + return 1;
Do we need to differenciate between relaxed and an actual match?
> }
>
> static int wm8960_configure_clocking(struct snd_soc_codec *codec)
> @@ -668,6 +684,7 @@ static int wm8960_configure_clocking(struct snd_soc_codec *codec)
> int sysclk, bclk, lrclk, freq_out, freq_in;
> u16 iface1 = snd_soc_read(codec, WM8960_IFACE1);
> int i, j, k;
> + int best_sysclk_div, best_dac_div, best_bclk_div = -1;
> int ret;
>
> if (!(iface1 & (1<<6))) {
> @@ -705,10 +722,14 @@ static int wm8960_configure_clocking(struct snd_soc_codec *codec)
> ret = wm8960_configure_sysclk(wm8960, freq_out, &i, &j, &k);
> if (ret == 0)
> goto configure_clock;
> - else if (wm8960->clk_id != WM8960_SYSCLK_AUTO) {
> + else if (ret < 0 && wm8960->clk_id != WM8960_SYSCLK_AUTO) {
> dev_err(codec->dev, "failed to configure clock\n");
> return -EINVAL;
> }
> + /* there is still hope, keep this if no PLL out available */
> + best_sysclk_div = i;
> + best_dac_div = j;
> + best_bclk_div = k;
Enabling the PLL just to avoid running the BCLK a little fast
doesn't really seem worth it and it would make the code much more
obvious.
> }
> /* get a available pll out frequency and set pll */
> for (i = 0; i < ARRAY_SIZE(sysclk_divs); ++i) {
> @@ -736,8 +757,14 @@ static int wm8960_configure_clocking(struct snd_soc_codec *codec)
> }
>
> if (i == ARRAY_SIZE(sysclk_divs)) {
> - dev_err(codec->dev, "failed to configure clock\n");
> - return -EINVAL;
> + if (best_bclk_div != -1) {
> + i = best_sysclk_div;
> + j = best_dac_div;
> + k = best_bclk_div;
> + } else {
> + dev_err(codec->dev, "failed to configure clock\n");
> + return -EINVAL;
> + }
> }
>
> configure_clock:
> --
> 2.7.4
Thanks,
Charles
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
