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

2014-12-03 Thread Sonny Rao
On Wed, Dec 3, 2014 at 3:22 PM, Dylan Reid  wrote:
> On Wed, Dec 3, 2014 at 3:03 PM, Sonny Rao  wrote:
>> On Wed, Dec 3, 2014 at 12:03 PM, Mark Brown  wrote:
>>> On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
 On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown  wrote:
>>>
 > I would expect that the clock for the CODEC should be managed by the
 > CODEC if at all possible - that seems more logical than having the CPU
 > I2S controller request and manage it if it's a separate clock.  Why add
 > this to the CPU side driver?
>>>
 This output clock has a mux and can either be a fixed 12Mhz output or
 can be derived from the same fractional divider which drives the i2s
 block.   I thought it was simpler to keep them all the same, but need
 to put ownership in the i2s in anticipation of the i2s driver setting
 it's own clock rate.
>>>
 If you think this is an implementation detail and this output clock
 should just be owned by the codec driver, even though I'm guessing it
 will just have to be the same as i2s, then I think we can drop this
 and make sure simple card (or whatever other codec driver) claims this
 clock.
>>>
>>> simple-card obviously isn't a CODEC driver...
>>
>> Yeah, sorry.
>>
>>> For generality I think
>>> the clock does need to be exposed to the CODEC driver, otherwise this
>>> will work differently to how other systems are working and we can't
>>> substitute in a different clock on the CODEC side so easily if it
>>> doesn't happen to use the output from the I2S block.
>>
>> Ok, then I think what we will do is abandon this patch and I will send
>> something that adds this functionality to the particular codec that
>> I'm interested in -- max98090.
>
> Sorry I didn't read this earlier.  I don't think that this belongs in
> the max98090.  The original patch description is a bit confusing.  The
> clock being grabbed here is actually i2s mclk.  My understanding is
> that, on this SoC, the mclk is driven from a different IP block than
> the rest of the i2s signals.  The i2s driver needs to be told about
> the clock and enable/disable it at the appropriate times.  I'm
> assuming it's optional because there are boards using this SoC with
> i2s slave mode that don't drive mclk at all.
>
> Please correct me if I'm wrong on any of the above.

I don't think you're wrong, and I'm an audio/i2s neophyte so I think
you're probably right and hopefully Mark can confirm that this is how
we want it.

One important thing to point out, which might be causing confusion, is
that this driver is claiming a clock which it internally calls "mclk"
but the way it's specified for rk3288 in the DT, that one is just the
one which drives the internal logic and has a gate.

This clock I'm adding is the actual mclk which is being driven to the
i2s slave device, and it has it's own gate and also has a mux, and we
need to claim both to be able to enable proper clock gating.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-03 Thread Dylan Reid
On Wed, Dec 3, 2014 at 3:03 PM, Sonny Rao  wrote:
> On Wed, Dec 3, 2014 at 12:03 PM, Mark Brown  wrote:
>> On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
>>> On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown  wrote:
>>
>>> > I would expect that the clock for the CODEC should be managed by the
>>> > CODEC if at all possible - that seems more logical than having the CPU
>>> > I2S controller request and manage it if it's a separate clock.  Why add
>>> > this to the CPU side driver?
>>
>>> This output clock has a mux and can either be a fixed 12Mhz output or
>>> can be derived from the same fractional divider which drives the i2s
>>> block.   I thought it was simpler to keep them all the same, but need
>>> to put ownership in the i2s in anticipation of the i2s driver setting
>>> it's own clock rate.
>>
>>> If you think this is an implementation detail and this output clock
>>> should just be owned by the codec driver, even though I'm guessing it
>>> will just have to be the same as i2s, then I think we can drop this
>>> and make sure simple card (or whatever other codec driver) claims this
>>> clock.
>>
>> simple-card obviously isn't a CODEC driver...
>
> Yeah, sorry.
>
>> For generality I think
>> the clock does need to be exposed to the CODEC driver, otherwise this
>> will work differently to how other systems are working and we can't
>> substitute in a different clock on the CODEC side so easily if it
>> doesn't happen to use the output from the I2S block.
>
> Ok, then I think what we will do is abandon this patch and I will send
> something that adds this functionality to the particular codec that
> I'm interested in -- max98090.

