2014-01-14 19:34:26

by Daniel Matuschek

[permalink] [raw]
Subject: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

WM8804 can run with PLL frequencies of 256xfs and 128xfs for
most sample rates. At 192kHz only 128xfs is supported. The
existing driver selects 128xfs automatically for some lower
samples rates. By using an additional mclk_div divider, it
is now possible to control the behaviour. This allows using
256xfs PLL frequency on all sample rates up to 96kHz. It
should allow lower jitter and better signal quality. The
behavior has to be controlled by the sound card driver,
because some sample frequency share the same setting. e.g.
192kHz and 96kHz use 24.576MHz master clock. The only
difference is the MCLK divider.

Signed-off-by: Daniel Matuschek <[email protected]>

---
sound/soc/codecs/wm8804.c | 17 ++++++++++++++---
sound/soc/codecs/wm8804.h | 4 ++++
2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8804.c b/sound/soc/codecs/wm8804.c
index 1704b1e..4619bf8 100644
--- a/sound/soc/codecs/wm8804.c
+++ b/sound/soc/codecs/wm8804.c
@@ -63,6 +63,7 @@ struct wm8804_priv {
struct regmap *regmap;
struct regulator_bulk_data supplies[WM8804_NUM_SUPPLIES];
struct notifier_block disable_nb[WM8804_NUM_SUPPLIES];
+ int mclk_div;
};

