[PATCH 0/2] ASoC: max98090/max98095: Add master clock handing

[PATCH 0/2] ASoC: max98090/max98095: Add master clock handing

[Tushar Behera]

These CODECs are used on Snow/Peach-pit boards and the master clock
needs to be set at 24MHz.

Adding support in respective codec drivers so that we can update the
master clock frequency from sound card driver through set_sysclk.

These patches are prepared as per review comments on following patch.
[PATCH] ASoC: samsung: Add 'mclk' handling for Snow sound-card driver
https://lkml.org/lkml/2014/5/20/4

Tushar Behera (2):
  ASoC: max98090: Add master clock handling
  ASoC: max98095: Add master clock handling

 .../devicetree/bindings/sound/max98090.txt         |    6 ++++++
 .../devicetree/bindings/sound/max98095.txt         |    6 ++++++
 sound/soc/codecs/max98090.c                        |   13 +++++++++++++
 sound/soc/codecs/max98090.h                        |    1 +
 sound/soc/codecs/max98095.c                        |   14 ++++++++++++++
 5 files changed, 40 insertions(+)

-- 
1.7.9.5

[PATCH 1/2] ASoC: max98090: Add master clock handling

[Tushar Behera]

If master clock is provided through device tree, then update
the master clock frequency during set_sysclk.

Documentation has been updated to reflect the change.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 .../devicetree/bindings/sound/max98090.txt         |    6 ++++++
 sound/soc/codecs/max98090.c                        |   13 +++++++++++++
 sound/soc/codecs/max98090.h                        |    1 +
 3 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/max98090.txt b/Documentation/devicetree/bindings/sound/max98090.txt
index e4c8b36..a5e63fa 100644
--- a/Documentation/devicetree/bindings/sound/max98090.txt
+++ b/Documentation/devicetree/bindings/sound/max98090.txt
@@ -10,6 +10,12 @@ Required properties:
 
 - interrupts : The CODEC's interrupt output.
 
+Optional properties:
+
+- clocks: The phandle of the master clock to the CODEC
+
+- clock-names: Should be "mclk"
+
 Pins on the device (for linking into audio routes):
 
   * MIC1
diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
index 9b76f5a..41c61eb 100644
--- a/sound/soc/codecs/max98090.c
+++ b/sound/soc/codecs/max98090.c
@@ -17,6 +17,7 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/clk.h>
 #include <sound/jack.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -1929,6 +1930,11 @@ static int max98090_dai_set_sysclk(struct snd_soc_dai *dai,
 	if (freq == max98090->sysclk)
 		return 0;
 
+	if (!IS_ERR(max98090->mclk)) {
+		freq = clk_round_rate(max98090->mclk, freq);
+		clk_set_rate(max98090->mclk, freq);
+	}
+
 	/* Setup clocks for slave mode, and using the PLL
 	 * PSCLK = 0x01 (when master clk is 10MHz to 20MHz)
 	 *		 0x02 (when master clk is 20MHz to 40MHz)..
@@ -2213,6 +2219,10 @@ static int max98090_probe(struct snd_soc_codec *codec)
 
 	dev_dbg(codec->dev, "max98090_probe\n");
 
+	max98090->mclk = devm_clk_get(codec->dev, "mclk");
+	if (!IS_ERR(max98090->mclk))
+		clk_prepare_enable(max98090->mclk);
+
 	max98090->codec = codec;
 
 	/* Reset the codec, the DSP core, and disable all interrupts */
@@ -2308,6 +2318,9 @@ static int max98090_remove(struct snd_soc_codec *codec)
 
 	cancel_delayed_work_sync(&max98090->jack_work);
 
+	if (!IS_ERR(max98090->mclk))
+		clk_disable_unprepare(max98090->mclk);
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/max98090.h b/sound/soc/codecs/max98090.h
index 5a3c8d0..cf1b606 100644
--- a/sound/soc/codecs/max98090.h
+++ b/sound/soc/codecs/max98090.h
@@ -1524,6 +1524,7 @@ struct max98090_priv {
 	struct snd_soc_codec *codec;
 	enum max98090_type devtype;
 	struct max98090_pdata *pdata;
+	struct clk *mclk;
 	unsigned int sysclk;
 	unsigned int bclk;
 	unsigned int lrclk;
-- 
1.7.9.5

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Mark Brown]

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