Sorry I didn't read this earlier.  I don't think that this belongs in
the max98090.  The original patch description is a bit confusing.  The
clock being grabbed here is actually i2s mclk.  My understanding is
that, on this SoC, the mclk is driven from a different IP block than
the rest of the i2s signals.  The i2s driver needs to be told about
the clock and enable/disable it at the appropriate times.  I'm
assuming it's optional because there are boards using this SoC with
i2s slave mode that don't drive mclk at all.

Please correct me if I'm wrong on any of the above.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-03 Thread Sonny Rao
On Wed, Dec 3, 2014 at 12:03 PM, Mark Brown  wrote:
> On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
>> On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown  wrote:
>
>> > I would expect that the clock for the CODEC should be managed by the
>> > CODEC if at all possible - that seems more logical than having the CPU
>> > I2S controller request and manage it if it's a separate clock.  Why add
>> > this to the CPU side driver?
>
>> This output clock has a mux and can either be a fixed 12Mhz output or
>> can be derived from the same fractional divider which drives the i2s
>> block.   I thought it was simpler to keep them all the same, but need
>> to put ownership in the i2s in anticipation of the i2s driver setting
>> it's own clock rate.
>
>> If you think this is an implementation detail and this output clock
>> should just be owned by the codec driver, even though I'm guessing it
>> will just have to be the same as i2s, then I think we can drop this
>> and make sure simple card (or whatever other codec driver) claims this
>> clock.
>
> simple-card obviously isn't a CODEC driver...

Yeah, sorry.

> For generality I think
> the clock does need to be exposed to the CODEC driver, otherwise this
> will work differently to how other systems are working and we can't
> substitute in a different clock on the CODEC side so easily if it
> doesn't happen to use the output from the I2S block.

Ok, then I think what we will do is abandon this patch and I will send
something that adds this functionality to the particular codec that
I'm interested in -- max98090.

I'm a little tied up at the moment so I'm not going to send that for a
little while, but will come eventually.

Thanks for the advice!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-03 Thread Mark Brown
On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
> On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown  wrote:

> > I would expect that the clock for the CODEC should be managed by the
> > CODEC if at all possible - that seems more logical than having the CPU
> > I2S controller request and manage it if it's a separate clock.  Why add
> > this to the CPU side driver?

> This output clock has a mux and can either be a fixed 12Mhz output or
> can be derived from the same fractional divider which drives the i2s
> block.   I thought it was simpler to keep them all the same, but need
> to put ownership in the i2s in anticipation of the i2s driver setting
> it's own clock rate.

> If you think this is an implementation detail and this output clock
> should just be owned by the codec driver, even though I'm guessing it
> will just have to be the same as i2s, then I think we can drop this
> and make sure simple card (or whatever other codec driver) claims this
> clock.

simple-card obviously isn't a CODEC driver...  For generality I think
the clock does need to be exposed to the CODEC driver, otherwise this
will work differently to how other systems are working and we can't
substitute in a different clock on the CODEC side so easily if it
doesn't happen to use the output from the I2S block.


signature.asc
Description: Digital signature


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

2014-12-03 Thread Sonny Rao
On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown  wrote:
> On Wed, Dec 03, 2014 at 03:18:38PM +0800, Jianqun Xu wrote:
>> From: Sonny Rao 
>>
>> 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.
>
> I would expect that the clock for the CODEC should be managed by the
> CODEC if at all possible - that seems more logical than having the CPU
> I2S controller request and manage it if it's a separate clock.  Why add
> this to the CPU side driver?

It's a good question.  Right now the way I'm running this stuff we're
mostly setting all the i2s the clock rates from the codec driver, but
I think this isn't the correct way to go, and the i2s driver needs to
set it's rate based on the hw params, but that isn't happening (yet).

This output clock has a mux and can either be a fixed 12Mhz output or
can be derived from the same fractional divider which drives the i2s
block.   I thought it was simpler to keep them all the same, but need
to put ownership in the i2s in anticipation of the i2s driver setting
it's own clock rate.

If you think this is an implementation detail and this output clock
should just be owned by the codec driver, even though I'm guessing it
will just have to be the same as i2s, then I think we can drop this
and make sure simple card (or whatever other codec driver) claims this
clock.


>
> We've not always done this for older systems due to the lack of a usable
> clock API but that's starting to be addressed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-03 Thread Mark Brown
On Wed, Dec 03, 2014 at 03:18:38PM +0800, Jianqun Xu wrote:
> From: Sonny Rao 
> 
> 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.

I would expect that the clock for the CODEC should be managed by the
CODEC if at all possible - that seems more logical than having the CPU
I2S controller request and manage it if it's a separate clock.  Why add
this to the CPU side driver?

