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

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

[Jianqun Xu]

Reference to Chapter 3 Clock & Reset Unit (CRU) of RK3288 TRM, i2s0_clkout has
two source clock, XIN24M_DIV2 for 12M and clk_i2s0 for more kinds of
frequencies, I2S0(i2s_clk) has only single source clock clk_i2s0.

i2s0_clkout: generate to output to outside of chip, generally for codec
I2S0(i2s_clk): generate for i2s controller inside of chip

Jianqun Xu (2):
  ASoC: rockchip-i2s: dt: add i2s_clkout to list of clocks
  ASoC: rockchip: i2s: add support for grabbing output clock to codec

 Documentation/devicetree/bindings/sound/rockchip-i2s.txt |  5 +++--
 sound/soc/rockchip/rockchip_i2s.c                        | 11 +++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH 1/2] ASoC: rockchip-i2s: dt: add i2s_clkout to list of clocks

[Jianqun Xu]

Add i2s_clkout property, which enables output clock to chip outside,
this is generally for audio codec outside.

Also add it to example.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 Documentation/devicetree/bindings/sound/rockchip-i2s.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
index 9b82c20..3d23b15 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
@@ -21,6 +21,7 @@ Required properties:
 - clock-names: should contain followings:
    - "i2s_hclk": clock for I2S BUS
    - "i2s_clk" : clock for I2S controller
+   - "i2s_clko" : clock for codec outside of chip
 
 Example for rk3288 I2S controller:
 
@@ -32,6 +33,6 @@ i2s@ff890000 {
 	#size-cells = <0>;
 	dmas = <&pdma1 0>, <&pdma1 1>;
 	dma-names = "tx", "rx";
-	clock-names = "i2s_hclk", "i2s_clk";
-	clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>;
+	clock-names = "i2s_hclk", "i2s_clk", "i2s_clko";
+	clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>, <&cru SCLK_I2S0_CLKOUT>;
 };
-- 
1.9.1

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

[Jianqun Xu]

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>
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
 sound/soc/rockchip/rockchip_i2s.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

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);
-- 
1.9.1

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

[Sonny Rao]

Jay,

On Tue, Nov 18, 2014 at 7:07 PM, Jianqun Xu <jay.xu@rock-chips.com> wrote:

Mostly FYI, but if you take someone else's patch, you should also
retain their authorship by saying:
From: <person>
at the top of the message. I don't really mind, but please keep it in
mind for the future, thanks.

> 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>
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
>  sound/soc/rockchip/rockchip_i2s.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> 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);
> --
> 1.9.1
>

[PATCH v2 1/2] ASoC: rockchip-i2s: dt: add an optional property "i2s_clk_out"

[Jianqun Xu]

Add an property "i2s_clk_out", which enables to output clock to outside
of rockchip SoCs. Let's make it optional since not each board needs it.

Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
---
changes since v1:
- make "i2s_clk_out" optional, suggested by Sonny

 Documentation/devicetree/bindings/sound/rockchip-i2s.txt | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
index 9b82c20..6dedd0d 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
@@ -22,6 +22,10 @@ Required properties:
    - "i2s_hclk": clock for I2S BUS
    - "i2s_clk" : clock for I2S controller
 
+Optional property:
+   - "i2s_clk_out" : clock output to outside of chip, usally as master clock for audio
+   codec chip
+
 Example for rk3288 I2S controller:
 
 i2s@ff890000 {
@@ -32,6 +36,6 @@ i2s@ff890000 {
 	#size-cells = <0>;
 	dmas = <&pdma1 0>, <&pdma1 1>;
 	dma-names = "tx", "rx";
-	clock-names = "i2s_hclk", "i2s_clk";
-	clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>;
+	clock-names = "i2s_hclk", "i2s_clk", "i2s_clk_out";
+	clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>, <&cru SCLK_I2S0_CLKOUT>;
 };
-- 
1.9.1

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

[Jianqun Xu]

Patch is 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>
---
changes since v1:
- modify commit message

 sound/soc/rockchip/rockchip_i2s.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

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);
-- 
1.9.1

Re: [PATCH v2 1/2] ASoC: rockchip-i2s: dt: add an optional property "i2s_clk_out"

[Mark Brown]

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

On Wed, Nov 19, 2014 at 04:07:27PM +0800, Jianqun Xu wrote:
> Add an property "i2s_clk_out", which enables to output clock to outside
> of rockchip SoCs. Let's make it optional since not each board needs it.

Don't send new patches as individual followups to patches in the series,
repost the series.  It becomes very difficult to follow what the current
series is (which is the latest version of everything, what ordering, is
everything still in the current version of the series).

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

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

[Heiko Stübner]

Hi Jianqun,


Am Mittwoch, 19. November 2014, 16:09:27 schrieb Jianqun Xu:
> Patch is from Sonny Rao <sonnyrao@chromium.org>
that line above should read exactly:
From: Sonny Rao <sonnyrao@chromium.org>

that way git will also set the author correctly when importing the patch mbox.

> 
> 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>

I think we you're submitting someone elses patch you'll need a Signed-off-by 
line for yourself too [which marks that you were allowed to submit it etc], so 
it should read something like

Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>


> ---
> changes since v1:
> - modify commit message
> 
>  sound/soc/rockchip/rockchip_i2s.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> 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;
> +	}

blank line here?

> +	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);


Heiko

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

[Kever Yang]

Hi Jay,

On 11/19/2014 04:09 PM, Jianqun Xu wrote:
> Patch is from Sonny Rao <sonnyrao@chromium.org>
Here should be,

From: Sonny Rao <sonnyrao@chromium.org>

- Kever

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

[Mark Brown]

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

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.

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

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

[Doug Anderson]

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

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

[Jianqun]

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);
>> +
> 
> 
> 

-- 
Jianqun Xu

****************************************************************************
*IMPORTANT NOTICE:*This email is from Fuzhou Rockchip Electronics Co.,
Ltd .The contents of this email and any attachments may contain
information that is privileged, confidential and/or exempt from
disclosure under applicable law and relevant NDA. If you are not the
intended recipient, you are hereby notified that any disclosure,
copying, distribution, or use of the information is STRICTLY PROHIBITED.
Please immediately contact the sender as soon as possible and destroy
the material in its entirety in any format. Thank you.
****************************************************************************

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

[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/>


> +       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);
> +

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

[Jianqun Xu]

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(+)

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);
-- 
1.9.1