[alsa-devel] [PATCH] ASoC: wm8994: Manage wm8994's MCLK in codec driver
Previously, When we use wm8994 codec, we should control its MCLK in machine driver. But, It should be managed by wm8994 codec driver, not machine driver.
This patch add MCLK clock DT parsing logic and control own MCLK.
Signed-off-by: Inha Song ideal.song@samsung.com --- sound/soc/codecs/wm8994.c | 27 ++++++++++++++++++++++++++- sound/soc/codecs/wm8994.h | 1 + 2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c index 247b390..d80dcc6 100644 --- a/sound/soc/codecs/wm8994.c +++ b/sound/soc/codecs/wm8994.c @@ -16,6 +16,7 @@ #include <linux/init.h> #include <linux/delay.h> #include <linux/pm.h> +#include <linux/clk.h> #include <linux/gcd.h> #include <linux/i2c.h> #include <linux/platform_device.h> @@ -2470,6 +2471,7 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, { struct wm8994_priv *wm8994 = snd_soc_codec_get_drvdata(codec); struct wm8994 *control = wm8994->wm8994; + int i;
wm_hubs_set_bias_level(codec, level);
@@ -2491,8 +2493,19 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, break; }
- if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) + if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) { + for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) { + if (!IS_ERR(wm8994->clk_mclk[i])) + clk_prepare_enable(wm8994->clk_mclk[i]); + } active_reference(codec); + } else if (codec->dapm.bias_level == SND_SOC_BIAS_ON) { + for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) { + if (!IS_ERR(wm8994->clk_mclk[i])) + clk_disable_unprepare( + wm8994->clk_mclk[i]); + } + } break;
case SND_SOC_BIAS_STANDBY: @@ -4451,6 +4464,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8994 = { static int wm8994_probe(struct platform_device *pdev) { struct wm8994_priv *wm8994; + int i;
wm8994 = devm_kzalloc(&pdev->dev, sizeof(struct wm8994_priv), GFP_KERNEL); @@ -4460,6 +4474,17 @@ static int wm8994_probe(struct platform_device *pdev)
wm8994->wm8994 = dev_get_drvdata(pdev->dev.parent);
+ for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) { + char mclk_name[] = "MCLKx"; + + snprintf(mclk_name, sizeof(mclk_name), "MCLK%d", i + 1); + + wm8994->clk_mclk[i] = devm_clk_get(pdev->dev.parent, mclk_name); + if (IS_ERR(wm8994->clk_mclk[i])) + if (PTR_ERR(wm8994->clk_mclk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + } + pm_runtime_enable(&pdev->dev); pm_runtime_idle(&pdev->dev);
diff --git a/sound/soc/codecs/wm8994.h b/sound/soc/codecs/wm8994.h index 6536f8d..8cf06a4 100644 --- a/sound/soc/codecs/wm8994.h +++ b/sound/soc/codecs/wm8994.h @@ -78,6 +78,7 @@ struct wm8994_priv { int sysclk[2]; int sysclk_rate[2]; int mclk[2]; + struct clk *clk_mclk[2]; int aifclk[2]; int aifdiv[2]; int channels[2];
Previously, When we use wm8994 codec, we should control its MCLK in machine driver. But, It should be managed by wm8994 codec driver, not machine driver.
This patch add MCLK clock DT parsing logic and control own MCLK.
Signed-off-by: Inha Song ideal.song@samsung.com --- sound/soc/codecs/wm8994.c | 27 ++++++++++++++++++++++++++- sound/soc/codecs/wm8994.h | 1 + 2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c index 247b390..d80dcc6 100644 --- a/sound/soc/codecs/wm8994.c +++ b/sound/soc/codecs/wm8994.c @@ -16,6 +16,7 @@ #include <linux/init.h> #include <linux/delay.h> #include <linux/pm.h> +#include <linux/clk.h> #include <linux/gcd.h> #include <linux/i2c.h> #include <linux/platform_device.h> @@ -2470,6 +2471,7 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, { struct wm8994_priv *wm8994 = snd_soc_codec_get_drvdata(codec); struct wm8994 *control = wm8994->wm8994; + int i;
wm_hubs_set_bias_level(codec, level);
@@ -2491,8 +2493,19 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, break; }
- if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) + if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) { + for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) { + if (!IS_ERR(wm8994->clk_mclk[i])) + clk_prepare_enable(wm8994->clk_mclk[i]); + } active_reference(codec); + } else if (codec->dapm.bias_level == SND_SOC_BIAS_ON) { + for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) { + if (!IS_ERR(wm8994->clk_mclk[i])) + clk_disable_unprepare( + wm8994->clk_mclk[i]); + } + } break;
case SND_SOC_BIAS_STANDBY: @@ -4451,6 +4464,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8994 = { static int wm8994_probe(struct platform_device *pdev) { struct wm8994_priv *wm8994; + int i;
wm8994 = devm_kzalloc(&pdev->dev, sizeof(struct wm8994_priv), GFP_KERNEL); @@ -4460,6 +4474,17 @@ static int wm8994_probe(struct platform_device *pdev)
wm8994->wm8994 = dev_get_drvdata(pdev->dev.parent);
+ for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) { + char mclk_name[] = "MCLKx"; + + snprintf(mclk_name, sizeof(mclk_name), "MCLK%d", i + 1); + + wm8994->clk_mclk[i] = devm_clk_get(pdev->dev.parent, mclk_name); + if (IS_ERR(wm8994->clk_mclk[i])) + if (PTR_ERR(wm8994->clk_mclk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + } + pm_runtime_enable(&pdev->dev); pm_runtime_idle(&pdev->dev);
diff --git a/sound/soc/codecs/wm8994.h b/sound/soc/codecs/wm8994.h index 6536f8d..8cf06a4 100644 --- a/sound/soc/codecs/wm8994.h +++ b/sound/soc/codecs/wm8994.h @@ -78,6 +78,7 @@ struct wm8994_priv { int sysclk[2]; int sysclk_rate[2]; int mclk[2]; + struct clk *clk_mclk[2]; int aifclk[2]; int aifdiv[2]; int channels[2];
Hi Inha,
On 03/12/2015 11:49 AM, Inha Song wrote:
Previously, When we use wm8994 codec, we should control its MCLK in machine driver. But, It should be managed by wm8994 codec driver, not machine driver.
This patch add MCLK clock DT parsing logic and control own MCLK.
Signed-off-by: Inha Song ideal.song@samsung.com
sound/soc/codecs/wm8994.c | 27 ++++++++++++++++++++++++++- sound/soc/codecs/wm8994.h | 1 + 2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c index 247b390..d80dcc6 100644 --- a/sound/soc/codecs/wm8994.c +++ b/sound/soc/codecs/wm8994.c @@ -16,6 +16,7 @@ #include <linux/init.h> #include <linux/delay.h> #include <linux/pm.h> +#include <linux/clk.h> #include <linux/gcd.h> #include <linux/i2c.h> #include <linux/platform_device.h> @@ -2470,6 +2471,7 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, { struct wm8994_priv *wm8994 = snd_soc_codec_get_drvdata(codec); struct wm8994 *control = wm8994->wm8994;
int i;
wm_hubs_set_bias_level(codec, level);
@@ -2491,8 +2493,19 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, break; }
if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY)
if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) {for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) {if (!IS_ERR(wm8994->clk_mclk[i]))clk_prepare_enable(wm8994->clk_mclk[i]);
I think that you don't need to check error by using 'if' statement. clk_prepare_enable() function will check the NULL state of clk instance before preparing and enabling the clock.
for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) clk_prepare_enable(wm8994->clk_mclk[i]);
But, wm8994_probe() have to initialize the NULL for wm8994->clk_mclk[i]. I added the comment about this on below.
} active_reference(codec);} else if (codec->dapm.bias_level == SND_SOC_BIAS_ON) {for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) {if (!IS_ERR(wm8994->clk_mclk[i]))clk_disable_unprepare(wm8994->clk_mclk[i]);}}break;
case SND_SOC_BIAS_STANDBY:
@@ -4451,6 +4464,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8994 = { static int wm8994_probe(struct platform_device *pdev) { struct wm8994_priv *wm8994;
int i;
wm8994 = devm_kzalloc(&pdev->dev, sizeof(struct wm8994_priv), GFP_KERNEL);
@@ -4460,6 +4474,17 @@ static int wm8994_probe(struct platform_device *pdev)
wm8994->wm8994 = dev_get_drvdata(pdev->dev.parent);
- for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) {
char mclk_name[] = "MCLKx";snprintf(mclk_name, sizeof(mclk_name), "MCLK%d", i + 1);wm8994->clk_mclk[i] = devm_clk_get(pdev->dev.parent, mclk_name);if (IS_ERR(wm8994->clk_mclk[i]))if (PTR_ERR(wm8994->clk_mclk) == -EPROBE_DEFER)return -EPROBE_DEFER;
I think you better to modify exception handling as following:
if (IS_ERR(wm8994->clk_mclk[i])) { wm8994->clk_mclk[i] = NULL; return PTR_ERR(wm8994->clk_mclk); }
- }
- pm_runtime_enable(&pdev->dev); pm_runtime_idle(&pdev->dev);
diff --git a/sound/soc/codecs/wm8994.h b/sound/soc/codecs/wm8994.h index 6536f8d..8cf06a4 100644 --- a/sound/soc/codecs/wm8994.h +++ b/sound/soc/codecs/wm8994.h @@ -78,6 +78,7 @@ struct wm8994_priv { int sysclk[2]; int sysclk_rate[2]; int mclk[2];
- struct clk *clk_mclk[2]; int aifclk[2]; int aifdiv[2]; int channels[2];
Thanks, Chanwoo Choi
Hi, Chanwoo,
Thanks for your comments.
On Thu, 12 Mar 2015 13:33:49 +0900 Chanwoo Choi cw00.choi@samsung.com wrote:
Hi Inha,
On 03/12/2015 11:49 AM, Inha Song wrote:
Previously, When we use wm8994 codec, we should control its MCLK in machine driver. But, It should be managed by wm8994 codec driver, not machine driver.
This patch add MCLK clock DT parsing logic and control own MCLK.
Signed-off-by: Inha Song ideal.song@samsung.com
sound/soc/codecs/wm8994.c | 27 ++++++++++++++++++++++++++- sound/soc/codecs/wm8994.h | 1 + 2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c index 247b390..d80dcc6 100644 --- a/sound/soc/codecs/wm8994.c +++ b/sound/soc/codecs/wm8994.c @@ -16,6 +16,7 @@ #include <linux/init.h> #include <linux/delay.h> #include <linux/pm.h> +#include <linux/clk.h> #include <linux/gcd.h> #include <linux/i2c.h> #include <linux/platform_device.h> @@ -2470,6 +2471,7 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, { struct wm8994_priv *wm8994 = snd_soc_codec_get_drvdata(codec); struct wm8994 *control = wm8994->wm8994;
int i;
wm_hubs_set_bias_level(codec, level);
@@ -2491,8 +2493,19 @@ static int wm8994_set_bias_level(struct snd_soc_codec *codec, break; }
if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY)
if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) {for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) {if (!IS_ERR(wm8994->clk_mclk[i]))clk_prepare_enable(wm8994->clk_mclk[i]);I think that you don't need to check error by using 'if' statement. clk_prepare_enable() function will check the NULL state of clk instance before preparing and enabling the clock.
So that, I can handle it more concisely. Thanks, I will fix this.
for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) clk_prepare_enable(wm8994->clk_mclk[i]);But, wm8994_probe() have to initialize the NULL for wm8994->clk_mclk[i]. I added the comment about this on below.
} active_reference(codec);} else if (codec->dapm.bias_level == SND_SOC_BIAS_ON) {for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) {if (!IS_ERR(wm8994->clk_mclk[i]))clk_disable_unprepare(wm8994->clk_mclk[i]);
ditto
}}break;
case SND_SOC_BIAS_STANDBY:
@@ -4451,6 +4464,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8994 = { static int wm8994_probe(struct platform_device *pdev) { struct wm8994_priv *wm8994;
int i;
wm8994 = devm_kzalloc(&pdev->dev, sizeof(struct wm8994_priv), GFP_KERNEL);
@@ -4460,6 +4474,17 @@ static int wm8994_probe(struct platform_device *pdev)
wm8994->wm8994 = dev_get_drvdata(pdev->dev.parent);
- for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) {
char mclk_name[] = "MCLKx";snprintf(mclk_name, sizeof(mclk_name), "MCLK%d", i + 1);wm8994->clk_mclk[i] = devm_clk_get(pdev->dev.parent, mclk_name);if (IS_ERR(wm8994->clk_mclk[i]))if (PTR_ERR(wm8994->clk_mclk) == -EPROBE_DEFER)return -EPROBE_DEFER;I think you better to modify exception handling as following:
if (IS_ERR(wm8994->clk_mclk[i])) { wm8994->clk_mclk[i] = NULL; return PTR_ERR(wm8994->clk_mclk);
I will fix. (MCLK is optional properties in wm8994)
if (IS_ERR(wm8994->clk_mclk[i])) wm8994->clk_mclk[i] = NULL;
Best Regards, Inha Song.
}
- }
- pm_runtime_enable(&pdev->dev); pm_runtime_idle(&pdev->dev);
diff --git a/sound/soc/codecs/wm8994.h b/sound/soc/codecs/wm8994.h index 6536f8d..8cf06a4 100644 --- a/sound/soc/codecs/wm8994.h +++ b/sound/soc/codecs/wm8994.h @@ -78,6 +78,7 @@ struct wm8994_priv { int sysclk[2]; int sysclk_rate[2]; int mclk[2];
- struct clk *clk_mclk[2]; int aifclk[2]; int aifdiv[2]; int channels[2];
Thanks, Chanwoo Choi
On Thu, Mar 12, 2015 at 11:49:39AM +0900, Inha Song wrote:
if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY)
if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) {for (i = 0; i < ARRAY_SIZE(wm8994->clk_mclk); i++) {if (!IS_ERR(wm8994->clk_mclk[i]))clk_prepare_enable(wm8994->clk_mclk[i]);}
This isn't checking return values. It's also enabling both clocks all the time which isn't always going to be what's desired - it may result in an expensive high frequency reference clock being enabled when only a 32kHz clock is needed for example. That doesn't seem desirable.
active_reference(codec);
Should this perhaps be being done in active_reference instead?
participants (3)
-
Chanwoo Choi -
Inha Song -
Mark Brown