On Thu, May 22, 2014 at 02:47:07PM +0530, Tushar Behera wrote:

> +	max98090->mclk = devm_clk_get(codec->dev, "mclk");
> +	if (!IS_ERR(max98090->mclk))
> +		clk_prepare_enable(max98090->mclk);
> +

Ths doesn't handle deferred probe, we need to at least return an error
if we get -EPROBE_DEFER.  

It'd also be better to move the enabling to set_bias_level() if possible
(I don't know if the clock is needed for register access) though less
essential.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Tushar Behera]

On 22 May 2014 16:00, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, May 22, 2014 at 02:47:07PM +0530, Tushar Behera wrote:
>
>> +     max98090->mclk = devm_clk_get(codec->dev, "mclk");
>> +     if (!IS_ERR(max98090->mclk))
>> +             clk_prepare_enable(max98090->mclk);
>> +
>
> Ths doesn't handle deferred probe, we need to at least return an error
> if we get -EPROBE_DEFER.
>

Ok.

> It'd also be better to move the enabling to set_bias_level() if possible
> (I don't know if the clock is needed for register access) though less
> essential.

I tested with moving clk_enable/clk_disable calls to set_bias_level():
SND_SOC_BIAS_PREPARE. That works well for me. Does it look okay?

@@ -1801,6 +1801,25 @@ static int max98090_set_bias_level(struct
snd_soc_codec *codec,
                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(max98090->mclk)) {
+                       if (codec->dapm.bias_level == SND_SOC_BIAS_ON)
+                               clk_disable(max98090->mclk);
+                       else
+                               clk_enable(max98090->mclk);
+               }
                break;

-- 
Tushar Behera
--
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: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Mark Brown]

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

On Fri, May 23, 2014 at 11:05:17AM +0530, Tushar Behera wrote:

> I tested with moving clk_enable/clk_disable calls to set_bias_level():
> SND_SOC_BIAS_PREPARE. That works well for me. Does it look okay?

> +               if (!IS_ERR(max98090->mclk)) {
> +                       if (codec->dapm.bias_level == SND_SOC_BIAS_ON)
> +                               clk_disable(max98090->mclk);
> +                       else
> +                               clk_enable(max98090->mclk);
> +               }
>                 break;

Should be clk_prepare_enable() and similarly for the disable and you
should check the error codes but yes, that looks good.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Tushar Behera]

On 23 May 2014 16:44, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, May 23, 2014 at 11:05:17AM +0530, Tushar Behera wrote:
>
>> I tested with moving clk_enable/clk_disable calls to set_bias_level():
>> SND_SOC_BIAS_PREPARE. That works well for me. Does it look okay?
>
>> +               if (!IS_ERR(max98090->mclk)) {
>> +                       if (codec->dapm.bias_level == SND_SOC_BIAS_ON)
>> +                               clk_disable(max98090->mclk);
>> +                       else
>> +                               clk_enable(max98090->mclk);
>> +               }
>>                 break;
>
> Should be clk_prepare_enable() and similarly for the disable and you
> should check the error codes but yes, that looks good.

I was planning to keep clk_prepare/clk_unprepare in probe/remove.

We are not doing anything specific with the return codes here, so
might be I will add only a error message in case enable/disable fails.

-- 
Tushar Behera
--
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: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Mark Brown]

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

On Fri, May 23, 2014 at 05:06:27PM +0530, Tushar Behera wrote:
> On 23 May 2014 16:44, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> > Should be clk_prepare_enable() and similarly for the disable and you
> > should check the error codes but yes, that looks good.

> I was planning to keep clk_prepare/clk_unprepare in probe/remove.

Why - what purpose would it serve to leave the clock prepared but not
enabled?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Tushar Behera]

