All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function
@ 2018-05-23  6:13 mac.chiang
  2018-05-24  9:45 ` Adam Thomson
  2018-05-25  7:47 ` mac.chiang
  0 siblings, 2 replies; 7+ messages in thread
From: mac.chiang @ 2018-05-23  6:13 UTC (permalink / raw)
  To: alsa-devel

From: Mac Chiang <mac.chiang@intel.com>

unexpected error: PLL input clock 0 below valid range

backport from broonie's tree
commit fd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config to codec_init function")

The MCLK for DA7219 does not change in this platform, but is
currently being configured everytime as part of the platform_clock
event handler for DAPM. The upshot of this is that we have
unnecessary calls to this function, and it also means that if
a stream hasn't yet been started, DA7219 driver does not have the
correct MCLK rates programmed and so the HP detection feature does
not operate as expected.

This patch rectifies this issue by moving the sysclk call to
codec_init function so it's only called once at initialisation.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
Singed-off-by: Mac Chiang <mac.chiang@intel.com>
---
 sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c
index 60e739f..88fbc6a 100644
--- a/sound/soc/intel/boards/kbl_da7219_max98357a.c
+++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c
@@ -65,14 +65,6 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 		return -EIO;
 	}
 
-	/* Configure sysclk for codec */
-	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
-				     SND_SOC_CLOCK_IN);
-	if (ret) {
-		dev_err(card->dev, "can't set codec sysclk configuration\n");
-		return ret;
-	}
-
 	if (SND_SOC_DAPM_EVENT_OFF(event)) {
 		ret = snd_soc_dai_set_pll(codec_dai, 0,
 				     DA7219_SYSCLK_MCLK, 0, 0);
@@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
 	struct snd_soc_component *component = rtd->codec_dai->component;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	struct snd_soc_jack *jack;
 	int ret;
 
+	/* Configure sysclk for codec */
+	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000,
+						SND_SOC_CLOCK_IN);
+	if (ret) {
+		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
+		return ret;
+	}
+
 	/*
 	 * Headset buttons map to the google Reference headset.
 	 * These can be configured by userspace.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function
  2018-05-23  6:13 [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function mac.chiang
@ 2018-05-24  9:45 ` Adam Thomson
  2018-05-24 20:26   ` Pierre-Louis Bossart
  2018-05-25  7:47 ` mac.chiang
  1 sibling, 1 reply; 7+ messages in thread
From: Adam Thomson @ 2018-05-24  9:45 UTC (permalink / raw)
  To: mac.chiang, alsa-devel

On 23 May 2018 07:14, Mac Chiang wrote:

> From: Mac Chiang <mac.chiang@intel.com>
> 
> unexpected error: PLL input clock 0 below valid range
> 
> backport from broonie's tree
> commit fd0f237572ad
> ("ASoC: Intel: bxt: Move codec sysclk config to codec_init function")
> 
> The MCLK for DA7219 does not change in this platform, but is
> currently being configured everytime as part of the platform_clock
> event handler for DAPM. The upshot of this is that we have
> unnecessary calls to this function, and it also means that if
> a stream hasn't yet been started, DA7219 driver does not have the
> correct MCLK rates programmed and so the HP detection feature does
> not operate as expected.
> 
> This patch rectifies this issue by moving the sysclk call to
> codec_init function so it's only called once at initialisation.
> 
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
> Singed-off-by: Mac Chiang <mac.chiang@intel.com>
> ---
>  sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c
> b/sound/soc/intel/boards/kbl_da7219_max98357a.c
> index 60e739f..88fbc6a 100644
> --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c
> +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c
> @@ -65,14 +65,6 @@ static int platform_clock_control(struct
> snd_soc_dapm_widget *w,
>  		return -EIO;
>  	}
> 
> -	/* Configure sysclk for codec */
> -	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
> -				     SND_SOC_CLOCK_IN);

Code here is setting the MCLK rate for the codec as being 24.576MHz but...

> -	if (ret) {
> -		dev_err(card->dev, "can't set codec sysclk configuration\n");
> -		return ret;
> -	}
> -
>  	if (SND_SOC_DAPM_EVENT_OFF(event)) {
>  		ret = snd_soc_dai_set_pll(codec_dai, 0,
>  				     DA7219_SYSCLK_MCLK, 0, 0);
> @@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct
> snd_soc_pcm_runtime *rtd)
>  {
>  	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>  	struct snd_soc_component *component = rtd->codec_dai->component;
> +	struct snd_soc_dai *codec_dai = rtd->codec_dai;
>  	struct snd_soc_jack *jack;
>  	int ret;
> 
> +	/* Configure sysclk for codec */
> +	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000,
> +						SND_SOC_CLOCK_IN);