We've not always done this for older systems due to the lack of a usable
clock API but that's starting to be addressed.


signature.asc
Description: Digital signature


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

2014-12-03 Thread Mark Brown
On Wed, Dec 03, 2014 at 03:18:38PM +0800, Jianqun Xu wrote:
 From: Sonny Rao sonny...@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.

I would expect that the clock for the CODEC should be managed by the
CODEC if at all possible - that seems more logical than having the CPU
I2S controller request and manage it if it's a separate clock.  Why add
this to the CPU side driver?

We've not always done this for older systems due to the lack of a usable
clock API but that's starting to be addressed.


signature.asc
Description: Digital signature


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

2014-12-03 Thread Sonny Rao
On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown broo...@kernel.org wrote:
 On Wed, Dec 03, 2014 at 03:18:38PM +0800, Jianqun Xu wrote:
 From: Sonny Rao sonny...@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.

 I would expect that the clock for the CODEC should be managed by the
 CODEC if at all possible - that seems more logical than having the CPU
 I2S controller request and manage it if it's a separate clock.  Why add
 this to the CPU side driver?

It's a good question.  Right now the way I'm running this stuff we're
mostly setting all the i2s the clock rates from the codec driver, but
I think this isn't the correct way to go, and the i2s driver needs to
set it's rate based on the hw params, but that isn't happening (yet).

This output clock has a mux and can either be a fixed 12Mhz output or
can be derived from the same fractional divider which drives the i2s
block.   I thought it was simpler to keep them all the same, but need
to put ownership in the i2s in anticipation of the i2s driver setting
it's own clock rate.

If you think this is an implementation detail and this output clock
should just be owned by the codec driver, even though I'm guessing it
will just have to be the same as i2s, then I think we can drop this
and make sure simple card (or whatever other codec driver) claims this
clock.



 We've not always done this for older systems due to the lack of a usable
 clock API but that's starting to be addressed.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-03 Thread Mark Brown
On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
 On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown broo...@kernel.org wrote:

  I would expect that the clock for the CODEC should be managed by the
  CODEC if at all possible - that seems more logical than having the CPU
  I2S controller request and manage it if it's a separate clock.  Why add
  this to the CPU side driver?

 This output clock has a mux and can either be a fixed 12Mhz output or
 can be derived from the same fractional divider which drives the i2s
 block.   I thought it was simpler to keep them all the same, but need
 to put ownership in the i2s in anticipation of the i2s driver setting
 it's own clock rate.

 If you think this is an implementation detail and this output clock
 should just be owned by the codec driver, even though I'm guessing it
 will just have to be the same as i2s, then I think we can drop this
 and make sure simple card (or whatever other codec driver) claims this
 clock.

simple-card obviously isn't a CODEC driver...  For generality I think
the clock does need to be exposed to the CODEC driver, otherwise this
will work differently to how other systems are working and we can't
substitute in a different clock on the CODEC side so easily if it
doesn't happen to use the output from the I2S block.


signature.asc
Description: Digital signature


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

2014-12-03 Thread Sonny Rao
On Wed, Dec 3, 2014 at 12:03 PM, Mark Brown broo...@kernel.org wrote:
 On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
 On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown broo...@kernel.org wrote:

  I would expect that the clock for the CODEC should be managed by the
  CODEC if at all possible - that seems more logical than having the CPU
  I2S controller request and manage it if it's a separate clock.  Why add
  this to the CPU side driver?

 This output clock has a mux and can either be a fixed 12Mhz output or
 can be derived from the same fractional divider which drives the i2s
 block.   I thought it was simpler to keep them all the same, but need
 to put ownership in the i2s in anticipation of the i2s driver setting
 it's own clock rate.

 If you think this is an implementation detail and this output clock
 should just be owned by the codec driver, even though I'm guessing it
 will just have to be the same as i2s, then I think we can drop this
 and make sure simple card (or whatever other codec driver) claims this
 clock.

 simple-card obviously isn't a CODEC driver...

Yeah, sorry.

 For generality I think
 the clock does need to be exposed to the CODEC driver, otherwise this
 will work differently to how other systems are working and we can't
 substitute in a different clock on the CODEC side so easily if it
 doesn't happen to use the output from the I2S block.

Ok, then I think what we will do is abandon this patch and I will send
something that adds this functionality to the particular codec that
I'm interested in -- max98090.

