[PATCH] ASoC: Ensure we delay long enough for WM8994 FLL to lock when starting

[PATCH] ASoC: Ensure we delay long enough for WM8994 FLL to lock when starting

[Mark Brown]

This delay is very conservative.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm8994.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index 38abdff..2eb4594 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -1725,6 +1725,8 @@ static int _wm8994_set_fll(struct snd_soc_codec *codec, int id, int src,
 		snd_soc_update_bits(codec, WM8994_FLL1_CONTROL_1 + reg_offset,
 				    WM8994_FLL1_ENA | WM8994_FLL1_FRAC,
 				    reg);
+
+		msleep(5);
 	}
 
 	wm8994->fll[id].in = freq_in;
-- 
1.7.5.4

Re: [PATCH] ASoC: Ensure we delay long enough for WM8994 FLL to lock when starting

[Sangbeom Kim]

Hi, Mark
Saturday, July 02, 2011 9:35 AM, Mark Brown wrote:

> This delay is very conservative.

What's the meaning of conservative?
It means 5ms delay long enough? 
or at least, FLL lock time should be ensured 5ms?

Thanks,
SB Kim

Re: [PATCH] ASoC: Ensure we delay long enough for WM8994 FLL to lock when starting

[Mark Brown]

On Mon, Jul 04, 2011 at 08:59:45AM +0900, Sangbeom Kim wrote:
> Saturday, July 02, 2011 9:35 AM, Mark Brown wrote:

> > This delay is very conservative.

> What's the meaning of conservative?

In this context it means safe.

> It means 5ms delay long enough? 
> or at least, FLL lock time should be ensured 5ms?

Yes, the 5ms should be more than enough time for the FLL to lock - in
most situations the time taken to do the register writes surrounding
enabling and then using the FLL are sufficient to cover the startup
time.

Re: [PATCH] ASoC: Ensure we delay long enough for WM8994 FLL to lock when starting

[Sangbeom Kim]

On Mon, Jul 04, 2011 at 09:45 AM +0900, Mark Brown wrote:
> > > This delay is very conservative.
> 
> > What's the meaning of conservative?
> 
> In this context it means safe.
> 
> > It means 5ms delay long enough?
> > or at least, FLL lock time should be ensured 5ms?
> 
> Yes, the 5ms should be more than enough time for the FLL to lock - in
> most situations the time taken to do the register writes surrounding
> enabling and then using the FLL are sufficient to cover the startup
> time.

OK, 
I have a one more question.
Is there any checking method for locking FLL?
(ex. Checking register bit for FLL lock)
Just using msleep looks not enough to ensure FLL lock.

Thanks,
SB Kim

Re: [PATCH] ASoC: Ensure we delay long enough for WM8994 FLL to lock when starting

[Mark Brown]

On Mon, Jul 04, 2011 at 10:13:56AM +0900, Sangbeom Kim wrote:
> Is there any checking method for locking FLL?
> (ex. Checking register bit for FLL lock)

There is an interrupt which I've got a patch to use (it's in need of a
bit more testing so not yet published).  You can also poll the status
bits but on a lot of systems that works out as being too expensive due
to I/O costs.

> Just using msleep looks not enough to ensure FLL lock.

Practically speaking it's fine; the FLL lock time is specified
performance characteristic of the device.  If we can't lock when we
expect to then the system is in trouble and it's not clear what we can
do about usefully. 

Re: [PATCH] ASoC: Ensure we delay long enough for WM8994 FLL to lock when starting

[Liam Girdwood]

On 02/07/11 01:35, Mark Brown wrote:
> This delay is very conservative.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  sound/soc/codecs/wm8994.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
> index 38abdff..2eb4594 100644
> --- a/sound/soc/codecs/wm8994.c
> +++ b/sound/soc/codecs/wm8994.c
> @@ -1725,6 +1725,8 @@ static int _wm8994_set_fll(struct snd_soc_codec *codec, int id, int src,
>  		snd_soc_update_bits(codec, WM8994_FLL1_CONTROL_1 + reg_offset,
>  				    WM8994_FLL1_ENA | WM8994_FLL1_FRAC,
>  				    reg);
> +
> +		msleep(5);
>  	}
>  
>  	wm8994->fll[id].in = freq_in;

Acked-by: Liam Girdwood <lrg@ti.com>