Hi Mark,
>>>> +static u16 wm8940_reg_defaults[] = {
>>>> + [WM8940_SOFTRESET] = 0x8940,
>>>> + [WM8940_POWER1] = 0x0000,
>
>>> I'd really rather use the more standard ASoC style here with a simple
>>> table of values. The driver relies on the fact that the register
>>> cache is fully specified for suspend and resume and having a straight
>>> table makes sure that there aren't any missing values in the cache.
>
>> Ok, though I'd place the alternate argument that it lead to rather
>> less readable code than this sort of syntax where it readily apparent
>> exactly what is in each register.
>
> Sure - if you do this using an alternative syntax that'd be fine. For
> example, several of the other drivers use comments to provide the
> register names.
Yup, that's what I've moved over to.
>
>>>> + SOC_SINGLE("Digital Loopback Switch adc to dac", WM8940_COMPANDINGCTL,
>>>> + 0, 1, 0),
>
>>> This should be called Digital Sidetone Switch and probably ought to be a
>>> DAPM control - there's an ADC to DAC route.
>
>> I'll take your word for it!
>
> If you switch the ADC to the DAC then you've got an audio path between
> them.
Got that. It was the fact it was a Sidetone Switch that I was surprised by!
>
>>>> + SOC_SINGLE_TLV("Speaker Playback Volume", WM8940_SPKVOL,
>>>> + 0, 63, 0, wm8940_spk_vol_tlv),
>>>> + SOC_SINGLE("Speaker Playback Mute", WM8940_SPKVOL, 6, 1, 0),
>
>>> Speaker Playback Switch; this also means that the sense of the control
>>> needs to be inverted. Look at the control in alsamixer and you'll see
>>> that it figures out that these both control the same thing and displays
>>> them as a single Speaker Playback control in the UI.
>
>> I wish, alsamixer isn't exactly playing ball with busy box for some reason
>> I haven't chased down. Looking at this lot using amixer isn't much fun!
>
> amixer should show you the same information - yo'll get a simple mixer
> control with both switch and volume capabilites.
It does indeed, but there are so many controls it is a pain to find the one
you want making checking this lot rather time consuming.
>> I'll try and work out how to do it right before next posting! Need to lure
>> the board side of things into a configuration where this is relevant.
>
> If you've got a scope you can use that to generate clocks with broken
> audio.
Yup, fun and games to come.
>
>>>> + case SND_SOC_BIAS_PREPARE:
>>>> + /* ensure bufioen and biasen */
>>>> + pwr_reg |= (1 << 2) | (1 << 3);
>>>> + /* set vmid to 2.5k for fast start */
>>>> + wm8940_write(codec, WM8940_POWER1, pwr_reg | 0x3);
>>>> + break;
>
>>> This isn't what the low resistance VMID setting is for;
>
>> I admit I didn't really understand what was going on here, so I think I cribbed
>> it from another driver. What should the setting be? (and what is that setting
>> for?)
>
> Sorry, didn't finish writing that bit. You should be setting the bias
> for normal operation in prepare - the fast startup is only for use
> during initial bringup of VMID since the lower resistrance allows the
> capacitor used to charge more quickly.
>
> The resistance of the VMID resistor string allows you to trade the
> accuracy with which the VMID reference voltage is held (and therefore
> audio performance) for power consumption. Since it can take time to
> charge the capacitors and bring VMID up to the required voltage it's
> desirable to maintian VMID while the system is active in order to allow
> rapid startup of an audio stream but doing so consumes power so the
> higher resistance value is provided in order to allow that consumption
> to be reduced. The savings involved are very small but can produce a
> noticable benefit in small, power sensitive devices like MP3 players.
Ah, that makes sense now.
Thanks
Jonathan
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