On 23 May 2014 17:11, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, May 23, 2014 at 05:06:27PM +0530, Tushar Behera wrote:
>> On 23 May 2014 16:44, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>> > Should be clk_prepare_enable() and similarly for the disable and you
>> > should check the error codes but yes, that looks good.
>
>> I was planning to keep clk_prepare/clk_unprepare in probe/remove.
>
> Why - what purpose would it serve to leave the clock prepared but not
> enabled?

I was getting some kernel slow path warning initially while preparing
this code, but not able to reproduce that warning now.

-- 
Tushar Behera
--
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: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Stephen Warren]

On 05/22/2014 03:17 AM, Tushar Behera wrote:
> If master clock is provided through device tree, then update
> the master clock frequency during set_sysclk.
> 
> Documentation has been updated to reflect the change.

> diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c

> @@ -1929,6 +1930,11 @@ static int max98090_dai_set_sysclk(struct snd_soc_dai *dai,
>  	if (freq == max98090->sysclk)
>  		return 0;
>  
> +	if (!IS_ERR(max98090->mclk)) {
> +		freq = clk_round_rate(max98090->mclk, freq);
> +		clk_set_rate(max98090->mclk, freq);
> +	}

What are the intended semantics of set_sysclk()?
sound/soc/tegra/tegra_wm98090.c assumes that set_sysclk() is a
notification to the CODEC driver to tell it what rate the MCLK input is
set to (the rate is set before calling set_sysclk), whereas the code
above assumes that this function is to tell the CODEC to somehow
configure its input clock to be a particular rate. I have a feeling the
code above might fail on Tegra.

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Mark Brown]

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

On Thu, May 22, 2014 at 09:51:38AM -0600, Stephen Warren wrote:
> On 05/22/2014 03:17 AM, Tushar Behera wrote:

> > +	if (!IS_ERR(max98090->mclk)) {
> > +		freq = clk_round_rate(max98090->mclk, freq);
> > +		clk_set_rate(max98090->mclk, freq);
> > +	}

> What are the intended semantics of set_sysclk()?
> sound/soc/tegra/tegra_wm98090.c assumes that set_sysclk() is a
> notification to the CODEC driver to tell it what rate the MCLK input is
> set to (the rate is set before calling set_sysclk), whereas the code
> above assumes that this function is to tell the CODEC to somehow
> configure its input clock to be a particular rate. I have a feeling the
> code above might fail on Tegra.

It's a bit of both.  Since we've never had a generic clock API (and
still don't really due to everything being a shambles) it's generally
just a notification to the CODEC that it's at the specified rate,
however if the CODEC knows about its dependencies it seems sensible for
it to tell the clock API about what was asked.  Ideally we want to
remove all the ASoC specific clock control and use the clock API.

The Tegra driver will presumably succeed unless someone does the
appropriate clock hookup in DT, at which point clk_set_rate() for the
rate the clock is already at really ought to succeed so things should
continue to work.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Stephen Warren]

