[PATCH] ASoC: rt5640: add master clock handling for rt5640

[PATCH] ASoC: rt5640: add master clock handling for rt5640

[Sugar Zhang]

enable/disable master clock when codec is active or not.

Signed-off-by: Sugar Zhang <sugar.zhang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

 Documentation/devicetree/bindings/sound/rt5640.txt |  3 +++
 sound/soc/codecs/rt5640.c                          | 31 ++++++++++++++++++++++
 sound/soc/codecs/rt5640.h                          |  2 ++
 3 files changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rt5640.txt b/Documentation/devicetree/bindings/sound/rt5640.txt
index 9e62f6e..57fe646 100644
--- a/Documentation/devicetree/bindings/sound/rt5640.txt
+++ b/Documentation/devicetree/bindings/sound/rt5640.txt
@@ -12,6 +12,9 @@ Required properties:
 
 Optional properties:
 
+- clocks: The phandle of the master clock to the CODEC
+- clock-names: Should be "mclk"
+
 - realtek,in1-differential
 - realtek,in2-differential
 - realtek,in3-differential
diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 11d032c..6cd84fb 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -1949,7 +1949,33 @@ static int rt5640_set_dai_pll(struct snd_soc_dai *dai, int pll_id, int source,
 static int rt5640_set_bias_level(struct snd_soc_codec *codec,
 			enum snd_soc_bias_level level)
 {
+	struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
+	int ret;
+
 	switch (level) {
+	case SND_SOC_BIAS_ON:
+		break;
+
+	case SND_SOC_BIAS_PREPARE:
+		/*
+		 * SND_SOC_BIAS_PREPARE is called while preparing for a
+		 * transition to ON or away from ON. If current bias_level
+		 * is SND_SOC_BIAS_ON, then it is preparing for a transition
+		 * away from ON. Disable the clock in that case, otherwise
+		 * enable it.
+		 */
+		if (IS_ERR(rt5640->mclk))
+			break;
+
+		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_ON) {
+			clk_disable_unprepare(rt5640->mclk);
+		} else {
+			ret = clk_prepare_enable(rt5640->mclk);
+			if (ret)
+				return ret;
+		}
+		break;
+
 	case SND_SOC_BIAS_STANDBY:
 		if (SND_SOC_BIAS_OFF == snd_soc_codec_get_bias_level(codec)) {
 			snd_soc_update_bits(codec, RT5640_PWR_ANLG1,
@@ -2088,6 +2114,11 @@ static int rt5640_probe(struct snd_soc_codec *codec)
 	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
 	struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
 
+	/* Check if MCLK provided */
+	rt5640->mclk = devm_clk_get(codec->dev, "mclk");
+	if (PTR_ERR(rt5640->mclk) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	rt5640->codec = codec;
 
 	snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 83a7150..1761c3a9 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -12,6 +12,7 @@
 #ifndef _RT5640_H
 #define _RT5640_H
 
+#include <linux/clk.h>
 #include <sound/rt5640.h>
 
 /* Info */
@@ -2097,6 +2098,7 @@ struct rt5640_priv {
 	struct snd_soc_codec *codec;
 	struct rt5640_platform_data pdata;
 	struct regmap *regmap;
+	struct clk *mclk;
 
 	int sysclk;
 	int sysclk_src;
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Pierre-Louis Bossart]

On 2/22/16 7:26 PM, Sugar Zhang wrote:
> enable/disable master clock when codec is active or not.
>
> Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
> ---
>
>   Documentation/devicetree/bindings/sound/rt5640.txt |  3 +++
>   sound/soc/codecs/rt5640.c                          | 31 ++++++++++++++++++++++
>   sound/soc/codecs/rt5640.h                          |  2 ++
>   3 files changed, 36 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/rt5640.txt b/Documentation/devicetree/bindings/sound/rt5640.txt
> index 9e62f6e..57fe646 100644
> --- a/Documentation/devicetree/bindings/sound/rt5640.txt
> +++ b/Documentation/devicetree/bindings/sound/rt5640.txt
> @@ -12,6 +12,9 @@ Required properties:
>
>   Optional properties:
>
> +- clocks: The phandle of the master clock to the CODEC
> +- clock-names: Should be "mclk"

This patch assumes that the information on mclk comes from DeviceTree. 
The mclk may also be enabled/disabled in the machine driver with an 
explicit transition to use an internal clock when the codec is not used.
I hope this patch doesn't preclude such usages or there will be a 
conflict with the patches we are about to upstream for Baytrail/cht devices.

> +
>   - realtek,in1-differential
>   - realtek,in2-differential
>   - realtek,in3-differential
> diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
> index 11d032c..6cd84fb 100644
> --- a/sound/soc/codecs/rt5640.c
> +++ b/sound/soc/codecs/rt5640.c
> @@ -1949,7 +1949,33 @@ static int rt5640_set_dai_pll(struct snd_soc_dai *dai, int pll_id, int source,
>   static int rt5640_set_bias_level(struct snd_soc_codec *codec,
>   			enum snd_soc_bias_level level)
>   {
> +	struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
> +	int ret;
> +
>   	switch (level) {
> +	case SND_SOC_BIAS_ON:
> +		break;
> +
> +	case SND_SOC_BIAS_PREPARE:
> +		/*
> +		 * SND_SOC_BIAS_PREPARE is called while preparing for a
> +		 * transition to ON or away from ON. If current bias_level
> +		 * is SND_SOC_BIAS_ON, then it is preparing for a transition
> +		 * away from ON. Disable the clock in that case, otherwise
> +		 * enable it.
> +		 */
> +		if (IS_ERR(rt5640->mclk))
> +			break;
> +
> +		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_ON) {
> +			clk_disable_unprepare(rt5640->mclk);
> +		} else {
> +			ret = clk_prepare_enable(rt5640->mclk);
> +			if (ret)
> +				return ret;
> +		}
> +		break;
> +
>   	case SND_SOC_BIAS_STANDBY:
>   		if (SND_SOC_BIAS_OFF == snd_soc_codec_get_bias_level(codec)) {
>   			snd_soc_update_bits(codec, RT5640_PWR_ANLG1,
> @@ -2088,6 +2114,11 @@ static int rt5640_probe(struct snd_soc_codec *codec)
>   	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>   	struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
>
> +	/* Check if MCLK provided */
> +	rt5640->mclk = devm_clk_get(codec->dev, "mclk");
> +	if (PTR_ERR(rt5640->mclk) == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +
>   	rt5640->codec = codec;
>
>   	snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
> diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
> index 83a7150..1761c3a9 100644
> --- a/sound/soc/codecs/rt5640.h
> +++ b/sound/soc/codecs/rt5640.h
> @@ -12,6 +12,7 @@
>   #ifndef _RT5640_H
>   #define _RT5640_H
>
> +#include <linux/clk.h>
>   #include <sound/rt5640.h>
>
>   /* Info */
> @@ -2097,6 +2098,7 @@ struct rt5640_priv {
>   	struct snd_soc_codec *codec;
>   	struct rt5640_platform_data pdata;
>   	struct regmap *regmap;
> +	struct clk *mclk;
>
>   	int sysclk;
>   	int sysclk_src;
>

Re: [alsa-devel] [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Heiko Stuebner]

Am Dienstag, 23. Februar 2016, 08:50:04 schrieb Pierre-Louis Bossart:
> On 2/22/16 7:26 PM, Sugar Zhang wrote:
> > enable/disable master clock when codec is active or not.
> > 
> > Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
> > ---
> > 
> >   Documentation/devicetree/bindings/sound/rt5640.txt |  3 +++
> >   sound/soc/codecs/rt5640.c                          | 31
> >   ++++++++++++++++++++++ sound/soc/codecs/rt5640.h                     
> >       |  2 ++
> >   3 files changed, 36 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/rt5640.txt
> > b/Documentation/devicetree/bindings/sound/rt5640.txt index
> > 9e62f6e..57fe646 100644
> > --- a/Documentation/devicetree/bindings/sound/rt5640.txt
> > +++ b/Documentation/devicetree/bindings/sound/rt5640.txt
> > 
> > @@ -12,6 +12,9 @@ Required properties:
> >   Optional properties:
> > +- clocks: The phandle of the master clock to the CODEC
> > +- clock-names: Should be "mclk"
> 
> This patch assumes that the information on mclk comes from DeviceTree.
> The mclk may also be enabled/disabled in the machine driver with an
> explicit transition to use an internal clock when the codec is not used.
> I hope this patch doesn't preclude such usages or there will be a
> conflict with the patches we are about to upstream for Baytrail/cht
> devices.

As you can see, the clock property itself is optional and the clk_get below 
acts accordingly of also continuing if the clock is not present.
So it won't affect any users doing it otherwise.


Heiko

> > +
> > 
> >   - realtek,in1-differential
> >   - realtek,in2-differential
> >   - realtek,in3-differential
> > 
> > diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
> > index 11d032c..6cd84fb 100644
> > --- a/sound/soc/codecs/rt5640.c
> > +++ b/sound/soc/codecs/rt5640.c
> > @@ -1949,7 +1949,33 @@ static int rt5640_set_dai_pll(struct snd_soc_dai
> > *dai, int pll_id, int source,> 
> >   static int rt5640_set_bias_level(struct snd_soc_codec *codec,
> >   
> >   			enum snd_soc_bias_level level)
> >   
> >   {
> > 
> > +	struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
> > +	int ret;
> > +
> > 
> >   	switch (level) {
> > 
> > +	case SND_SOC_BIAS_ON:
> > +		break;
> > +
> > +	case SND_SOC_BIAS_PREPARE:
> > +		/*
> > +		 * SND_SOC_BIAS_PREPARE is called while preparing for a
> > +		 * transition to ON or away from ON. If current bias_level
> > +		 * is SND_SOC_BIAS_ON, then it is preparing for a transition
> > +		 * away from ON. Disable the clock in that case, otherwise
> > +		 * enable it.
> > +		 */
> > +		if (IS_ERR(rt5640->mclk))
> > +			break;
> > +
> > +		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_ON) {
> > +			clk_disable_unprepare(rt5640->mclk);
> > +		} else {
> > +			ret = clk_prepare_enable(rt5640->mclk);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +		break;
> > +
> > 
> >   	case SND_SOC_BIAS_STANDBY:
> >   		if (SND_SOC_BIAS_OFF == snd_soc_codec_get_bias_level(codec)) {
> >   		
> >   			snd_soc_update_bits(codec, RT5640_PWR_ANLG1,
> > 
> > @@ -2088,6 +2114,11 @@ static int rt5640_probe(struct snd_soc_codec
> > *codec)> 
> >   	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
> >   	struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
> > 
> > +	/* Check if MCLK provided */
> > +	rt5640->mclk = devm_clk_get(codec->dev, "mclk");
> > +	if (PTR_ERR(rt5640->mclk) == -EPROBE_DEFER)
> > +		return -EPROBE_DEFER;
> > +
> > 
> >   	rt5640->codec = codec;
> >   	
> >   	snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
> > 
> > diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
> > index 83a7150..1761c3a9 100644
> > --- a/sound/soc/codecs/rt5640.h
> > +++ b/sound/soc/codecs/rt5640.h
> > @@ -12,6 +12,7 @@
> > 
> >   #ifndef _RT5640_H
> >   #define _RT5640_H
> > 
> > +#include <linux/clk.h>
> > 
> >   #include <sound/rt5640.h>
> >   
> >   /* Info */
> > 
> > @@ -2097,6 +2098,7 @@ struct rt5640_priv {
> > 
> >   	struct snd_soc_codec *codec;
> >   	struct rt5640_platform_data pdata;
> >   	struct regmap *regmap;
> > 
> > +	struct clk *mclk;
> > 
> >   	int sysclk;
> >   	int sysclk_src;

Re: [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Mark Brown]

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

On Wed, Feb 24, 2016 at 12:08:31AM +0100, Heiko Stuebner wrote:
> Am Dienstag, 23. Februar 2016, 08:50:04 schrieb Pierre-Louis Bossart:

> > This patch assumes that the information on mclk comes from DeviceTree.
> > The mclk may also be enabled/disabled in the machine driver with an
> > explicit transition to use an internal clock when the codec is not used.
> > I hope this patch doesn't preclude such usages or there will be a
> > conflict with the patches we are about to upstream for Baytrail/cht
> > devices.

> As you can see, the clock property itself is optional and the clk_get below 
> acts accordingly of also continuing if the clock is not present.
> So it won't affect any users doing it otherwise.

That said we really do need x86 to transition to use the clock API in
order to integrate with external devices, where the machine driver does
manage clocks we want that to move to being done using the clock API
rather than custom APIs.

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

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

Re: [alsa-devel] [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Pierre-Louis Bossart]

On 2/23/16 9:32 PM, Mark Brown wrote:
> On Wed, Feb 24, 2016 at 12:08:31AM +0100, Heiko Stuebner wrote:
>> Am Dienstag, 23. Februar 2016, 08:50:04 schrieb Pierre-Louis
>> Bossart:
>
>>> This patch assumes that the information on mclk comes from
>>> DeviceTree. The mclk may also be enabled/disabled in the machine
>>> driver with an explicit transition to use an internal clock when
>>> the codec is not used. I hope this patch doesn't preclude such
>>> usages or there will be a conflict with the patches we are about
>>> to upstream for Baytrail/cht devices.
>
>> As you can see, the clock property itself is optional and the
>> clk_get below acts accordingly of also continuing if the clock is
>> not present. So it won't affect any users doing it otherwise.
>
> That said we really do need x86 to transition to use the clock API
> in order to integrate with external devices, where the machine driver
> does manage clocks we want that to move to being done using the clock
> API rather than custom APIs.

For Baytrail audio we have a single platform clock that can be turned
on/off and set to 19.2 MHz or 25 MHz. No other controls are available,
no multipliers or complicated dependencies on other clocks, parents or 
children and no other users for this clock but the audio subsystem.
I looked at the clock framework and couldn't figure out how it would 
simply map the hardware so for now the use of the MCLK is only enabled 
with an on/off or set-rate(19.2|25) custom API. I am not an expert here 
so if this clock framework becomes a requirement to upstream code I 
would appreciate any pointers to do the right thing. I really couldn't 
find a simple example with 'Put your code here' comments to use this 
framework.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]

On Wed, Feb 24, 2016 at 10:10:05AM -0600, Pierre-Louis Bossart wrote:
> On 2/23/16 9:32 PM, Mark Brown wrote:

> >That said we really do need x86 to transition to use the clock API
> >in order to integrate with external devices, where the machine driver
> >does manage clocks we want that to move to being done using the clock
> >API rather than custom APIs.

> For Baytrail audio we have a single platform clock that can be turned
> on/off and set to 19.2 MHz or 25 MHz. No other controls are available,
> no multipliers or complicated dependencies on other clocks, parents or
> children and no other users for this clock but the audio subsystem.
> I looked at the clock framework and couldn't figure out how it would simply
> map the hardware so for now the use of the MCLK is only enabled with an
> on/off or set-rate(19.2|25) custom API. I am not an expert here so if this
> clock framework becomes a requirement to upstream code I would appreciate
> any pointers to do the right thing. I really couldn't find a simple example
> with 'Put your code here' comments to use this framework.

There's a lot of helpers like -fixed, -gate and so on - this sounds like
you could probably use those helpers (there should be lots of examples
in the kernel) or just implement a simple provider (see clk-provider.h)
depending on how the control is mapped in.  If it's that simple just
open coding a provider ought to do the job.  Then set up the client
linkage with clkdev in some way that makes sense for your platform.

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

Re: [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Pierre-Louis Bossart]

On 2/25/16 7:08 PM, Mark Brown wrote:
> On Wed, Feb 24, 2016 at 10:10:05AM -0600, Pierre-Louis Bossart wrote:
>> On 2/23/16 9:32 PM, Mark Brown wrote:
>
>>> That said we really do need x86 to transition to use the clock API
>>> in order to integrate with external devices, where the machine driver
>>> does manage clocks we want that to move to being done using the clock
>>> API rather than custom APIs.
>
>> For Baytrail audio we have a single platform clock that can be turned
>> on/off and set to 19.2 MHz or 25 MHz. No other controls are available,
>> no multipliers or complicated dependencies on other clocks, parents or
>> children and no other users for this clock but the audio subsystem.
>> I looked at the clock framework and couldn't figure out how it would simply
>> map the hardware so for now the use of the MCLK is only enabled with an
>> on/off or set-rate(19.2|25) custom API. I am not an expert here so if this
>> clock framework becomes a requirement to upstream code I would appreciate
>> any pointers to do the right thing. I really couldn't find a simple example
>> with 'Put your code here' comments to use this framework.
>
> There's a lot of helpers like -fixed, -gate and so on - this sounds like
> you could probably use those helpers (there should be lots of examples
> in the kernel) or just implement a simple provider (see clk-provider.h)
> depending on how the control is mapped in.  If it's that simple just
> open coding a provider ought to do the job.  Then set up the client
> linkage with clkdev in some way that makes sense for your platform.

Yes there are documentation and helpers I looked into, but to the best 
of my limited knowledge not for the gate + dual-rate case. Then you jump 
to complicated configurations combining multiple base clocks that don't 
really make sense here. After one frustrating day of trying to wrap 100 
lines of code into the clock framework I gave up and kept the simple 
custom driver. There is a single client for this clock so I don't quite 
see the benefits/ROI of mapping a simple piece of hardware with a 
complex model.

Re: [alsa-devel] [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Fri, Feb 26, 2016 at 09:43:15AM -0600, Pierre-Louis Bossart wrote:

> Yes there are documentation and helpers I looked into, but to the best of my
> limited knowledge not for the gate + dual-rate case. Then you jump to
> complicated configurations combining multiple base clocks that don't really
> make sense here. After one frustrating day of trying to wrap 100 lines of
> code into the clock framework I gave up and kept the simple custom driver.
> There is a single client for this clock so I don't quite see the
> benefits/ROI of mapping a simple piece of hardware with a complex model.

If the helpers aren't fitting your needs then surely just writing a
simple driver with set rate and enable callbacks isn't going to be too
hard?  The wm831x driver was pretty trivial to write and seems to be on
a similar order of complexity to this.

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

Re: [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Rob Herring]

On Tue, Feb 23, 2016 at 09:26:22AM +0800, Sugar Zhang wrote:
> enable/disable master clock when codec is active or not.
> 
> Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
> ---
> 
>  Documentation/devicetree/bindings/sound/rt5640.txt |  3 +++

Acked-by: Rob Herring <robh@kernel.org>

>  sound/soc/codecs/rt5640.c                          | 31 ++++++++++++++++++++++
>  sound/soc/codecs/rt5640.h                          |  2 ++
>  3 files changed, 36 insertions(+)
> 

Re: [PATCH] ASoC: rt5640: add master clock handling for rt5640

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

On Tue, Feb 23, 2016 at 09:26:22AM +0800, Sugar Zhang wrote:
> enable/disable master clock when codec is active or not.

This patch has already been applied, please send incremental patches if
any changes are required.

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