[PATCH] ASoC: fsl: enable ssi clock in probe function

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Shawn Guo]

For power saving, most IMX platform initilization code turns off
modules' clock, and expects driver turn on clock as needed.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/fsl/fsl_ssi.c |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 4f76b5d..4ed2afd 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
@@ -119,6 +120,7 @@ struct fsl_ssi_private {
 
 	bool new_binding;
 	bool ssi_on_imx;
+	struct clk *clk;
 	struct platform_device *imx_pcm_pdev;
 	struct imx_pcm_dma_params dma_params_tx;
 	struct imx_pcm_dma_params dma_params_rx;
@@ -722,6 +724,15 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx21-ssi")) {
 		u32 dma_events[2];
 		ssi_private->ssi_on_imx = true;
+
+		ssi_private->clk = clk_get(&pdev->dev, NULL);
+		if (IS_ERR(ssi_private->clk)) {
+			ret = PTR_ERR(ssi_private->clk);
+			dev_err(&pdev->dev, "could not get clock: %d\n", ret);
+			goto error_irq;
+		}
+		clk_prepare_enable(ssi_private->clk);
+
 		/*
 		 * We have burstsize be "fifo_depth - 2" to match the SSI
 		 * watermark setting in fsl_ssi_startup().
@@ -742,7 +753,7 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev)
 					"fsl,ssi-dma-events", dma_events, 2);
 		if (ret) {
 			dev_err(&pdev->dev, "could not get dma events\n");
-			goto error_irq;
+			goto error_clk;
 		}
 		ssi_private->dma_params_tx.dma = dma_events[0];
 		ssi_private->dma_params_rx.dma = dma_events[1];
@@ -830,6 +841,12 @@ error_dev:
 	dev_set_drvdata(&pdev->dev, NULL);
 	device_remove_file(&pdev->dev, dev_attr);
 
+error_clk:
+	if (ssi_private->ssi_on_imx) {
+		clk_disable_unprepare(ssi_private->clk);
+		clk_put(ssi_private->clk);
+	}
+
 error_irq:
 	free_irq(ssi_private->irq, ssi_private);
 
@@ -851,8 +868,11 @@ static int fsl_ssi_remove(struct platform_device *pdev)
 
 	if (!ssi_private->new_binding)
 		platform_device_unregister(ssi_private->pdev);
-	if (ssi_private->ssi_on_imx)
+	if (ssi_private->ssi_on_imx) {
 		platform_device_unregister(ssi_private->imx_pcm_pdev);
+		clk_disable_unprepare(ssi_private->clk);
+		clk_put(ssi_private->clk);
+	}
 	snd_soc_unregister_dai(&pdev->dev);
 	device_remove_file(&pdev->dev, &ssi_private->dev_attr);
 
-- 
1.7.5.4

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Mark Brown]

On Thu, Mar 29, 2012 at 10:53:41AM +0800, Shawn Guo wrote:
> For power saving, most IMX platform initilization code turns off
> modules' clock, and expects driver turn on clock as needed.

Applied, thanks (though checking return codes would be nice).  

Ideally you'd be able to enable the clocks only when needed at runtime
though the power savings would doubtless be very small, pm_runtime
callbacks might be helpful.  Having the clocks present on PowerPC (even
if just dummy) so we don't have the conditional code would also be nice.
-------------- 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/20120329/5c1f2aaf/attachment.sig>

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Timur Tabi]

Mark Brown wrote:
> Applied, thanks (though checking return codes would be nice).  

Speaking of return codes, I can't even get ASoC for-3.5 to compile:

/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
'soc_bind_dai_link':
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:805:11: error:
'EPROBE_DEFER' undeclared (first use in this function)
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:805:11: note: each
undeclared identifier is reported only once for each function it appears in
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
'soc_check_aux_dev':
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:1346:10: error:
'EPROBE_DEFER' undeclared (first use in this function)
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
'soc_probe_aux_dev':
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:1369:10: error:
'EPROBE_DEFER' undeclared (first use in this function)
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
'soc_check_aux_dev':
/home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:1347:1: warning:
control reaches end of non-void function



-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Shawn Guo]

On Thu, Mar 29, 2012 at 11:01:21AM -0500, Timur Tabi wrote:
> Mark Brown wrote:
> > Applied, thanks (though checking return codes would be nice).  
> 
> Speaking of return codes, I can't even get ASoC for-3.5 to compile:
> 
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
> 'soc_bind_dai_link':
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:805:11: error:
> 'EPROBE_DEFER' undeclared (first use in this function)
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:805:11: note: each
> undeclared identifier is reported only once for each function it appears in
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
> 'soc_check_aux_dev':
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:1346:10: error:
> 'EPROBE_DEFER' undeclared (first use in this function)
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
> 'soc_probe_aux_dev':
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:1369:10: error:
> 'EPROBE_DEFER' undeclared (first use in this function)
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
> 'soc_check_aux_dev':
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:1347:1: warning:
> control reaches end of non-void function
> 
Yes.  To build the branch, we have to revert the commit below.

d18b63ab0b04af1bad01e59a96b88fc8cda08752
ASoC: core: Use driver core probe deferral

-- 
Regards,
Shawn

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Timur Tabi]

Shawn Guo wrote:
> d18b63ab0b04af1bad01e59a96b88fc8cda08752
> ASoC: core: Use driver core probe deferral

That fixed it, but the SHA in my repository is
97883ef0a8f61fbd5fb8242980f6998290d1dd95

Anyway, I just want to say that for-3.5 still works on PowerPC.

I do have one question though.  I don't know much about the clock layer.
When you do this:

	ssi_private->clk = clk_get(&pdev->dev, NULL);

What clock are you getting?

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Mark Brown]

On Thu, Mar 29, 2012 at 11:01:21AM -0500, Timur Tabi wrote:
> Mark Brown wrote:
> > Applied, thanks (though checking return codes would be nice).  

> Speaking of return codes, I can't even get ASoC for-3.5 to compile:

> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c: In function
> 'soc_bind_dai_link':
> /home/b04825/git/linux.ssi-clk/sound/soc/soc-core.c:805:11: error:
> 'EPROBE_DEFER' undeclared (first use in this function)

It only works on top of Linus' tree, I'll be rebasing it once -rc1 comes
out.
-------------- 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/20120329/af107ce7/attachment.sig>

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Mark Brown]

On Thu, Mar 29, 2012 at 11:23:42AM -0500, Timur Tabi wrote:

> I do have one question though.  I don't know much about the clock layer.
> When you do this:

> 	ssi_private->clk = clk_get(&pdev->dev, NULL);

> What clock are you getting?

The default clock - personally I much prefer always naming the clocks
even if there's one of them but more people seem to prefer this style.
-------------- 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/20120329/ffaf352d/attachment.sig>

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Shawn Guo]

On Thu, Mar 29, 2012 at 11:23:42AM -0500, Timur Tabi wrote:
> Shawn Guo wrote:
> > d18b63ab0b04af1bad01e59a96b88fc8cda08752
> > ASoC: core: Use driver core probe deferral
> 
> That fixed it, but the SHA in my repository is
> 97883ef0a8f61fbd5fb8242980f6998290d1dd95
> 
> Anyway, I just want to say that for-3.5 still works on PowerPC.
> 
I guess you only meant mpc85xx_defconfig (and mpc85xx_smp_defconfig).
I sent the patch after compile-testing for mpc85xx_defconfig, and
took it for granted that mpc8610_hpcd_defconfig will also compile.
But I just discovered that PPC_CLOCK is not enabled for
mpc8610_hpcd_defconfig, so it fails on the final link.

I just sent a patch proposing to fix it by always building in PPC_CLOCK
for FSL_SOC.

-- 
Regards,
Shawn

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Timur Tabi]

Shawn Guo wrote:
> I guess you only meant mpc85xx_defconfig (and mpc85xx_smp_defconfig).
> I sent the patch after compile-testing for mpc85xx_defconfig, and
> took it for granted that mpc8610_hpcd_defconfig will also compile.
> But I just discovered that PPC_CLOCK is not enabled for
> mpc8610_hpcd_defconfig, so it fails on the final link.

Argh.  I was thinking about testing mpc8610 as well, but I thought, "Hey,
p1022ds works, why bother testing the 8610?  Why would it fail?"

> I just sent a patch proposing to fix it by always building in PPC_CLOCK
> for FSL_SOC.

You'll need to get Kumar Gala's ACK on that one, but if only the 8610 is
broken, then I don't think he'll like it.

-- 
Timur Tabi
Linux kernel developer at Freescale

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Shawn Guo]

On 30 March 2012 21:03, Timur Tabi <timur@freescale.com> wrote:
...
> You'll need to get Kumar Gala's ACK on that one, but if only the 8610 is
> broken, then I don't think he'll like it.
>
It's only about building in arch/powerpc/kernel/clock.c, which is a
pretty dummy clk API and does not take much cost.

Regards,
Shawn

[PATCH] ASoC: fsl: enable ssi clock in probe function

[Timur Tabi]

Shawn Guo wrote:
>> > You'll need to get Kumar Gala's ACK on that one, but if only the 8610 is
>> > broken, then I don't think he'll like it.
>> >
> It's only about building in arch/powerpc/kernel/clock.c, which is a
> pretty dummy clk API and does not take much cost.

Like I said, it needs Kumar's review.  You should ping him on it.  He may
just want the 8610 fixed.

-- 
Timur Tabi
Linux kernel developer at Freescale