On 05/22/2014 11:34 AM, Mark Brown wrote:
> On Thu, May 22, 2014 at 09:51:38AM -0600, Stephen Warren wrote:
>> On 05/22/2014 03:17 AM, Tushar Behera wrote:
> 
>>> +	if (!IS_ERR(max98090->mclk)) {
>>> +		freq = clk_round_rate(max98090->mclk, freq);
>>> +		clk_set_rate(max98090->mclk, freq);
>>> +	}
> 
>> What are the intended semantics of set_sysclk()?
>> sound/soc/tegra/tegra_wm98090.c assumes that set_sysclk() is a
>> notification to the CODEC driver to tell it what rate the MCLK input is
>> set to (the rate is set before calling set_sysclk), whereas the code
>> above assumes that this function is to tell the CODEC to somehow
>> configure its input clock to be a particular rate. I have a feeling the
>> code above might fail on Tegra.
>
> It's a bit of both.  Since we've never had a generic clock API (and
> still don't really due to everything being a shambles) it's generally
> just a notification to the CODEC that it's at the specified rate,
> however if the CODEC knows about its dependencies it seems sensible for
> it to tell the clock API about what was asked.  Ideally we want to
> remove all the ASoC specific clock control and use the clock API.

I think we should nail down exactly what set_sysclk() means. Since it
takes an explicit MCLK clock rate (rather than e.g. sample rate) right
now, surely it's a notification of what the clock /is/, not a request
for the CODEC to set up its input clock. If we expect the CODEC to go to
the clock driver and request an MCLK for itself (e.g. based on sample
rate), surely that should happen from some function besides
set_sysclk(), with different semantics e.g. hw_params().

I'm not convinced that the CODEC can trigger its input clock changes in
general. In Tegra, there needs to be centralized clock management, e.g.
to prevent one audio stream using a 44.1KHz-based rate and another using
a 48KHz-based rate. That's because the main audio PLL can't generate
both sets of frequencies at once. This can't be done in individual CODEC
drivers, since they shouldn't know about the Tegra restrictions. Doing
it in the clock driver in response to the CODEC's request for a specific
input clock, might work, but then the CODEC would have to call into the
clk driver from e.g. hw_params() rather than set_sysclk(). If that's how
it's supposed to work, then this patch is adding code in the wrong
place. If set_sysclk() doesn't get removed from the CODEC API, then
doing all this clock setup logic in the machine driver, as the
tegra_wm89090.c machine driver does, seems to make the most sense for now.

> The Tegra driver will presumably succeed unless someone does the
> appropriate clock hookup in DT, at which point clk_set_rate() for the
> rate the clock is already at really ought to succeed so things should
> continue to work.

Ah yes, if we don't add the clock to DT, this code won't do anything, so
nothing will break.
--
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: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Mark Brown]

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

On Thu, May 22, 2014 at 11:50:55AM -0600, Stephen Warren wrote:

> I think we should nail down exactly what set_sysclk() means. Since it
> takes an explicit MCLK clock rate (rather than e.g. sample rate) right
> now, surely it's a notification of what the clock /is/, not a request
> for the CODEC to set up its input clock. If we expect the CODEC to go to

It really should be that from a device model/clock API point of view and
it certainly is with the patch proposed, the fact that it happens not to
have been is just a product of the poor clock support in Linux than
anything else.

> the clock driver and request an MCLK for itself (e.g. based on sample
> rate), surely that should happen from some function besides
> set_sysclk(), with different semantics e.g. hw_params().

I'm not sure where you see the CODEC going off and deciding the MCLK
rate itself here?  Essentially all that's happening here is that
set_sysclk() is behaving like the clock API does and setting its own
rate to what it was explicitly asked to set, including bouncing that
request up the chain.

The rounding could go if we wanted to be a bit stricter - if the machine
driver is doing a good job it should never change anything - but
otherwise I just can't see what you're concerned about.

> I'm not convinced that the CODEC can trigger its input clock changes in
> general. In Tegra, there needs to be centralized clock management, e.g.
> to prevent one audio stream using a 44.1KHz-based rate and another using
> a 48KHz-based rate. That's because the main audio PLL can't generate
> both sets of frequencies at once. This can't be done in individual CODEC
> drivers, since they shouldn't know about the Tegra restrictions. Doing
> it in the clock driver in response to the CODEC's request for a specific
> input clock, might work, but then the CODEC would have to call into the
> clk driver from e.g. hw_params() rather than set_sysclk(). If that's how
> it's supposed to work, then this patch is adding code in the wrong
> place. If set_sysclk() doesn't get removed from the CODEC API, then
> doing all this clock setup logic in the machine driver, as the
> tegra_wm89090.c machine driver does, seems to make the most sense for now.

What you're describing seems to make things worse not better for your
problem?  I'm sorry, I just can't follow what you're concerned about
here at all.  

The current situation is that the CODEC just gets told what it should
run SYSCLK at, all the patch does really is factor out the code to call
clk_set_rate() on a programmable parent clock and look that clock up in
DT.  Both before and after the patch the CODEC driver takes no decisions
on the rate.  If we started doing things in hw_params() the CODEC driver
would have to start taking decisions since it would only get the sample
rate and so on provided.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Stephen Warren]

On 05/22/2014 12:19 PM, Mark Brown wrote:
> On Thu, May 22, 2014 at 11:50:55AM -0600, Stephen Warren wrote:
> 
>> I think we should nail down exactly what set_sysclk() means. Since it
>> takes an explicit MCLK clock rate (rather than e.g. sample rate) right
>> now, surely it's a notification of what the clock /is/, not a request
>> for the CODEC to set up its input clock. If we expect the CODEC to go to
> 
> It really should be that from a device model/clock API point of view and
> it certainly is with the patch proposed, the fact that it happens not to
> have been is just a product of the poor clock support in Linux than
> anything else.
> 
>> the clock driver and request an MCLK for itself (e.g. based on sample
>> rate), surely that should happen from some function besides
>> set_sysclk(), with different semantics e.g. hw_params().
> 
> I'm not sure where you see the CODEC going off and deciding the MCLK
> rate itself here?  Essentially all that's happening here is that
> set_sysclk() is behaving like the clock API does and setting its own
> rate to what it was explicitly asked to set, including bouncing that
> request up the chain.

If set_sysclk() is never allowed to do anything other than forward the
value it receives to the clock API, I'm much less concerned, althoguh
still a bit.

My main worry is that this patch opens the door for set_sysclk() to
perform some kind of calculation to determine the MCLK rate. Right now,
this patch doesn't do that, but there's nothing obvious from the code
that no CODEC is allowed to do that. After all, sysclk has a parameter
to indicate *which* clock in the CODEC to set. Some CODEC driver author
might write something where the machine driver tells the CODEC driver
the desired rate of some CODEC-internal PLL, from which set_sysclk()
calculates what it needs for MCLK, and then goes off and requests that
value from the clock API.

Ignoring that, I'm still not sure that the CODEC driver setting the MCLK
rate is appropriate. If we have 1 MCLK output from an SoC, connected to
2 different CODECs, why wouldn't the overall machine driver call
clk_set_rate() on that clock, and then notify the two CODEC drivers of
that rate. Making each CODEC's set_sysclk() call clk_set_rate() on the
same clock with the same value seems redundant, albeit it should work
out fine since they both request the same rate.

Re: [PATCH 1/2] ASoC: max98090: Add master clock handling

[Mark Brown]

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

On Thu, May 22, 2014 at 04:24:55PM -0600, Stephen Warren wrote:

> My main worry is that this patch opens the door for set_sysclk() to
> perform some kind of calculation to determine the MCLK rate. Right now,
> this patch doesn't do that, but there's nothing obvious from the code
> that no CODEC is allowed to do that. After all, sysclk has a parameter
> to indicate *which* clock in the CODEC to set. Some CODEC driver author
> might write something where the machine driver tells the CODEC driver
> the desired rate of some CODEC-internal PLL, from which set_sysclk()
> calculates what it needs for MCLK, and then goes off and requests that
> value from the clock API.

I really think you're reading too much into this - the set_sysclk() API
isn't any different to the clock API here really and most of the
potential for doing really problematic stuff and defining your clocks to
mean funny things exists anyway.  Of course people could do tasteless
things but that's always going to be a risk and we also want to try to
minimise the amount of redundant code people have to write.

It should be fairly easy to spot substantial abuse since the driver
would need to call clk_set_rate() with a different rate.

> Ignoring that, I'm still not sure that the CODEC driver setting the MCLK
> rate is appropriate. If we have 1 MCLK output from an SoC, connected to
> 2 different CODECs, why wouldn't the overall machine driver call
> clk_set_rate() on that clock, and then notify the two CODEC drivers of
> that rate. Making each CODEC's set_sysclk() call clk_set_rate() on the
> same clock with the same value seems redundant, albeit it should work
> out fine since they both request the same rate.

Right, it should work fine and it's less work for the machine drivers.
The alternative is that every machine driver using a device with a
programmable input has the code to get that clock from the CODEC device
and set it in sync with telling the CODEC about it which is redundant
and makes life harder for generic drivers like simple card (which
obviously can't know the specifics of every CODEC it works with).

It's certainly a more normal thing from a device model and device tree
point of view for the CODEC to be responsible for getting the resource
and it seems natural to extend that to such basic management as setting
the rate.  

If anything I think want to expose some or all of the CODEC clock trees
to the clock API, sometimes (rarely but not never) the CODEC PLLs and
FLLs are used to supply things outside the audio subsystem if they
happen to be going spare, it's difficult to do that at the minute since
there's no guaranteed API for being a clock provider.  At the minute
we're mostly reimplementing a custom clock API which seems like the
wrong way to be doing things.  That might go towards answering your
concerns since set_sysclk() would end up as a clock API call (or at
least a thin wrapper around one).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 2/2] ASoC: max98095: Add master clock handling

[Tushar Behera]

If master clock is provided through device tree, then update
the master clock frequency during set_sysclk.

Documentation has been updated to reflect the change.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 .../devicetree/bindings/sound/max98095.txt         |    6 ++++++
 sound/soc/codecs/max98095.c                        |   14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/max98095.txt b/Documentation/devicetree/bindings/sound/max98095.txt
index bacbeaa..318a4c8 100644
--- a/Documentation/devicetree/bindings/sound/max98095.txt
+++ b/Documentation/devicetree/bindings/sound/max98095.txt
@@ -8,6 +8,12 @@ Required properties:
 
 - reg : The I2C address of the device.
 
+Optional properties:
+
+- clocks: The phandle of the master clock to the CODEC
+
+- clock-names: Should be "mclk"
+
 Example:
 
 max98095: codec@11 {
diff --git a/sound/soc/codecs/max98095.c b/sound/soc/codecs/max98095.c
index d6c1e4c..4d66b95 100644
--- a/sound/soc/codecs/max98095.c
+++ b/sound/soc/codecs/max98095.c
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/pm.h>
 #include <linux/i2c.h>
+#include <linux/clk.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -42,6 +43,7 @@ struct max98095_priv {
 	struct regmap *regmap;
 	enum max98095_type devtype;
 	struct max98095_pdata *pdata;
+	struct clk *mclk;
 	unsigned int sysclk;
 	struct max98095_cdata dai[3];
 	const char **eq_texts;
@@ -1395,6 +1397,11 @@ static int max98095_dai_set_sysclk(struct snd_soc_dai *dai,
 	if (freq == max98095->sysclk)
 		return 0;
 
+	if (!IS_ERR(max98095->mclk)) {
+		freq = clk_round_rate(max98095->mclk, freq);
+		clk_set_rate(max98095->mclk, freq);
+	}
+
 	/* Setup clocks for slave mode, and using the PLL
 	 * PSCLK = 0x01 (when master clk is 10MHz to 20MHz)
 	 *         0x02 (when master clk is 20MHz to 40MHz)..
@@ -2238,6 +2245,10 @@ static int max98095_probe(struct snd_soc_codec *codec)
 	struct i2c_client *client;
 	int ret = 0;
 
+	max98095->mclk = devm_clk_get(codec->dev, "mclk");
+	if (!IS_ERR(max98095->mclk))
+		clk_prepare_enable(max98095->mclk);
+
 	/* reset the codec, the DSP core, and disable all interrupts */
 	max98095_reset(codec);
 
@@ -2340,6 +2351,9 @@ static int max98095_remove(struct snd_soc_codec *codec)
 	if (max98095->headphone_jack || max98095->mic_jack)
 		max98095_jack_detect_disable(codec);
 
+	if (!IS_ERR(max98095->mclk))
+		clk_disable_unprepare(max98095->mclk);
+
 	if (client->irq)
 		free_irq(client->irq, codec);
 
-- 
1.7.9.5

Re: [PATCH 2/2] ASoC: max98095: Add master clock handling

[Mark Brown]

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

On Thu, May 22, 2014 at 02:47:08PM +0530, Tushar Behera wrote:
> If master clock is provided through device tree, then update
> the master clock frequency during set_sysclk.

Same issues here.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]