ASoC: Intel: kbl: Move codec sysclk config to codec_init function
Subsystems (3)
PATCH 1/1 ASoC: Intel: kbl: Move codec sysclk config to codec_init function
From: Mac Chiang <[email protected]>
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.
--- sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
sound/soc/intel/boards/kbl_da7219_max98357a.c +9 -8
@@ -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.
Signature · 1 line
2.7.4
Quoted 32 lines · Mac Chiang
From: Mac Chiang <[email protected]>
unexpected error: PLL input clock 0 below valid range
backport from broonie's tree
commitfd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config tocodec_initfunction")The MCLK for DA7219 does not change in this platform, but is currently being configured everytime as part of the
platform_clockevent 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_initfunction so it's only called once at initialisation.Signed-off-by Adam Thomson <[email protected]>Singed-off-by Mac Chiang <[email protected]>--- sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-0
sound/soc/intel/boards/kbl_da7219_max98357a.c static int platform_clock_control(struct · line 71
Quoted 7 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-3@@ -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...
Quoted 10 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-5-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,sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-0DA7219_SYSCLK_MCLK, 0, 0);@@ -169,9 +161,18 @@ static int kabylake_da7219_codec_init(struct snd_soc_pcm_runtime *rtd)
sound/soc/intel/boards/kbl_da7219_max98357a.c static int kabylake_da7219_codec_init(struct · line 171
Quoted 10 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +4
-0{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?
Quoted 15 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +5
-0+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
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Quoted 18 lines · Adam Thomson
Quoted 16 lines · Mac Chiang
From: Mac Chiang <[email protected]>
unexpected error: PLL input clock 0 below valid range
backport from broonie's tree
commitfd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config tocodec_initfunction")The MCLK for DA7219 does not change in this platform, but is currently being configured everytime as part of the
platform_clockevent 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.
Quoted 6 lines
Quoted 6 lines
This patch rectifies this issue by moving the sysclk call to
codec_initfunction so it's only called once at initialisation.Signed-off-by Adam Thomson <[email protected]>Singed-off-by Mac Chiang <[email protected]>
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:
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.
Quoted 36 lines
Quoted 10 lines
--- sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-0sound/soc/intel/boards/kbl_da7219_max98357a.c static int platform_clock_control(struct · line 71
Quoted 7 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-3@@ -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...
Quoted 17 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-5-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);sound/soc/intel/boards/kbl_da7219_max98357a.c +1
-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;
Quoted 4 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c static int kabylake_da7219_codec_init(struct snd_soc_pcm_runtime *rtd) · line 170
Quoted 3 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +3
-0+/* 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.
Quoted 12 lines
Quoted 12 lines
sound/soc/intel/boards/kbl_da7219_max98357a.c +5
-0+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
Quoted 8 lines
Quoted 4 lines
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
PATCH 1/1 ASoC: Intel: kbl: Move codec sysclk config to codec_init function
In reply to [email protected]From: Mac Chiang <[email protected]>
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.
--- sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
sound/soc/intel/boards/kbl_da7219_max98357a.c +9 -8
@@ -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.
Signature · 1 line
2.7.4
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] ..."
Quoted 15 lines
From: Mac Chiang <[email protected]>
fix an issue on KBL which applied the merged solution from broonie's tree
commitfd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config tocodec_initfunction")The MCLK for DA7219 does not change in this platform, but is currently being configured everytime as part of the
platform_clockevent 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.
Quoted 7 lines
This patch rectifies this issue by moving the sysclk call to
codec_initfunction so it's only called once at initialisation.Signed-off-by Adam Thomson <[email protected]>Signed-off-by Mark Brown <[email protected]>Signed-off-by Mac Chiang <[email protected]>
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.
Quoted 52 lines
--- sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-8@@ -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,sound/soc/intel/boards/kbl_da7219_max98357a.c +9
-0DA7219_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
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Quoted 15 lines · Adam Thomson
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] ..."
Quoted 6 lines
From: Mac Chiang <[email protected]>
fix an issue on KBL which applied the merged solution from broonie's tree
commitfd0f237572ad
("ASoC: Intel: bxt: Move codec sysclk config tocodec_initfunction")
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
Quoted 8 lines
Quoted 8 lines
The MCLK for DA7219 does not change in this platform, but is currently being configured everytime as part of the
platform_clockevent 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.'
Quoted 7 lines
Quoted 7 lines
This patch rectifies this issue by moving the sysclk call to
codec_initfunction so it's only called once at initialisation.Signed-off-by Adam Thomson <[email protected]>Signed-off-by Mark Brown <[email protected]>
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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Mac Chiang <[email protected]>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
Quoted 57 lines
Quoted 49 lines
--- sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)sound/soc/intel/boards/kbl_da7219_max98357a.c +0
-8@@ -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);sound/soc/intel/boards/kbl_da7219_max98357a.c +9
-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.4Quoted 4 lines
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Mark Brown
maintainerApplied "ASoC: Intel: kbl: Move codec sysclk config to codec_init function" to the asoc tree
In reply to [email protected]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
Quoted 1 line
From
56c3a95385df558a471f7fcedba73c9ed836a906Mon Sep 17 00:00:00 2001
From: Mac Chiang <[email protected]>
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.
--- sound/soc/intel/boards/kbl_da7219_max98357a.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
sound/soc/intel/boards/kbl_da7219_max98357a.c +9 -8
@@ -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.
Signature · 1 line
2.17.0