I'm a little tied up at the moment so I'm not going to send that for a
little while, but will come eventually.

Thanks for the advice!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-03 Thread Dylan Reid
On Wed, Dec 3, 2014 at 3:03 PM, Sonny Rao sonny...@chromium.org wrote:
 On Wed, Dec 3, 2014 at 12:03 PM, Mark Brown broo...@kernel.org wrote:
 On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
 On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown broo...@kernel.org wrote:

  I would expect that the clock for the CODEC should be managed by the
  CODEC if at all possible - that seems more logical than having the CPU
  I2S controller request and manage it if it's a separate clock.  Why add
  this to the CPU side driver?

 This output clock has a mux and can either be a fixed 12Mhz output or
 can be derived from the same fractional divider which drives the i2s
 block.   I thought it was simpler to keep them all the same, but need
 to put ownership in the i2s in anticipation of the i2s driver setting
 it's own clock rate.

 If you think this is an implementation detail and this output clock
 should just be owned by the codec driver, even though I'm guessing it
 will just have to be the same as i2s, then I think we can drop this
 and make sure simple card (or whatever other codec driver) claims this
 clock.

 simple-card obviously isn't a CODEC driver...

 Yeah, sorry.

 For generality I think
 the clock does need to be exposed to the CODEC driver, otherwise this
 will work differently to how other systems are working and we can't
 substitute in a different clock on the CODEC side so easily if it
 doesn't happen to use the output from the I2S block.

 Ok, then I think what we will do is abandon this patch and I will send
 something that adds this functionality to the particular codec that
 I'm interested in -- max98090.

Sorry I didn't read this earlier.  I don't think that this belongs in
the max98090.  The original patch description is a bit confusing.  The
clock being grabbed here is actually i2s mclk.  My understanding is
that, on this SoC, the mclk is driven from a different IP block than
the rest of the i2s signals.  The i2s driver needs to be told about
the clock and enable/disable it at the appropriate times.  I'm
assuming it's optional because there are boards using this SoC with
i2s slave mode that don't drive mclk at all.

Please correct me if I'm wrong on any of the above.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-03 Thread Sonny Rao
On Wed, Dec 3, 2014 at 3:22 PM, Dylan Reid dgr...@chromium.org wrote:
 On Wed, Dec 3, 2014 at 3:03 PM, Sonny Rao sonny...@chromium.org wrote:
 On Wed, Dec 3, 2014 at 12:03 PM, Mark Brown broo...@kernel.org wrote:
 On Wed, Dec 03, 2014 at 11:38:13AM -0800, Sonny Rao wrote:
 On Wed, Dec 3, 2014 at 11:20 AM, Mark Brown broo...@kernel.org wrote:

  I would expect that the clock for the CODEC should be managed by the
  CODEC if at all possible - that seems more logical than having the CPU
  I2S controller request and manage it if it's a separate clock.  Why add
  this to the CPU side driver?

 This output clock has a mux and can either be a fixed 12Mhz output or
 can be derived from the same fractional divider which drives the i2s
 block.   I thought it was simpler to keep them all the same, but need
 to put ownership in the i2s in anticipation of the i2s driver setting
 it's own clock rate.

 If you think this is an implementation detail and this output clock
 should just be owned by the codec driver, even though I'm guessing it
 will just have to be the same as i2s, then I think we can drop this
 and make sure simple card (or whatever other codec driver) claims this
 clock.

 simple-card obviously isn't a CODEC driver...

 Yeah, sorry.

 For generality I think
 the clock does need to be exposed to the CODEC driver, otherwise this
 will work differently to how other systems are working and we can't
 substitute in a different clock on the CODEC side so easily if it
 doesn't happen to use the output from the I2S block.

 Ok, then I think what we will do is abandon this patch and I will send
 something that adds this functionality to the particular codec that
 I'm interested in -- max98090.

 Sorry I didn't read this earlier.  I don't think that this belongs in
 the max98090.  The original patch description is a bit confusing.  The
 clock being grabbed here is actually i2s mclk.  My understanding is
 that, on this SoC, the mclk is driven from a different IP block than
 the rest of the i2s signals.  The i2s driver needs to be told about
 the clock and enable/disable it at the appropriate times.  I'm
 assuming it's optional because there are boards using this SoC with
 i2s slave mode that don't drive mclk at all.

 Please correct me if I'm wrong on any of the above.

I don't think you're wrong, and I'm an audio/i2s neophyte so I think
you're probably right and hopefully Mark can confirm that this is how
we want it.

