On Thu 2011-02-17 09:52:52, Arnaud Patard wrote:
zhaoming.zeng@freescale.com writes:
Hi,
From: Zeng Zhaoming zhaoming.zeng@freescale.com
Add Freescale SGTL5000 codec support
Signed-off-by: Zeng Zhaoming zhaoming.zeng@freescale.com
changes since v2:
- clean up register default values
- rewrite codec power up code, add sgtl5000_set_power_regs()
- rewrite codec clock configure code sgtl5000_set_clock()
- reimplement PM hooks, restore register by particular order.
- clean up dapm code, remove dac and adc event hooks.
- clean up codec private structure, remove unnecessary fields.
- add comments for uncommon code.
Thanks for Mark's review.
[...]
+/*
- custom put function for PCM playback volume
- PCM volume with 0.5017 dB steps from 0 to -90 dB
- 0x3B and less = Reserved
- 0x3C = 0 dB
- 0x3D = -0.5 dB
- 0xF0 = -90 dB
- 0xFC and greater = Muted
- */
+static int dac_put_volsw(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
- int reg, l, r;
- l = ucontrol->value.integer.value[0];
- r = ucontrol->value.integer.value[1];
- l = l < 0 ? 0 : l;
- l = l > 0xfc - 0x3c ? 0xfc - 0x3c : l;
- r = r < 0 ? 0 : r;
- r = r > 0xfc - 0x3c ? 0xfc - 0x3c : r;
- l = 0xfc - l;
- r = 0xfc - r;
- reg = l << SGTL5000_DAC_VOL_LEFT_SHIFT |
r << SGTL5000_DAC_VOL_RIGHT_SHIFT;- snd_soc_update_bits_locked(codec, SGTL5000_CHIP_DAC_VOL, reg, reg);
Why are you using update_bits_locked and not snd_soc_write ? Here snd_soc_update_bits_locked is just good at making the PCM playback mixer acting weird (I even got to a point that with a 0 value in the mixer, I was unable to raise the volume). snd_soc_write just works.
[...]
hi, Arnaud, thanks. I just find this problem too. It will be changed to snd_soc_write() in v4.
+static int sgtl5000_probe(struct snd_soc_codec *codec) +{
- struct sgtl5000_priv *sgtl5000 = snd_soc_codec_get_drvdata(codec);
- u16 reg;
- int ret;
- int rev;
- int i;
- /* setup i2c data ops */
- ret = snd_soc_codec_set_cache_io(codec, 16, 16, SND_SOC_I2C);
- if (ret < 0) {
dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);return ret;- }
- /* read chip information */
- reg = snd_soc_read(codec, SGTL5000_CHIP_ID);
I've a problem with doing that here. You do assume that the regulators are enabled at this point which is not always the case. This results on a timeout on i2c and then reg contains 0. Please enable regultors firsts And no, I don't see having the regulators for things like vddio set to "always on" in the machine file as a good idea. If the driver is not loaded, having the codec off is good for power consumption.
you are right, I always enable regulator in platform code, it is better to enable it when driver is loading. I will fix it in v4.
Arnaud _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel