[alsa-devel] [PATCH 1/3] asoc tlv320aic3x: revisit clock setup
This patch cleans up the clocking setup for aic3x codecs. It drops the dividers table and determines the PLL control values programatically. Under certain conditions, the PLL is disabled entirely which could save some power.
Signed-off-by: Daniel Mack daniel@caiaq.de
At Fri, 25 Apr 2008 11:17:07 +0200, Daniel Mack wrote:
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 630684f..87e5040 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -51,6 +51,8 @@ #define AUDIO_NAME "aic3x" #define AIC3X_VERSION "0.1"
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
We already have a standard abs() in linux/kernel.h.
thanks,
Takashi
On Fri, Apr 25, 2008 at 11:28:44AM +0200, Takashi Iwai wrote:
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 630684f..87e5040 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -51,6 +51,8 @@ #define AUDIO_NAME "aic3x" #define AIC3X_VERSION "0.1"
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
We already have a standard abs() in linux/kernel.h.
Ok, thanks for mentioning that. See the patch below. Also bumped the version number here.
Best regards, Daniel
At Fri, 25 Apr 2008 11:36:57 +0200, Daniel Mack wrote:
On Fri, Apr 25, 2008 at 11:28:44AM +0200, Takashi Iwai wrote:
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 630684f..87e5040 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -51,6 +51,8 @@ #define AUDIO_NAME "aic3x" #define AIC3X_VERSION "0.1"
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
We already have a standard abs() in linux/kernel.h.
Ok, thanks for mentioning that. See the patch below. Also bumped the version number here.
You don't need to include <linux/kernel.h> there. It's almost always included.
Takashi
2008/4/25 Daniel Mack daniel@caiaq.org:
On Fri, Apr 25, 2008 at 11:28:44AM +0200, Takashi Iwai wrote:
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
We already have a standard abs() in linux/kernel.h.
Ok, thanks for mentioning that. See the patch below. Also bumped the version number here.
Did you forgot to take into account doubled fsref below (from the first
version) when calculating codec_clk because now it is doubled earlier?
+ codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000); + + if (params_rate(params) >= 88200) + fsref *= 2;
There should not be dead code like
+ /* printk(KERN_INFO "%s(): bypassing PLL with Q=%d\n", + __func__, pll_q); */
I forgot to mention in my previous comment that KERN_DEBUG would be better here or just remove the printk.
Jarkko
On Fri, Apr 25, 2008 at 12:55:23PM +0300, Jarkko Nikula wrote:
Did you forgot to take into account doubled fsref below (from the first version) when calculating codec_clk because now it is doubled earlier?
Oops, yes, you're right. Sorry for that.
/* printk(KERN_INFO "%s(): bypassing PLL with Q=%d\n",__func__, pll_q); */I forgot to mention in my previous comment that KERN_DEBUG would be better here or just remove the printk.
Ok, did some error handling with a printk() instead.
Daniel
On Fri, Apr 25, 2008 at 1:18 PM, Daniel Mack daniel@caiaq.org wrote:
On Fri, Apr 25, 2008 at 12:55:23PM +0300, Jarkko Nikula wrote:
Did you forgot to take into account doubled fsref below (from the first version) when calculating codec_clk because now it is doubled earlier?
Oops, yes, you're right. Sorry for that.
Doesn't work for 64000 kHz since 48000 * 10 / 64000 = 7.5 and later
calculations => 7*2 / 5 - 2 = 2 - 2 but this might to work:
... data = (fsref * 20) / rate; if (rate < 64000) data /= 2; data /= 5; ...
Jarkko
On Fri, Apr 25, 2008 at 1:47 PM, Daniel Mack daniel@caiaq.org wrote:
On Fri, Apr 25, 2008 at 01:40:42PM +0300, Jarkko Nikula wrote:
Doesn't work for 64000 kHz since 48000 * 10 / 64000 = 7.5 and later
calculations => 7*2 / 5 - 2 = 2 - 2 but this might to work:
Good catch, thanks.
This dead code below was there but otherwise I think this is good now.
+ /* printk(KERN_INFO "%s(): bypassing PLL with Q=%d\n", + __func__, pll_q); */
Jarkko
On Fri, Apr 25, 2008 at 3:58 PM, Daniel Mack daniel@caiaq.org wrote:
On Fri, Apr 25, 2008 at 02:30:29PM +0300, Jarkko Nikula wrote:
This dead code below was there but otherwise I think this is good now.
/* printk(KERN_INFO "%s(): bypassing PLL with Q=%d\n",__func__, pll_q); */Oh well. Last shot now, I hope ;)
Acked-by: Jarkko Nikula jarkko.nikula@nokia.com
participants (3)
-
Daniel Mack -
Jarkko Nikula -
Takashi Iwai