static int txsrc_get(struct snd_kcontrol *kcontrol,
@@ -318,7 +319,7 @@ static struct {

#define FIXED_PLL_SIZE ((1ULL << 22) * 10)
static int pll_factors(struct pll_div *pll_div, unsigned int target,
- unsigned int source)
+ unsigned int source, unsigned int mclk_div)
{
u64 Kpart;
unsigned long int K, Ndiv, Nmod, tmp;
@@ -330,7 +331,8 @@ static int pll_factors(struct pll_div *pll_div, unsigned int target,
*/
for (i = 0; i < ARRAY_SIZE(post_table); i++) {
tmp = target * post_table[i].div;
- if (tmp >= 90000000 && tmp <= 100000000) {
+ if ((tmp >= 90000000 && tmp <= 100000000) &&
+ (mclk_div == post_table[i].mclkdiv)) {
pll_div->freqmode = post_table[i].freqmode;
pll_div->mclkdiv = post_table[i].mclkdiv;
target *= post_table[i].div;
@@ -387,8 +389,12 @@ static int wm8804_set_pll(struct snd_soc_dai *dai, int pll_id,
} else {
int ret;
struct pll_div pll_div;
+ struct wm8804_priv *wm8804;

- ret = pll_factors(&pll_div, freq_out, freq_in);
+ wm8804 = snd_soc_codec_get_drvdata(codec);
+
+ ret = pll_factors(&pll_div, freq_out, freq_in,
+ wm8804->mclk_div);
if (ret)
return ret;

@@ -452,6 +458,7 @@ static int wm8804_set_clkdiv(struct snd_soc_dai *dai,
int div_id, int div)
{
struct snd_soc_codec *codec;
+ struct wm8804_priv *wm8804;

codec = dai->codec;
switch (div_id) {
@@ -459,6 +466,10 @@ static int wm8804_set_clkdiv(struct snd_soc_dai *dai,
snd_soc_update_bits(codec, WM8804_PLL5, 0x30,
(div & 0x3) << 4);
break;
+ case WM8804_MCLK_DIV:
+ wm8804 = snd_soc_codec_get_drvdata(codec);
+ wm8804->mclk_div = div;
+ break;
default:
dev_err(dai->dev, "Unknown clock divider: %d\n", div_id);
return -EINVAL;
diff --git a/sound/soc/codecs/wm8804.h b/sound/soc/codecs/wm8804.h
index 8ec14f5..e72d4f4 100644
--- a/sound/soc/codecs/wm8804.h
+++ b/sound/soc/codecs/wm8804.h
@@ -57,5 +57,9 @@
#define WM8804_CLKOUT_SRC_OSCCLK 4

#define WM8804_CLKOUT_DIV 1
+#define WM8804_MCLK_DIV 2
+
+#define WM8804_MCLKDIV_256FS 0
+#define WM8804_MCLKDIV_128FS 1

#endif /* _WM8804_H */
--
1.7.9.5


2014-01-17 00:55:05

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

On Tue, Jan 14, 2014 at 08:34:10PM +0100, Daniel Matuschek wrote:
> WM8804 can run with PLL frequencies of 256xfs and 128xfs for
> most sample rates. At 192kHz only 128xfs is supported. The
> existing driver selects 128xfs automatically for some lower

Charles (or someone else from Wolfson), you commented on previous
versions of this - are you still OK with it?


Attachments:
(No filename) (364.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-01-17 09:48:29

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

On Tue, Jan 14, 2014 at 08:34:10PM +0100, Daniel Matuschek wrote:
> WM8804 can run with PLL frequencies of 256xfs and 128xfs for
> most sample rates. At 192kHz only 128xfs is supported. The
> existing driver selects 128xfs automatically for some lower
> samples rates. By using an additional mclk_div divider, it
> is now possible to control the behaviour. This allows using
> 256xfs PLL frequency on all sample rates up to 96kHz. It
> should allow lower jitter and better signal quality. The
> behavior has to be controlled by the sound card driver,
> because some sample frequency share the same setting. e.g.
> 192kHz and 96kHz use 24.576MHz master clock. The only
> difference is the MCLK divider.
>
> Signed-off-by: Daniel Matuschek <[email protected]>

Acked-by: Charles Keepax <[email protected]>


Sorry about the slight delay travelling with limited internet at
the mo.

Thanks,
Charles

2014-01-17 10:53:29

by Dimitris Papastamos

[permalink] [raw]
Subject: RE: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

> Charles (or someone else from Wolfson), you commented on previous
> versions of this - are you still OK with it?

Looks good to me.

Privacy & Confidentiality Notice
-------------------------------------------------
This message and any attachments contain privileged and confidential information that is intended solely for the person(s) to whom it is addressed. If you are not an intended recipient you must not: read; copy; distribute; discuss; take any action in or make any reliance upon the contents of this message; nor open or read any attachment. If you have received this message in error, please notify us as soon as possible on the following telephone number and destroy this message including any attachments. Thank you.
-------------------------------------------------
Wolfson Microelectronics plc
Tel: +44 (0)131 272 7000
Fax: +44 (0)131 272 7001
Web: http://www.wolfsonmicro.com

Registered in Scotland

Company number SC089839

Registered office:

Westfield House, 26 Westfield Road, Edinburgh, EH11 2QB, UK

2014-01-17 12:22:35

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

On Tue, Jan 14, 2014 at 08:34:10PM +0100, Daniel Matuschek wrote:

> WM8804 can run with PLL frequencies of 256xfs and 128xfs for
> most sample rates. At 192kHz only 128xfs is supported. The
> existing driver selects 128xfs automatically for some lower

This patch doesn't apply against current code. Please check and resend,
from MAINTAINERS:

SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC)
M: Liam Girdwood <[email protected]>
M: Mark Brown <[email protected]>
T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
L: [email protected] (moderated for non-subscribers)
W: http://alsa-project.org/main/index.php/ASoC
S: Supported
F: Documentation/sound/alsa/soc/
F: sound/soc/
F: include/sound/soc*

specifically the for-next branch (or ideally topic/wm8804 for wm8804
when it exists).


Attachments:
(No filename) (873.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-01-17 16:43:20

by Florian Meier

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

I have tested your patch.
There is a (non blocking) error message regarding .idle_bias_off, but I
assume that should not have something to do with your patch. Can we just
set idle_bias_off to false here?

Otherwise, it looks good to me.

On 01/14/2014 08:34 PM, Daniel Matuschek wrote:
> WM8804 can run with PLL frequencies of 256xfs and 128xfs for
> most sample rates. At 192kHz only 128xfs is supported. The
> existing driver selects 128xfs automatically for some lower
> samples rates. By using an additional mclk_div divider, it
> is now possible to control the behaviour. This allows using
> 256xfs PLL frequency on all sample rates up to 96kHz. It
> should allow lower jitter and better signal quality. The
> behavior has to be controlled by the sound card driver,
> because some sample frequency share the same setting. e.g.
> 192kHz and 96kHz use 24.576MHz master clock. The only
> difference is the MCLK divider.
>
> Signed-off-by: Daniel Matuschek <[email protected]>
>
> ---
> sound/soc/codecs/wm8804.c | 17 ++++++++++++++---
> sound/soc/codecs/wm8804.h | 4 ++++
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/codecs/wm8804.c b/sound/soc/codecs/wm8804.c
> index 1704b1e..4619bf8 100644
> --- a/sound/soc/codecs/wm8804.c
> +++ b/sound/soc/codecs/wm8804.c
> @@ -63,6 +63,7 @@ struct wm8804_priv {
> struct regmap *regmap;
> struct regulator_bulk_data supplies[WM8804_NUM_SUPPLIES];
> struct notifier_block disable_nb[WM8804_NUM_SUPPLIES];
> + int mclk_div;
> };
>
> static int txsrc_get(struct snd_kcontrol *kcontrol,
> @@ -318,7 +319,7 @@ static struct {
>
> #define FIXED_PLL_SIZE ((1ULL << 22) * 10)
> static int pll_factors(struct pll_div *pll_div, unsigned int target,
> - unsigned int source)
> + unsigned int source, unsigned int mclk_div)
> {
> u64 Kpart;
> unsigned long int K, Ndiv, Nmod, tmp;
> @@ -330,7 +331,8 @@ static int pll_factors(struct pll_div *pll_div,
> unsigned int target,
> */
> for (i = 0; i < ARRAY_SIZE(post_table); i++) {
> tmp = target * post_table[i].div;
> - if (tmp >= 90000000 && tmp <= 100000000) {
> + if ((tmp >= 90000000 && tmp <= 100000000) &&
> + (mclk_div == post_table[i].mclkdiv)) {
> pll_div->freqmode = post_table[i].freqmode;
> pll_div->mclkdiv = post_table[i].mclkdiv;
> target *= post_table[i].div;
> @@ -387,8 +389,12 @@ static int wm8804_set_pll(struct snd_soc_dai *dai,
> int pll_id,
> } else {
> int ret;
> struct pll_div pll_div;
> + struct wm8804_priv *wm8804;
>
> - ret = pll_factors(&pll_div, freq_out, freq_in);
> + wm8804 = snd_soc_codec_get_drvdata(codec);
> +
> + ret = pll_factors(&pll_div, freq_out, freq_in,
> + wm8804->mclk_div);
> if (ret)
> return ret;
>
> @@ -452,6 +458,7 @@ static int wm8804_set_clkdiv(struct snd_soc_dai *dai,
> int div_id, int div)
> {
> struct snd_soc_codec *codec;
> + struct wm8804_priv *wm8804;
>
> codec = dai->codec;
> switch (div_id) {
> @@ -459,6 +466,10 @@ static int wm8804_set_clkdiv(struct snd_soc_dai *dai,
> snd_soc_update_bits(codec, WM8804_PLL5, 0x30,
> (div & 0x3) << 4);
> break;
> + case WM8804_MCLK_DIV:
> + wm8804 = snd_soc_codec_get_drvdata(codec);
> + wm8804->mclk_div = div;
> + break;
> default:
> dev_err(dai->dev, "Unknown clock divider: %d\n", div_id);
> return -EINVAL;
> diff --git a/sound/soc/codecs/wm8804.h b/sound/soc/codecs/wm8804.h
> index 8ec14f5..e72d4f4 100644
> --- a/sound/soc/codecs/wm8804.h
> +++ b/sound/soc/codecs/wm8804.h
> @@ -57,5 +57,9 @@
> #define WM8804_CLKOUT_SRC_OSCCLK 4
>
> #define WM8804_CLKOUT_DIV 1
> +#define WM8804_MCLK_DIV 2
> +
> +#define WM8804_MCLKDIV_256FS 0
> +#define WM8804_MCLKDIV_128FS 1
>
> #endif /* _WM8804_H */

2014-01-17 17:59:49

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

On Fri, Jan 17, 2014 at 05:43:14PM +0100, Florian Meier wrote:
> I have tested your patch.
> There is a (non blocking) error message regarding .idle_bias_off, but I
> assume that should not have something to do with your patch. Can we just
> set idle_bias_off to false here?

What is the error message? I remember that the first version of this
had an undocumented change to idle_bias_off in it, I expect that needed
to split out into a separate patch rather than dropped.


Attachments:
(No filename) (474.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-01-17 18:26:22

by Daniel Matuschek

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

The idle_bias_off should not be part of this patch. I will check this again.

Am 17.01.2014 um 18:59 schrieb Mark Brown <[email protected]>:

> On Fri, Jan 17, 2014 at 05:43:14PM +0100, Florian Meier wrote:
>> I have tested your patch.
>> There is a (non blocking) error message regarding .idle_bias_off, but I
>> assume that should not have something to do with your patch. Can we just
>> set idle_bias_off to false here?
>
> What is the error message? I remember that the first version of this
> had an undocumented change to idle_bias_off in it, I expect that needed
> to split out into a separate patch rather than dropped.

2014-01-17 18:44:09

by Florian Meier

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

On 01/17/2014 07:33 PM, Mark Brown wrote:
> On Fri, Jan 17, 2014 at 07:06:24PM +0100, Florian Meier wrote:
>>
> Intentionally off-list?

Oh no - I am sorry!

>> If I remember correctly the error was
>> "codec can not start from non-off bias with idle_bias_off==true"
>
>> I think the solution is just to set idle_bias_off = false and everything
>> seems to be working with that. I just don't know if there might be any
>> side effects.
>
> Setting it to false increases power consumption since the device is
> kept more powered on when idle but reduces startup time from idle. For
> digital only devices like the wm8804 there shouldn't be any reason to
> keep it powered up when not in use, the startup time is generally
> negligable anyway.
>

So a better solution would be to set SND_SOC_BIAS_OFF instead of
SND_SOC_BIAS_STANDBY at the end of probe?

2014-01-17 18:47:22

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

On Fri, Jan 17, 2014 at 07:44:02PM +0100, Florian Meier wrote:
> On 01/17/2014 07:33 PM, Mark Brown wrote:

> > Setting it to false increases power consumption since the device is
> > kept more powered on when idle but reduces startup time from idle. For
> > digital only devices like the wm8804 there shouldn't be any reason to
> > keep it powered up when not in use, the startup time is generally
> > negligable anyway.

> So a better solution would be to set SND_SOC_BIAS_OFF instead of
> SND_SOC_BIAS_STANDBY at the end of probe?

Yes (making sure that it is actually in that state).


Attachments:
(No filename) (589.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-01-20 20:03:42

by Daniel Matuschek

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

I'm sorry for this. Looks like I still had problems with some whitespaces. I will resend the patch and hope, it will work now.

Daniel

Am 17.01.2014 um 13:22 schrieb Mark Brown <[email protected]>:

> On Tue, Jan 14, 2014 at 08:34:10PM +0100, Daniel Matuschek wrote:
>
>> WM8804 can run with PLL frequencies of 256xfs and 128xfs for
>> most sample rates. At 192kHz only 128xfs is supported. The
>> existing driver selects 128xfs automatically for some lower
>
> This patch doesn't apply against current code. Please check and resend,
> from MAINTAINERS:
>
> SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC)
> M: Liam Girdwood <[email protected]>
> M: Mark Brown <[email protected]>
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
> L: [email protected] (moderated for non-subscribers)
> W: http://alsa-project.org/main/index.php/ASoC
> S: Supported
> F: Documentation/sound/alsa/soc/
> F: sound/soc/
> F: include/sound/soc*
>
> specifically the for-next branch (or ideally topic/wm8804 for wm8804
> when it exists).
> _______________________________________________
> Alsa-devel mailing list
> [email protected]
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

2014-01-22 11:58:27

by Ben Dooks

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH] ASoC: wm8804: Allow control of master clock divider in PLL generation

On Fri, Jan 17, 2014 at 10:35:10AM +0000, Dimitris Papastamos wrote:
> > Charles (or someone else from Wolfson), you commented on previous
> > versions of this - are you still OK with it?
>
> Looks good to me.
>
> Privacy & Confidentiality Notice
> -------------------------------------------------
> This message and any attachments contain privileged and confidential information that is intended solely for the person(s) to whom it is addressed. If you are not an intended recipient you must not: read; copy; distribute; discuss; take any action in or make any reliance upon the contents of this message; nor open or read any attachment. If you have received this message in error, please notify us as soon as possible on the following telephone number and destroy this message including any attachments. Thank you.

Shouldn't be posting them to public lists then.

--
Ben Dooks, [email protected], http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.