...here you're setting MCLK rate to be 19.2MHz. Is that correct?

> +	if (ret) {
> +		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
> +		return ret;
> +	}
> +
>  	/*
>  	 * Headset buttons map to the google Reference headset.
>  	 * These can be configured by userspace.
> --
> 2.7.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function
  2018-05-24  9:45 ` Adam Thomson
@ 2018-05-24 20:26   ` Pierre-Louis Bossart
  0 siblings, 0 replies; 7+ messages in thread
From: Pierre-Louis Bossart @ 2018-05-24 20:26 UTC (permalink / raw)
  To: Adam Thomson, mac.chiang, alsa-devel



On 05/24/2018 04:45 AM, Adam Thomson wrote:
> On 23 May 2018 07:14, Mac Chiang wrote:
>
>> From: Mac Chiang <mac.chiang@intel.com>
>>
>> unexpected error: PLL input clock 0 below valid range
>>
>> backport from broonie's tree
>> commit fd0f237572ad
>> ("ASoC: Intel: bxt: Move codec sysclk config to codec_init function")
>>
>> The MCLK for DA7219 does not change in this platform, but is
>> currently being configured everytime as part of the platform_clock
>> event handler for DAPM. The upshot of this is that we have
>> unnecessary calls to this function, and it also means that if
>> a stream hasn't yet been started, DA7219 driver does not have the
>> correct MCLK rates programmed and so the HP detection feature does
>> not operate as expected.
>>
>> This patch rectifies this issue by moving the sysclk call to
>> codec_init function so it's only called once at initialisation.
>>
>> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
>> Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
>> Singed-off-by: Mac Chiang <mac.chiang@intel.com>
Mac, I provided you internal feedback that your commit message was 
misleading.
This is not a 'backport', the intent here is to fix a problem on KBL 
using the same solution already merged for ApolloLake with commit 
fd0f237572ad ("ASoC: Intel: bxt: Move codec sysclk config to codec_init 
function"). You also want to be careful when including someone else's 
Signed-off-by and check for typos, if you run checkpatch.pl --strict 
you'd see this:

WARNING: 'everytime' may be misspelled - perhaps 'every time'?
#14:
currently being configured everytime as part of the platform_clock

WARNING: Non-standard signature: Singed-off-by:
#26:
Singed-off-by: Mac Chiang <mac.chiang@intel.com>

CHECK: Alignment should match open parenthesis
#57: FILE: sound/soc/intel/boards/kbl_da7219_max98357a.c:170:
+    ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000,
+                        SND_SOC_CLOCK_IN);

total: 0 errors, 2 warnings, 1 checks, 32 lines checked

In addition, both Adam and I mentioned that the clocks are different 
between the two platforms, so this patch is incorrect and cannot be 
applied as is, see below.

>> ---
>>   sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> b/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> index 60e739f..88fbc6a 100644
>> --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> @@ -65,14 +65,6 @@ static int platform_clock_control(struct
>> snd_soc_dapm_widget *w,
>>   		return -EIO;
>>   	}
>>
>> -	/* Configure sysclk for codec */
>> -	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
>> -				     SND_SOC_CLOCK_IN);
> Code here is setting the MCLK rate for the codec as being 24.576MHz but...
>
>> -	if (ret) {
>> -		dev_err(card->dev, "can't set codec sysclk configuration\n");
>> -		return ret;
>> -	}
>> -
>>   	if (SND_SOC_DAPM_EVENT_OFF(event)) {
>>   		ret = snd_soc_dai_set_pll(codec_dai, 0,
>>   				     DA7219_SYSCLK_MCLK, 0, 0);
>> @@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct
>> snd_soc_pcm_runtime *rtd)
>>   {
>>   	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>   	struct snd_soc_component *component = rtd->codec_dai->component;
>> +	struct snd_soc_dai *codec_dai = rtd->codec_dai;
>>   	struct snd_soc_jack *jack;
>>   	int ret;
>>
>> +	/* Configure sysclk for codec */
>> +	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 19200000,
>> +						SND_SOC_CLOCK_IN);
> ...here you're setting MCLK rate to be 19.2MHz. Is that correct?
That is clearly wrong. Likely a copy/paste from bxt_da7219_max98357a.c 
instead of moving the code within the same file.

