[RFC PATCH 0/2] ASoC: wm8994: master clock issues

[RFC PATCH 0/2] ASoC: wm8994: master clock issues

[olivier moysan]

Hello,

I face two issues using wm8994 codec driver.
Below is a description of the use cases rising the problem.
I added the patches which allow to fix these issues.
These minimalist patches can be seen as a starting point for a discussion on the best
way to handle the use cases exposed.


- ASoC: wm8994: default master clock selection

Use case:
CPU DAI and codec are managed through simple card.
Wolson codec wm8994 is set as slave of CPU DAI and CPU DAI feeds codec with master clock.
Master clock is derived from mclk-fs property and provided to CPU DAI and codec 
through snd_soc_dai_set_sysclk API. 

Analysis:
The simple card assumes id 0 for master clock.
However wm8994 codec exposes master clock ids from 1 to 4. (MCLK1, MCLK2 ..)

The proposal in this patch is to define a default selection regarding master clock
when id 0 is provided.


- ASoC: wm8994: delay aifxclk activation

Use case:
Wolson codec wm8994 is set as slave of CPU DAI and CPU DAI feeds codec with master clock.
CPU DAI generates master clock when enabled in trigger callback.
So master clock is available only at runtime. 
Master clock is not available when configuring codec through alsa controls.

Analysis:
When we configure aifx clock while master clock is not active, related registers are not updated in wm8894 codec.
A way to go around this problem, is to delay aifxclk activation until SND_SOC_DAPM_POST_PMU DAPM stage.
The purpose of patch here is to illustrate this workaround. 

What I cannot clearly figure out, is  how this may impact other uses of wm8894 codec.
I try to know if it's worth investigating further in this direction or if it will not be applicable.
If not applicable, what maybe the other options ?

regards
olivier


olivier moysan (2):
  ASoC: wm8994: default master clock selection
  ASoC: wm8994: delay aifxclk activation

 sound/soc/codecs/wm8994.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
1.9.1

[RFC PATCH 1/2] ASoC: wm8994: default master clock selection

[olivier moysan]

This patch allows the wm8994 codec driver to be used
with simple card when mclk fs is provided in dai link.