One important thing to point out, which might be causing confusion, is
that this driver is claiming a clock which it internally calls mclk
but the way it's specified for rk3288 in the DT, that one is just the
one which drives the internal logic and has a gate.

This clock I'm adding is the actual mclk which is being driven to the
i2s slave device, and it has it's own gate and also has a mux, and we
need to claim both to be able to enable proper clock gating.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-02 Thread Jianqun Xu
From: Sonny Rao 

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 
Signed-off-by: Jianqun Xu 
---
 sound/soc/rockchip/rockchip_i2s.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_i2s.c 
b/sound/soc/rockchip/rockchip_i2s.c
index c74ba37..fd144ac 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,15 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
return PTR_ERR(i2s->mclk);
}
 
+   i2s->oclk = devm_clk_get(>dev, "i2s_clk_out");
+   if (IS_ERR(i2s->oclk)) {
+   dev_dbg(>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(>dev, res);
if (IS_ERR(regs))
@@ -505,6 +515,9 @@ static int rockchip_i2s_remove(struct platform_device *pdev)
if (!pm_runtime_status_suspended(>dev))
i2s_runtime_suspend(>dev);
 
+   if (i2s->oclk)
+   clk_disable_unprepare(i2s->oclk);
+
clk_disable_unprepare(i2s->mclk);
clk_disable_unprepare(i2s->hclk);
snd_dmaengine_pcm_unregister(>dev);
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-12-02 Thread Jianqun Xu
From: Sonny Rao sonny...@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 sonny...@chromium.org
Signed-off-by: Jianqun Xu jay...@rock-chips.com
---
 sound/soc/rockchip/rockchip_i2s.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_i2s.c 
b/sound/soc/rockchip/rockchip_i2s.c
index c74ba37..fd144ac 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,15 @@ 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 +515,9 @@ 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


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-11-19 Thread Kever Yang

Hi Jay,

On 11/19/2014 04:09 PM, Jianqun Xu wrote:

Patch is from Sonny Rao 

Here should be,

From: Sonny Rao 

- Kever

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-11-19 Thread Heiko Stübner
Hi Jianqun,


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

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 

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 
Signed-off-by: Jianqun Xu 


> ---
> 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(>dev, "i2s_clk_out");
> + if (IS_ERR(i2s->oclk)) {
> + dev_dbg(>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(>dev, res);
>   if (IS_ERR(regs))
> @@ -505,6 +514,8 @@ static int rockchip_i2s_remove(struct platform_device
> *pdev) if (!pm_runtime_status_suspended(>dev))
>   i2s_runtime_suspend(>dev);
> 
> + if (i2s->oclk)
> + clk_disable_unprepare(i2s->oclk);
>   clk_disable_unprepare(i2s->mclk);
>   clk_disable_unprepare(i2s->hclk);
>   snd_dmaengine_pcm_unregister(>dev);


Heiko

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-11-19 Thread Jianqun Xu
Patch is from Sonny Rao 

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 
---
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(>dev, "i2s_clk_out");
+   if (IS_ERR(i2s->oclk)) {
+   dev_dbg(>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(>dev, res);
if (IS_ERR(regs))
@@ -505,6 +514,8 @@ static int rockchip_i2s_remove(struct platform_device *pdev)
if (!pm_runtime_status_suspended(>dev))
i2s_runtime_suspend(>dev);
 
+   if (i2s->oclk)
+   clk_disable_unprepare(i2s->oclk);
clk_disable_unprepare(i2s->mclk);
clk_disable_unprepare(i2s->hclk);
snd_dmaengine_pcm_unregister(>dev);
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-11-19 Thread Jianqun Xu
Patch is from Sonny Rao sonny...@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 sonny...@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


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-11-19 Thread Heiko Stübner
Hi Jianqun,


Am Mittwoch, 19. November 2014, 16:09:27 schrieb Jianqun Xu:
 Patch is from Sonny Rao sonny...@chromium.org
that line above should read exactly:
From: Sonny Rao sonny...@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 sonny...@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 sonny...@chromium.org
Signed-off-by: Jianqun Xu jay...@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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2014-11-19 Thread Kever Yang

Hi Jay,

On 11/19/2014 04:09 PM, Jianqun Xu wrote:

Patch is from Sonny Rao sonny...@chromium.org

Here should be,

From: Sonny Rao sonny...@chromium.org

- Kever

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/