* [RFC v2 PATCH] ASoC: wm8904: add CCF support
@ 2014-03-25 9:56 Bo Shen
2014-03-25 11:27 ` Mark Brown
2014-03-25 11:59 ` Boris BREZILLON
0 siblings, 2 replies; 4+ messages in thread
From: Bo Shen @ 2014-03-25 9:56 UTC (permalink / raw)
To: linux-arm-kernel
Enable WM8904 to support common clock framework.
- Now it supports use SoC provided clock or external oscillator
as MCLK.
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
Documentation/devicetree/bindings/sound/wm8904.txt | 48 ++++++++++++++++++++++
sound/soc/codecs/wm8904.c | 17 ++++++++
2 files changed, 65 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/wm8904.txt
diff --git a/Documentation/devicetree/bindings/sound/wm8904.txt b/Documentation/devicetree/bindings/sound/wm8904.txt
new file mode 100644
index 0000000..bcf4aa0
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/wm8904.txt
@@ -0,0 +1,48 @@
+WM8904 audio CODEC
+
+This device supports I2C only.
+
+Required properties:
+ - compatible: "wlf,wm8904"
+ - reg: the I2C address of the device.
+ - clocks and clock-names: reference to
+ <Documentation/devicetree/bindings/clock/clock-bindings.txt>
+
+Pins on the device (for linking into audio routes):
+
+ * IN1L
+ * IN1R
+ * IN2L
+ * IN2R
+ * IN3L
+ * IN3R
+ * HPOUTL
+ * HPOUTR
+ * LINEOUTL
+ * LINEOUTR
+ * MICBIAS
+
+Examples:
+
+1. Using SoC provided clock as mclk:
+codec: wm8904 at 1a {
+ compatible = "wlf,wm8904";
+ reg = <0x1a>;
+ clocks = <&pck0>;
+ clock-names = "mclk";
+};
+
+2. Using external oscillator as mclk:
+codec: wm8904 at 1a {
+ compatible = "wlf,wm8904";
+ reg = <0x1a>;
+
+ wm8904_osc {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ #clock-frequency = <12000000>;
+ }
+
+ clocks = <&wm8904_osc>;
+ clock-names = "mclk";
+};
diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index 49c35c3..4db6a26 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -11,6 +11,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/clk.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/init.h>
@@ -49,6 +50,7 @@ static const char *wm8904_supply_names[WM8904_NUM_SUPPLIES] = {
/* codec private data */
struct wm8904_priv {
struct regmap *regmap;
+ struct clk *mclk;
enum wm8904_type devtype;
@@ -1828,6 +1830,9 @@ static int wm8904_set_bias_level(struct snd_soc_codec *codec,
switch (level) {
case SND_SOC_BIAS_ON:
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ if (wm8904->mclk)
+ clk_prepare_enable(wm8904->mclk);
break;
case SND_SOC_BIAS_PREPARE:
@@ -1894,6 +1899,9 @@ static int wm8904_set_bias_level(struct snd_soc_codec *codec,
regulator_bulk_disable(ARRAY_SIZE(wm8904->supplies),
wm8904->supplies);
+ if (IS_ENABLED(CONFIG_COMMON_CLK))
+ if (wm8904->mclk)
+ clk_disable_unprepare(wm8904->mclk);
break;
}
codec->dapm.bias_level = level;
@@ -2139,6 +2147,15 @@ static int wm8904_i2c_probe(struct i2c_client *i2c,
return ret;
}
+ if (IS_ENABLED(CONFIG_COMMON_CLK)) {
+ wm8904->mclk = devm_clk_get(&i2c->dev, "mclk");
+ if (IS_ERR(wm8904->mclk)) {
+ dev_err(&i2c->dev, "Failed to get MCLK\n");
+ ret = PTR_ERR(wm8904->mclk);
+ goto err_enable;
+ }
+ }
+
ret = regmap_read(wm8904->regmap, WM8904_SW_RESET_AND_ID, &val);
if (ret < 0) {
dev_err(&i2c->dev, "Failed to read ID register: %d\n", ret);
--
1.8.5.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [RFC v2 PATCH] ASoC: wm8904: add CCF support
2014-03-25 9:56 [RFC v2 PATCH] ASoC: wm8904: add CCF support Bo Shen
@ 2014-03-25 11:27 ` Mark Brown
2014-03-26 1:32 ` [alsa-devel] " Bo Shen
2014-03-25 11:59 ` Boris BREZILLON
1 sibling, 1 reply; 4+ messages in thread
From: Mark Brown @ 2014-03-25 11:27 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 25, 2014 at 05:56:56PM +0800, Bo Shen wrote:
> case SND_SOC_BIAS_ON:
> + if (IS_ENABLED(CONFIG_COMMON_CLK))
> + if (wm8904->mclk)
> + clk_prepare_enable(wm8904->mclk);
No, this shouldn't depend on COMMON_CLK - there is no reason other clock
API implementations shouldn't be able to use this. Providing a clock is
something you can only do with COMMON_CLK but using one doesn't need
that, generally using a clock is done unconditionally.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140325/7949b8d6/attachment.sig>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC v2 PATCH] ASoC: wm8904: add CCF support
2014-03-25 9:56 [RFC v2 PATCH] ASoC: wm8904: add CCF support Bo Shen
2014-03-25 11:27 ` Mark Brown
@ 2014-03-25 11:59 ` Boris BREZILLON
1 sibling, 0 replies; 4+ messages in thread
From: Boris BREZILLON @ 2014-03-25 11:59 UTC (permalink / raw)
To: linux-arm-kernel
Hello Bo,
Le 25/03/2014 10:56, Bo Shen a ?crit :
> Enable WM8904 to support common clock framework.
> - Now it supports use SoC provided clock or external oscillator
> as MCLK.
>
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> ---
> Documentation/devicetree/bindings/sound/wm8904.txt | 48 ++++++++++++++++++++++
> sound/soc/codecs/wm8904.c | 17 ++++++++
> 2 files changed, 65 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/wm8904.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/wm8904.txt b/Documentation/devicetree/bindings/sound/wm8904.txt
> new file mode 100644
> index 0000000..bcf4aa0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/wm8904.txt
> @@ -0,0 +1,48 @@
> +WM8904 audio CODEC
> +
> +This device supports I2C only.
> +
> +Required properties:
> + - compatible: "wlf,wm8904"
> + - reg: the I2C address of the device.
> + - clocks and clock-names: reference to
> + <Documentation/devicetree/bindings/clock/clock-bindings.txt>
> +
> +Pins on the device (for linking into audio routes):
> +
> + * IN1L
> + * IN1R
> + * IN2L
> + * IN2R
> + * IN3L
> + * IN3R
> + * HPOUTL
> + * HPOUTR
> + * LINEOUTL
> + * LINEOUTR
> + * MICBIAS
> +
> +Examples:
> +
> +1. Using SoC provided clock as mclk:
> +codec: wm8904 at 1a {
> + compatible = "wlf,wm8904";
> + reg = <0x1a>;
> + clocks = <&pck0>;
> + clock-names = "mclk";
> +};
> +
> +2. Using external oscillator as mclk:
> +codec: wm8904 at 1a {
> + compatible = "wlf,wm8904";
> + reg = <0x1a>;
> +
> + wm8904_osc {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + #clock-frequency = <12000000>;
> + }
> +
> + clocks = <&wm8904_osc>;
> + clock-names = "mclk";
> +};
I think there is a misunderstanding here (and it certainly comes from my
explanations during our last talk :)): both cases should be handled the same
way because an external oscillator is no more or less than an external clk
(we don't care how this clk is generated).
I suggested to put the osc node inside the wm8904 one but I thought you were
trying to represent the internal 12MHz clk generated when using the fll free
running mode.
> diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
> index 49c35c3..4db6a26 100644
> --- a/sound/soc/codecs/wm8904.c
> +++ b/sound/soc/codecs/wm8904.c
> @@ -11,6 +11,7 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/clk.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/init.h>
> @@ -49,6 +50,7 @@ static const char *wm8904_supply_names[WM8904_NUM_SUPPLIES] = {
> /* codec private data */
> struct wm8904_priv {
> struct regmap *regmap;
> + struct clk *mclk;
>
> enum wm8904_type devtype;
>
> @@ -1828,6 +1830,9 @@ static int wm8904_set_bias_level(struct snd_soc_codec *codec,
>
> switch (level) {
> case SND_SOC_BIAS_ON:
> + if (IS_ENABLED(CONFIG_COMMON_CLK))
> + if (wm8904->mclk)
> + clk_prepare_enable(wm8904->mclk);
> break;
>
> case SND_SOC_BIAS_PREPARE:
> @@ -1894,6 +1899,9 @@ static int wm8904_set_bias_level(struct snd_soc_codec *codec,
>
> regulator_bulk_disable(ARRAY_SIZE(wm8904->supplies),
> wm8904->supplies);
> + if (IS_ENABLED(CONFIG_COMMON_CLK))
> + if (wm8904->mclk)
> + clk_disable_unprepare(wm8904->mclk);
> break;
> }
> codec->dapm.bias_level = level;
> @@ -2139,6 +2147,15 @@ static int wm8904_i2c_probe(struct i2c_client *i2c,
> return ret;
> }
>
> + if (IS_ENABLED(CONFIG_COMMON_CLK)) {
> + wm8904->mclk = devm_clk_get(&i2c->dev, "mclk");
> + if (IS_ERR(wm8904->mclk)) {
> + dev_err(&i2c->dev, "Failed to get MCLK\n");
> + ret = PTR_ERR(wm8904->mclk);
> + goto err_enable;
> + }
> + }
> +
> ret = regmap_read(wm8904->regmap, WM8904_SW_RESET_AND_ID, &val);
> if (ret < 0) {
> dev_err(&i2c->dev, "Failed to read ID register: %d\n", ret);
^ permalink raw reply [flat|nested] 4+ messages in thread
* [alsa-devel] [RFC v2 PATCH] ASoC: wm8904: add CCF support
2014-03-25 11:27 ` Mark Brown
@ 2014-03-26 1:32 ` Bo Shen
0 siblings, 0 replies; 4+ messages in thread
From: Bo Shen @ 2014-03-26 1:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark,
On 03/25/2014 07:27 PM, Mark Brown wrote:
> On Tue, Mar 25, 2014 at 05:56:56PM +0800, Bo Shen wrote:
>
>> case SND_SOC_BIAS_ON:
>> + if (IS_ENABLED(CONFIG_COMMON_CLK))
>> + if (wm8904->mclk)
>> + clk_prepare_enable(wm8904->mclk);
>
> No, this shouldn't depend on COMMON_CLK - there is no reason other clock
> API implementations shouldn't be able to use this. Providing a clock is
> something you can only do with COMMON_CLK but using one doesn't need
> that, generally using a clock is done unconditionally.
Actually in probe function, it does this check, if not CCF support, the
wm8904->mclk is NULL. So, we can remove the CCF check here.
Thanks.
Best Regards,
Bo Shen
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-26 1:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-25 9:56 [RFC v2 PATCH] ASoC: wm8904: add CCF support Bo Shen
2014-03-25 11:27 ` Mark Brown
2014-03-26 1:32 ` [alsa-devel] " Bo Shen
2014-03-25 11:59 ` Boris BREZILLON
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).