diff mbox

[2/2] ASoC: rockchip: i2s: add support for grabbing output clock to codec

Message ID 1417531977-30094-1-git-send-email-jay.xu@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

jay.xu@rock-chips.com Dec. 2, 2014, 2:52 p.m. UTC
From: Sonny Rao <sonnyrao@chromium.org>

We need to claim the clock which is driving the codec so that when we enable
clock gating, we continue to clock the codec when needed.  I make this an
optional clock since there might be some applications where we don't need it
but can still use the I2S block.

Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
---
 sound/soc/rockchip/rockchip_i2s.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Doug Anderson Dec. 2, 2014, 5:54 p.m. UTC | #1
Jianqun,

This ought to be a "v3" patch and ideally ought to describe
differences from v2 (after the cut).  Please have Kever or Chris
review your next patch before sending it out since I think they are
familiar with the process.


On Tue, Dec 2, 2014 at 6:52 AM, Jianqun Xu <jay.xu@rock-chips.com> wrote:
> From: Sonny Rao <sonnyrao@chromium.org>
>
> We need to claim the clock which is driving the codec so that when we enable
> clock gating, we continue to clock the codec when needed.  I make this an
> optional clock since there might be some applications where we don't need it
> but can still use the I2S block.
>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>

You still forgot your own signed-off-by.  Please try again.  See
<https://patchwork.kernel.org/patch/5334991/>


> +       i2s->oclk = devm_clk_get(&pdev->dev, "i2s_clk_out");
> +       if (IS_ERR(i2s->oclk)) {
> +               dev_dbg(&pdev->dev, "Didn't find output clock\n");
> +               i2s->oclk = NULL;
> +       }

You still forgot the blank line here requested by Heiko.  Please try
again.  See <https://patchwork.kernel.org/patch/5334991/>


> +       if (i2s->oclk)
> +               ret = clk_prepare_enable(i2s->oclk);
> +
jianqun Dec. 3, 2014, 1:03 a.m. UTC | #2
Hi Doug:

? 12/03/2014 01:54 AM, Doug Anderson ??:
> Jianqun,
> 
> This ought to be a "v3" patch and ideally ought to describe
> differences from v2 (after the cut).  Please have Kever or Chris
> review your next patch before sending it out since I think they are
> familiar with the process.
> 
> 
> On Tue, Dec 2, 2014 at 6:52 AM, Jianqun Xu <jay.xu@rock-chips.com> wrote:
>> From: Sonny Rao <sonnyrao@chromium.org>
>>
>> We need to claim the clock which is driving the codec so that when we enable
>> clock gating, we continue to clock the codec when needed.  I make this an
>> optional clock since there might be some applications where we don't need it
>> but can still use the I2S block.
>>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> 
> You still forgot your own signed-off-by.  Please try again.  See
> <https://patchwork.kernel.org/patch/5334991/>
ok, I will add my signed-off-by
> 
> 
>> +       i2s->oclk = devm_clk_get(&pdev->dev, "i2s_clk_out");
>> +       if (IS_ERR(i2s->oclk)) {
>> +               dev_dbg(&pdev->dev, "Didn't find output clock\n");
>> +               i2s->oclk = NULL;
>> +       }
> 
> You still forgot the blank line here requested by Heiko.  Please try
> again.  See <https://patchwork.kernel.org/patch/5334991/>
Although I thought there needn't a blank ~, I wll add it next patch
> 
> 
>> +       if (i2s->oclk)
>> +               ret = clk_prepare_enable(i2s->oclk);
>> +
> 
> 
>
Doug Anderson Dec. 3, 2014, 5:18 a.m. UTC | #3
On Tue, Dec 2, 2014 at 5:03 PM, Jianqun <xjq@rock-chips.com> wrote:
> Hi Doug:
>
> ? 12/03/2014 01:54 AM, Doug Anderson ??:
>> Jianqun,
>>
>> This ought to be a "v3" patch and ideally ought to describe
>> differences from v2 (after the cut).  Please have Kever or Chris
>> review your next patch before sending it out since I think they are
>> familiar with the process.

You still seem to be missing versions in your subject line and missing
info on what changed version to version.  The latest version you just
sent (v4?) is still missing it.

Unless Mark says so then I don't think you need to spin this series,
but please try to add that in the future.


>> You still forgot the blank line here requested by Heiko.  Please try
>> again.  See <https://patchwork.kernel.org/patch/5334991/>
> Although I thought there needn't a blank ~, I wll add it next patch

That would have been OK, but in that case you should have responded to
Heiko's patch and explained that you weren't taking his feedback (and
why you weren't).

-Doug
Mark Brown Dec. 3, 2014, 12:55 p.m. UTC | #4
On Tue, Dec 02, 2014 at 09:18:35PM -0800, Doug Anderson wrote:

> You still seem to be missing versions in your subject line and missing
> info on what changed version to version.  The latest version you just
> sent (v4?) is still missing it.

> Unless Mark says so then I don't think you need to spin this series,
> but please try to add that in the future.

Don't resubmit.  I actually don't care much about any of that stuff and
mostly ignore it unless the change set is saying something other than
"addressed review comments".

One thing you should avoid doing is posting new versions in reply to old
versions as has been done here, that just makes for a confusing and hard
to follow mail archive with new serieses buried in the middle of threads.
diff mbox

Patch

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index c74ba37..2820ade 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -28,6 +28,7 @@  struct rk_i2s_dev {
 
 	struct clk *hclk;
 	struct clk *mclk;
+	struct clk *oclk;
 
 	struct snd_dmaengine_dai_dma_data capture_dma_data;
 	struct snd_dmaengine_dai_dma_data playback_dma_data;
@@ -439,6 +440,14 @@  static int rockchip_i2s_probe(struct platform_device *pdev)
 		return PTR_ERR(i2s->mclk);
 	}
 
+	i2s->oclk = devm_clk_get(&pdev->dev, "i2s_clk_out");
+	if (IS_ERR(i2s->oclk)) {
+		dev_dbg(&pdev->dev, "Didn't find output clock\n");
+		i2s->oclk = NULL;
+	}
+	if (i2s->oclk)
+		ret = clk_prepare_enable(i2s->oclk);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(regs))
@@ -505,6 +514,8 @@  static int rockchip_i2s_remove(struct platform_device *pdev)
 	if (!pm_runtime_status_suspended(&pdev->dev))
 		i2s_runtime_suspend(&pdev->dev);
 
+	if (i2s->oclk)
+		clk_disable_unprepare(i2s->oclk);
 	clk_disable_unprepare(i2s->mclk);
 	clk_disable_unprepare(i2s->hclk);
 	snd_dmaengine_pcm_unregister(&pdev->dev);