>
>> +	if (ret) {
>> +		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
>> +		return ret;
>> +	}
>> +
>>   	/*
>>   	 * Headset buttons map to the google Reference headset.
>>   	 * These can be configured by userspace.
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function
  2018-05-23  6:13 [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function mac.chiang
  2018-05-24  9:45 ` Adam Thomson
@ 2018-05-25  7:47 ` mac.chiang
  2018-05-25  8:17   ` Adam Thomson
  2018-06-01 11:05   ` Applied "ASoC: Intel: kbl: Move codec sysclk config to codec_init function" to the asoc tree Mark Brown
  1 sibling, 2 replies; 7+ messages in thread
From: mac.chiang @ 2018-05-25  7:47 UTC (permalink / raw)
  To: alsa-devel

From: Mac Chiang <mac.chiang@intel.com>

fix an issue on KBL which applied the merged solution from
broonie's tree
commit fd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config to codec_init function")

The MCLK for DA7219 does not change in this platform, but is
currently being configured everytime as part of the platform_clock
event handler for DAPM. The upshot of this is that we have
unnecessary calls to this function, and it also means that if
a stream hasn't yet been started, DA7219 driver does not have the
correct MCLK rates programmed and so the HP detection feature does
not operate as expected.

This patch rectifies this issue by moving the sysclk call to
codec_init function so it's only called once at initialisation.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Mac Chiang <mac.chiang@intel.com>
---
 sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c
index c332a32..ad3029d 100644
--- a/sound/soc/intel/boards/kbl_da7219_max98357a.c
+++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c
@@ -66,14 +66,6 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 		return -EIO;
 	}
 
-	/* Configure sysclk for codec */
-	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
-				     SND_SOC_CLOCK_IN);
-	if (ret) {
-		dev_err(card->dev, "can't set codec sysclk configuration\n");
-		return ret;
-	}
-
 	if (SND_SOC_DAPM_EVENT_OFF(event)) {
 		ret = snd_soc_dai_set_pll(codec_dai, 0,
 				     DA7219_SYSCLK_MCLK, 0, 0);
@@ -170,9 +162,18 @@ static int kabylake_da7219_codec_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
 	struct snd_soc_component *component = rtd->codec_dai->component;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	struct snd_soc_jack *jack;
 	int ret;
 
+	/* Configure sysclk for codec */
+	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
+						SND_SOC_CLOCK_IN);
+	if (ret) {
+		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
+		return ret;
+	}
+
 	/*
 	 * Headset buttons map to the google Reference headset.
 	 * These can be configured by userspace.
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function
  2018-05-25  7:47 ` mac.chiang
@ 2018-05-25  8:17   ` Adam Thomson
  2018-05-25 15:20     ` Pierre-Louis Bossart
  2018-06-01 11:05   ` Applied "ASoC: Intel: kbl: Move codec sysclk config to codec_init function" to the asoc tree Mark Brown
  1 sibling, 1 reply; 7+ messages in thread
From: Adam Thomson @ 2018-05-25  8:17 UTC (permalink / raw)
  To: mac.chiang, alsa-devel

On 25 May 2018 08:47, Mac Chiang wrote:

You really need to run 'getmaintainers.pl' on this patch. If you want a
maintainer to see your change, the e-mail has to be directly addressed to them
as well as CC'd to the mailing list, otherwise chances are they won't see it.

Also, as this is the second iteration of the patch, the title should be
versioned, i.e. "[PATCH v2] ..."

> From: Mac Chiang <mac.chiang@intel.com>
> 
> fix an issue on KBL which applied the merged solution from
> broonie's tree
> commit fd0f237572ad
> ("ASoC: Intel: bxt: Move codec sysclk config to codec_init function")
> 
> The MCLK for DA7219 does not change in this platform, but is
> currently being configured everytime as part of the platform_clock
> event handler for DAPM. The upshot of this is that we have
> unnecessary calls to this function, and it also means that if
> a stream hasn't yet been started, DA7219 driver does not have the
> correct MCLK rates programmed and so the HP detection feature does
> not operate as expected.
> 
> This patch rectifies this issue by moving the sysclk call to
> codec_init function so it's only called once at initialisation.
> 
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Mac Chiang <mac.chiang@intel.com>

As Pierre mentioned yesterday, this is not an existing patch being reapplied, so
you cannot assume people's 'Signed-off-by' on this. Mark certainly won't add his
'Signed-off-by' tag until he merges it into his tree. Also, has Sathya actually
seen this actual change? If not how can you add his 'Acked-by' tag?

If you can make this patch format in-line with normal expectations, then the
code below looks ok to me.

> ---
>  sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c
> b/sound/soc/intel/boards/kbl_da7219_max98357a.c
> index c332a32..ad3029d 100644
> --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c
> +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c
> @@ -66,14 +66,6 @@ static int platform_clock_control(struct
> snd_soc_dapm_widget *w,
>  		return -EIO;
>  	}
> 
> -	/* Configure sysclk for codec */
> -	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
> -				     SND_SOC_CLOCK_IN);
> -	if (ret) {
> -		dev_err(card->dev, "can't set codec sysclk configuration\n");
> -		return ret;
> -	}
> -
>  	if (SND_SOC_DAPM_EVENT_OFF(event)) {
>  		ret = snd_soc_dai_set_pll(codec_dai, 0,
>  				     DA7219_SYSCLK_MCLK, 0, 0);
> @@ -170,9 +162,18 @@ static int kabylake_da7219_codec_init(struct
> snd_soc_pcm_runtime *rtd)
>  {
>  	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>  	struct snd_soc_component *component = rtd->codec_dai->component;
> +	struct snd_soc_dai *codec_dai = rtd->codec_dai;
>  	struct snd_soc_jack *jack;
>  	int ret;
> 
> +	/* Configure sysclk for codec */
> +	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
> +						SND_SOC_CLOCK_IN);
> +	if (ret) {
> +		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
> +		return ret;
> +	}
> +
>  	/*
>  	 * Headset buttons map to the google Reference headset.
>  	 * These can be configured by userspace.
> --
> 2.7.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function
  2018-05-25  8:17   ` Adam Thomson
@ 2018-05-25 15:20     ` Pierre-Louis Bossart
  0 siblings, 0 replies; 7+ messages in thread
From: Pierre-Louis Bossart @ 2018-05-25 15:20 UTC (permalink / raw)
  To: Adam Thomson, mac.chiang, alsa-devel



On 05/25/2018 03:17 AM, Adam Thomson wrote:
> On 25 May 2018 08:47, Mac Chiang wrote:
>
> You really need to run 'getmaintainers.pl' on this patch. If you want a
> maintainer to see your change, the e-mail has to be directly addressed to them
> as well as CC'd to the mailing list, otherwise chances are they won't see it.
>
> Also, as this is the second iteration of the patch, the title should be
> versioned, i.e. "[PATCH v2] ..."
>
>> From: Mac Chiang <mac.chiang@intel.com>
>>
>> fix an issue on KBL which applied the merged solution from
>> broonie's tree
>> commit fd0f237572ad
>> ("ASoC: Intel: bxt: Move codec sysclk config to codec_init function")
This is confusing cause and effect. I suggest you start with the 
description of the APL fix:

'On APL, commit fd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config to codec_init function") 
fixed an issue related to jack detection.'

then keep the following working as is
>>
>> The MCLK for DA7219 does not change in this platform, but is
>> currently being configured everytime as part of the platform_clock
>> event handler for DAPM. The upshot of this is that we have
>> unnecessary calls to this function, and it also means that if
>> a stream hasn't yet been started, DA7219 driver does not have the
>> correct MCLK rates programmed and so the HP detection feature does
>> not operate as expected.
add 'The same fix is needed on KBL.'
>>
>> This patch rectifies this issue by moving the sysclk call to
>> codec_init function so it's only called once at initialisation.
>>
>> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
>> Acked-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
>> Signed-off-by: Mark Brown <broonie@kernel.org>
And remove all previous 3 lines. You can only add someone else's tag if 
they have explicitly provided it to you or sent it to the mailing list. 
copy-paste is not acceptable.
>> Signed-off-by: Mac Chiang <mac.chiang@intel.com>
> As Pierre mentioned yesterday, this is not an existing patch being reapplied, so
> you cannot assume people's 'Signed-off-by' on this. Mark certainly won't add his
> 'Signed-off-by' tag until he merges it into his tree. Also, has Sathya actually
> seen this actual change? If not how can you add his 'Acked-by' tag?
>
> If you can make this patch format in-line with normal expectations, then the
> code below looks ok to me.
same here, the code makes sense. I will ack this patch once the commit 
message and tags are fixed. Please submit a v3 and read [1] before doing so.

[1] https://www.kernel.org/doc/html/v4.16/process/submitting-patches.html
>
>> ---
>>   sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> b/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> index c332a32..ad3029d 100644
>> --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c
>> @@ -66,14 +66,6 @@ static int platform_clock_control(struct
>> snd_soc_dapm_widget *w,
>>   		return -EIO;
>>   	}
>>
>> -	/* Configure sysclk for codec */
>> -	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
>> -				     SND_SOC_CLOCK_IN);
>> -	if (ret) {
>> -		dev_err(card->dev, "can't set codec sysclk configuration\n");
>> -		return ret;
>> -	}
>> -
>>   	if (SND_SOC_DAPM_EVENT_OFF(event)) {
>>   		ret = snd_soc_dai_set_pll(codec_dai, 0,
>>   				     DA7219_SYSCLK_MCLK, 0, 0);
>> @@ -170,9 +162,18 @@ static int kabylake_da7219_codec_init(struct
>> snd_soc_pcm_runtime *rtd)
>>   {
>>   	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>   	struct snd_soc_component *component = rtd->codec_dai->component;
>> +	struct snd_soc_dai *codec_dai = rtd->codec_dai;
>>   	struct snd_soc_jack *jack;
>>   	int ret;
>>
>> +	/* Configure sysclk for codec */
>> +	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
>> +						SND_SOC_CLOCK_IN);
>> +	if (ret) {
>> +		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
>> +		return ret;
>> +	}
>> +
>>   	/*
>>   	 * Headset buttons map to the google Reference headset.
>>   	 * These can be configured by userspace.
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Applied "ASoC: Intel: kbl: Move codec sysclk config to codec_init function" to the asoc tree
  2018-05-25  7:47 ` mac.chiang
  2018-05-25  8:17   ` Adam Thomson
@ 2018-06-01 11:05   ` Mark Brown
  1 sibling, 0 replies; 7+ messages in thread
From: Mark Brown @ 2018-06-01 11:05 UTC (permalink / raw)
  To: Mac Chiang; +Cc: alsa-devel, Mark Brown, Pierre-Louis Bossart

The patch

   ASoC: Intel: kbl: Move codec sysclk config to codec_init function

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 56c3a95385df558a471f7fcedba73c9ed836a906 Mon Sep 17 00:00:00 2001
From: Mac Chiang <mac.chiang@intel.com>
Date: Fri, 1 Jun 2018 01:18:32 +0800
Subject: [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init
 function

On APL, commit fd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config to codec_init function")
fixed an issue related to jack detection.

The MCLK for DA7219 does not change in this platform, but is
currently being configured everytime as part of the platform_clock
event handler for DAPM. The upshot of this is that we have
unnecessary calls to this function, and it also means that if
a stream hasn't yet been started, DA7219 driver does not have the
correct MCLK rates programmed and so the HP detection feature does
not operate as expected.

The same fix is needed on KBL.

This patch rectifies this issue by moving the sysclk call to
codec_init function so it's only called once at initialisation.

Signed-off-by: Mac Chiang <mac.chiang@intel.com>
Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c
index 60e739f3d6f3..94294c27d1db 100644
--- a/sound/soc/intel/boards/kbl_da7219_max98357a.c
+++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c
@@ -65,14 +65,6 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 		return -EIO;
 	}
 
-	/* Configure sysclk for codec */
-	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
-				     SND_SOC_CLOCK_IN);
-	if (ret) {
-		dev_err(card->dev, "can't set codec sysclk configuration\n");
-		return ret;
-	}
-
 	if (SND_SOC_DAPM_EVENT_OFF(event)) {
 		ret = snd_soc_dai_set_pll(codec_dai, 0,
 				     DA7219_SYSCLK_MCLK, 0, 0);
@@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
 	struct snd_soc_component *component = rtd->codec_dai->component;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	struct snd_soc_jack *jack;
 	int ret;
 
+	/* Configure sysclk for codec */
+	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK, 24576000,
+						SND_SOC_CLOCK_IN);
+	if (ret) {
+		dev_err(rtd->dev, "can't set codec sysclk configuration\n");
+		return ret;
+	}
+
 	/*
 	 * Headset buttons map to the google Reference headset.
 	 * These can be configured by userspace.
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-06-01 11:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23  6:13 [PATCH] ASoC: Intel: kbl: Move codec sysclk config to codec_init function mac.chiang
2018-05-24  9:45 ` Adam Thomson
2018-05-24 20:26   ` Pierre-Louis Bossart
2018-05-25  7:47 ` mac.chiang
2018-05-25  8:17   ` Adam Thomson
2018-05-25 15:20     ` Pierre-Louis Bossart
2018-06-01 11:05   ` Applied "ASoC: Intel: kbl: Move codec sysclk config to codec_init function" to the asoc tree Mark Brown

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.