[PATCH 10/10] S3C64XX I2S: En/Disable codec clocks.

[PATCH 10/10] S3C64XX I2S: En/Disable codec clocks.

[Jassi]

Disable audio-bus when PCLK is used as base clock for I2S controller
codec. Enable otherwise.

Signed-Off-by: Jassi <jassi.brar@samsung.com>
---
 sound/soc/s3c24xx/s3c64xx-i2s.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/s3c24xx/s3c64xx-i2s.c b/sound/soc/s3c24xx/s3c64xx-i2s.c
index 71aeb33..4438f5b 100644
--- a/sound/soc/s3c24xx/s3c64xx-i2s.c
+++ b/sound/soc/s3c24xx/s3c64xx-i2s.c
@@ -107,10 +107,12 @@ static int s3c64xx_i2s_set_sysclk(struct snd_soc_dai *cpu_dai,
 	switch (clk_id) {
 	case S3C64XX_CLKSRC_PCLK:
 		iismod &= ~S3C64XX_IISMOD_IMS_SYSMUX;
+		clk_disable(i2s->iis_cclk);
 		break;
 
 	case S3C64XX_CLKSRC_MUX:
 		iismod |= S3C64XX_IISMOD_IMS_SYSMUX;
+		clk_enable(i2s->iis_cclk);
 		break;
 
 	case S3C64XX_CODCLKSRC_INT:
-- 
1.6.2.5

[PATCH 10/10] S3C64XX I2S: En/Disable codec clocks.

[Mark Brown]

On Tue, Sep 15, 2009 at 07:02:42PM +0900, Jassi wrote:

>  	switch (clk_id) {
>  	case S3C64XX_CLKSRC_PCLK:
>  		iismod &= ~S3C64XX_IISMOD_IMS_SYSMUX;
> +		clk_disable(i2s->iis_cclk);
>  		break;
>  
>  	case S3C64XX_CLKSRC_MUX:
>  		iismod |= S3C64XX_IISMOD_IMS_SYSMUX;
> +		clk_enable(i2s->iis_cclk);
>  		break;

I'd expect that this will cause trouble eventually - machine drivers
tend to call set_sysclk() repeatedly, meaning that the clock will be
repeatedly enabled or disabled.  Eventually this will overflow the
counter the clock API is using, perhaps even cause it to complain loudly
about errors (eg, if the use count drops below zero).  A local reference
count should deal with those issues, or checking to see if iismod
actualy changed before updating the clock API.

Ideally we'd be able to stop the clock while the IIS interface is idle
for a small power saving.

[PATCH 10/10] S3C64XX I2S: En/Disable codec clocks.

[jassi brar]

On Tue, Sep 15, 2009 at 8:49 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Sep 15, 2009 at 07:02:42PM +0900, Jassi wrote:
>
>> ? ? ? switch (clk_id) {
>> ? ? ? case S3C64XX_CLKSRC_PCLK:
>> ? ? ? ? ? ? ? iismod &= ~S3C64XX_IISMOD_IMS_SYSMUX;
>> + ? ? ? ? ? ? clk_disable(i2s->iis_cclk);
>> ? ? ? ? ? ? ? break;
>>
>> ? ? ? case S3C64XX_CLKSRC_MUX:
>> ? ? ? ? ? ? ? iismod |= S3C64XX_IISMOD_IMS_SYSMUX;
>> + ? ? ? ? ? ? clk_enable(i2s->iis_cclk);
>> ? ? ? ? ? ? ? break;
>
> I'd expect that this will cause trouble eventually - machine drivers
> tend to call set_sysclk() repeatedly, meaning that the clock will be
> repeatedly enabled or disabled. ?Eventually this will overflow the
> counter the clock API is using, perhaps even cause it to complain loudly
> about errors (eg, if the use count drops below zero). ?A local reference
> count should deal with those issues, or checking to see if iismod
> actualy changed before updating the clock API.
Yes, this was a bad idea.

[PATCH 10/10] S3C64XX I2S: En/Disable codec clocks.

[Ben Dooks]

On Tue, Sep 15, 2009 at 09:35:48PM +0900, jassi brar wrote:
> On Tue, Sep 15, 2009 at 8:49 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Tue, Sep 15, 2009 at 07:02:42PM +0900, Jassi wrote:
> >
> >> ? ? ? switch (clk_id) {
> >> ? ? ? case S3C64XX_CLKSRC_PCLK:
> >> ? ? ? ? ? ? ? iismod &= ~S3C64XX_IISMOD_IMS_SYSMUX;
> >> + ? ? ? ? ? ? clk_disable(i2s->iis_cclk);
> >> ? ? ? ? ? ? ? break;
> >>
> >> ? ? ? case S3C64XX_CLKSRC_MUX:
> >> ? ? ? ? ? ? ? iismod |= S3C64XX_IISMOD_IMS_SYSMUX;
> >> + ? ? ? ? ? ? clk_enable(i2s->iis_cclk);
> >> ? ? ? ? ? ? ? break;
> >
> > I'd expect that this will cause trouble eventually - machine drivers
> > tend to call set_sysclk() repeatedly, meaning that the clock will be
> > repeatedly enabled or disabled. ?Eventually this will overflow the
> > counter the clock API is using, perhaps even cause it to complain loudly
> > about errors (eg, if the use count drops below zero). ?A local reference
> > count should deal with those issues, or checking to see if iismod
> > actualy changed before updating the clock API.
> Yes, this was a bad idea.

agreed.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20090916/40a97046/attachment.sig>