At Thu, 5 Jun 2008 09:54:06 +0100, Mark Brown wrote:
The WM8990 is a highly integrated ultra-low power hi-fi codec designed for handsets rich in multimedia features such as mobile TV, digital audio playback and gaming.
The bulk of this driver was written by Liam Girdwood with some additional development and updates for new ASoC APIs by me.
Signed-off-by: Liam Girdwood lg@opensource.wolfsonmicro.com Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com
Hm, I obviously didn't review it in the last cycle.
+#include <linux/kernel.h>
Heh, the same issue as wm8510.
+/*
- wm8990 register cache. Note that register 0 is not included in the
- cache.
- */
+static const u16 wm8990_reg[] = {
- /*0x8990,*/ /* R0 - Reset */
Ditto.
+/*
- read wm8990 register cache
- */
+static inline unsigned int wm8990_read_reg_cache(struct snd_soc_codec *codec,
- unsigned int reg)
+{
- u16 *cache = codec->reg_cache;
- BUG_ON(reg < 1 || reg > (ARRAY_SIZE(wm8990_reg) + 1));
Isn't it BUG_ON(reg < 1 || reg > ARRAY_SIZE(wm8990_reg)); ??
But at the first place this is bad, because codec_reg_show() in soc-core.c always loops from 0 to codec->reg_cache_size-1. At least, reg == 0 should be simply ignored.
- return cache[reg - 1];
+}
+/*
- write wm8990 register cache
- */
+static inline void wm8990_write_reg_cache(struct snd_soc_codec *codec,
- unsigned int reg, unsigned int value)
+{
- u16 *cache = codec->reg_cache;
- BUG_ON(reg < 1 || reg > (ARRAY_SIZE(wm8990_reg) + 1));
Ditto.
+static int wm8990_set_bias_level(struct snd_soc_codec *codec,
- enum snd_soc_bias_level level)
(snip)
- case SND_SOC_BIAS_STANDBY:
if (codec->bias_level == SND_SOC_BIAS_OFF) {/* Enable all output discharge bits */wm8990_write(codec, WM8990_ANTIPOP1, WM8990_DIS_LLINE |WM8990_DIS_RLINE | WM8990_DIS_OUT3 |WM8990_DIS_OUT4 | WM8990_DIS_LOUT |WM8990_DIS_ROUT);/* Enable POBCTRL, SOFT_ST, VMIDTOG and BUFDCOPEN */wm8990_write(codec, WM8990_ANTIPOP2, WM8990_SOFTST |WM8990_BUFDCOPEN | WM8990_POBCTRL |WM8990_VMIDTOG);/* Delay to allow output caps to discharge */schedule_timeout_interruptible(msecs_to_jiffies(300));
Why interruptible? Maybe msleep() is better for such a purpose?
/* Disable VMIDTOG */wm8990_write(codec, WM8990_ANTIPOP2, WM8990_SOFTST |WM8990_BUFDCOPEN | WM8990_POBCTRL);/* disable all output discharge bits */wm8990_write(codec, WM8990_ANTIPOP1, 0);/* Enable outputs */wm8990_write(codec, WM8990_POWER_MANAGEMENT_1, 0x1b00);schedule_timeout_interruptible(msecs_to_jiffies(50));
Ditto.
/* Enable VMID at 2x50k */wm8990_write(codec, WM8990_POWER_MANAGEMENT_1, 0x1f02);schedule_timeout_interruptible(msecs_to_jiffies(100));/* Enable VREF */wm8990_write(codec, WM8990_POWER_MANAGEMENT_1, 0x1f03);schedule_timeout_interruptible(msecs_to_jiffies(600));/* Enable BUFIOEN */wm8990_write(codec, WM8990_ANTIPOP2, WM8990_SOFTST |WM8990_BUFDCOPEN | WM8990_POBCTRL |WM8990_BUFIOEN);/* Disable outputs */wm8990_write(codec, WM8990_POWER_MANAGEMENT_1, 0x3);/* disable POBCTRL, SOFT_ST and BUFDCOPEN */wm8990_write(codec, WM8990_ANTIPOP2, WM8990_BUFIOEN);} else {/* ON -> standby */}break;- case SND_SOC_BIAS_OFF:
/* Enable POBCTRL and SOFT_ST */wm8990_write(codec, WM8990_ANTIPOP2, WM8990_SOFTST |WM8990_POBCTRL | WM8990_BUFIOEN);/* Enable POBCTRL, SOFT_ST and BUFDCOPEN */wm8990_write(codec, WM8990_ANTIPOP2, WM8990_SOFTST |WM8990_BUFDCOPEN | WM8990_POBCTRL |WM8990_BUFIOEN);/* mute DAC */val = wm8990_read_reg_cache(codec, WM8990_DAC_CTRL);wm8990_write(codec, WM8990_DAC_CTRL, val | WM8990_DAC_MUTE);/* Enable any disabled outputs */wm8990_write(codec, WM8990_POWER_MANAGEMENT_1, 0x1f03);/* Disable VMID */wm8990_write(codec, WM8990_POWER_MANAGEMENT_1, 0x1f01);schedule_timeout_interruptible(msecs_to_jiffies(300));
Ditto.
+static int wm8990_init(struct snd_soc_device *socdev) +{
- struct snd_soc_codec *codec = socdev->codec;
- u16 reg;
- int ret = 0;
- codec->name = "WM8990";
- codec->owner = THIS_MODULE;
- codec->read = wm8990_read_reg_cache;
- codec->write = wm8990_write;
- codec->set_bias_level = wm8990_set_bias_level;
- codec->dai = &wm8990_dai;
- codec->num_dai = 2;
- codec->reg_cache_size = sizeof(wm8990_reg);
A wrong value assignment in this file, too. But, ARRAY_SIZE() is also wrong since the max reg value is ARRAY_SIZE(). ARRAY_SIZE()+1 is the right value, but then it's really unclear what it means...
thanks,
Takashi