Signed-off-by: olivier moysan <olivier.moysan@st.com>
---
 sound/soc/codecs/wm8994.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index 3896523..ccf640a 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -2389,6 +2389,7 @@ static int wm8994_set_dai_sysclk(struct snd_soc_dai *dai,
 
 	switch (clk_id) {
 	case WM8994_SYSCLK_MCLK1:
+	default:
 		wm8994->sysclk[dai->id - 1] = WM8994_SYSCLK_MCLK1;
 		wm8994->mclk[0] = freq;
 		dev_dbg(dai->dev, "AIF%d using MCLK1 at %uHz\n",
@@ -2431,9 +2432,6 @@ static int wm8994_set_dai_sysclk(struct snd_soc_dai *dai,
 			snd_soc_update_bits(codec, WM8994_POWER_MANAGEMENT_2,
 					    WM8994_OPCLK_ENA, 0);
 		}
-
-	default:
-		return -EINVAL;
 	}
 
 	configure_clock(codec);
-- 
1.9.1

[RFC PATCH 2/2] ASoC: wm8994: delay aifxclk activation

[olivier moysan]

Signed-off-by: olivier moysan <olivier.moysan@st.com>
---
 sound/soc/codecs/wm8994.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index ccf640a..6782d9e 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -1262,7 +1262,7 @@ static int late_enable_ev(struct snd_soc_dapm_widget *w,
 	struct wm8994_priv *wm8994 = snd_soc_codec_get_drvdata(codec);
 
 	switch (event) {
-	case SND_SOC_DAPM_PRE_PMU:
+	case SND_SOC_DAPM_POST_PMU:
 		if (wm8994->aif1clk_enable) {
 			aif1clk_ev(w, kcontrol, SND_SOC_DAPM_PRE_PMU);
 			snd_soc_update_bits(codec, WM8994_AIF1_CLOCKING_1,
@@ -1615,6 +1615,8 @@ static SOC_ENUM_SINGLE_DECL(aif2dacr_src_enum,
 SND_SOC_DAPM_MUX_E("Right Headphone Mux", SND_SOC_NOPM, 0, 0, &wm_hubs_hpr_mux,
 		   late_enable_ev, SND_SOC_DAPM_PRE_PMU),
 
+SND_SOC_DAPM_POST("Late Enable PGA", late_enable_ev),
+
 SND_SOC_DAPM_POST("Late Disable PGA", late_disable_ev)
 };
 
-- 
1.9.1

Re: [RFC PATCH 2/2] ASoC: wm8994: delay aifxclk activation

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 416 bytes --]

On Thu, Mar 16, 2017 at 05:41:23PM +0100, olivier moysan wrote:
> Signed-off-by: olivier moysan <olivier.moysan@st.com>
> ---
>  sound/soc/codecs/wm8994.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

...with no changelog.  That's especially important here, we really don't
want to be messing with the clock activation ordering on this device
without a good and clear understanding of what's going on.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC PATCH 2/2] ASoC: wm8994: delay aifxclk activation

[Olivier MOYSAN]

Hello Mark,

On 03/16/2017 06:51 PM, Mark Brown wrote:
> On Thu, Mar 16, 2017 at 05:41:23PM +0100, olivier moysan wrote:
>> Signed-off-by: olivier moysan <olivier.moysan@st.com>
>> ---
>>  sound/soc/codecs/wm8994.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> ...with no changelog.  That's especially important here, we really don't
> want to be messing with the clock activation ordering on this device
> without a good and clear understanding of what's going on.
>

Sorry. I put it in cover letter only.
I send a v2 with more explanations.

regards
olivier

Re: [RFC PATCH 0/2] ASoC: wm8994: master clock issues

[Charles Keepax]

On Thu, Mar 16, 2017 at 05:41:21PM +0100, olivier moysan wrote:
> Hello,
> 
> I face two issues using wm8994 codec driver.
> Below is a description of the use cases rising the problem.
> I added the patches which allow to fix these issues.
> These minimalist patches can be seen as a starting point for a discussion on the best
> way to handle the use cases exposed.
> 
> 
> - ASoC: wm8994: default master clock selection
> 
> Use case:
> CPU DAI and codec are managed through simple card.
> Wolson codec wm8994 is set as slave of CPU DAI and CPU DAI feeds codec with master clock.
> Master clock is derived from mclk-fs property and provided to CPU DAI and codec 
> through snd_soc_dai_set_sysclk API. 
> 
> Analysis:
> The simple card assumes id 0 for master clock.
> However wm8994 codec exposes master clock ids from 1 to 4. (MCLK1, MCLK2 ..)
> 
> The proposal in this patch is to define a default selection regarding master clock
> when id 0 is provided.
> 
> 
> - ASoC: wm8994: delay aifxclk activation
> 
> Use case:
> Wolson codec wm8994 is set as slave of CPU DAI and CPU DAI feeds codec with master clock.
> CPU DAI generates master clock when enabled in trigger callback.
> So master clock is available only at runtime. 
> Master clock is not available when configuring codec through alsa controls.
> 
> Analysis:
> When we configure aifx clock while master clock is not active, related registers are not updated in wm8894 codec.
> A way to go around this problem, is to delay aifxclk activation until SND_SOC_DAPM_POST_PMU DAPM stage.
> The purpose of patch here is to illustrate this workaround. 
> 
> What I cannot clearly figure out, is  how this may impact other uses of wm8894 codec.
> I try to know if it's worth investigating further in this direction or if it will not be applicable.
> If not applicable, what maybe the other options ?
> 
> regards
> olivier

Would be good to CC patches@opensource.wolfsonmicro.com on the
next version of these patches as well, higher chance someone here
will notice and have a look over them.

Thanks,
Charles