Messages in this thread | | | Date | Tue, 22 Nov 2011 19:47:10 +0900 | Subject | Re: [PATCH 1/3] sound/soc/codecs: add LAPIS Semiconductor ML26124 | From | Tomoya MORINAGA <> |
| |
Hi Mark,
Thank you for your comments. I have some questions.
2011/11/21 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: >> +static const struct snd_kcontrol_new ml26124_snd_controls[] = { >> + SOC_SINGLE_TLV("Record Digital Volume", ML26124_RECORD_DIG_VOL, 0, 0xff, 1, rec_play_digi_vol), > > Capture, not Record.
OK, BTW, What's TLV ? Let me know the full spell of this "TLV".
> >> +static const struct snd_kcontrol_new ml26124_dsp_controls[] = { >> + SOC_SINGLE("Play Limitter ON/OFF", ML26124_FILTER_EN, 0, 1, 0), > Do you mean SOC_SINGLE("Play Limitter Switch", ML26124_FILTER_EN, 0, 1, 0) ?
>> + SOC_SINGLE("Set ALC position", ML26124_FILTER_EN, 5, 1, 0), > > What does this actually do? From the name it *really* doesn't look like > a mixer input. > The above means where connects the ALC. So, this doesn't relate to mixer input.
>> +static const struct snd_soc_dapm_route intercon[] = { >> +}; > > I can't see how any of the DAPM widgets you specify will ever work with > no interconnections. I see. However I couldn't understand the meaning of the widgets. So I didn't write anything. For example, wm8731 writes like below. Could you explain this ?
static const struct snd_soc_dapm_route wm8731_intercon[] = { {"DAC", NULL, "OSC", wm8731_check_osc}, {"ADC", NULL, "OSC", wm8731_check_osc}, {"DAC", NULL, "ACTIVE"}, {"ADC", NULL, "ACTIVE"},
/* output mixer */ {"Output Mixer", "Line Bypass Switch", "Line Input"}, {"Output Mixer", "HiFi Playback Switch", "DAC"}, {"Output Mixer", "Mic Sidetone Switch", "Mic Bias"},
/* outputs */ {"RHPOUT", NULL, "Output Mixer"}, {"ROUT", NULL, "Output Mixer"}, {"LHPOUT", NULL, "Output Mixer"}, {"LOUT", NULL, "Output Mixer"},
/* input mux */ {"Input Mux", "Line In", "Line Input"}, {"Input Mux", "Mic", "Mic Bias"}, {"ADC", NULL, "Input Mux"},
/* inputs */ {"Line Input", NULL, "LLINEIN"}, {"Line Input", NULL, "RLINEIN"}, {"Mic Bias", NULL, "MICIN"}, };
> >> + >> +static int snd_card_codec_reg_read(struct ml26124_priv *priv, >> + unsigned char reg, unsigned char *val) >> +{ >> + unsigned char data; >> + struct i2c_client *i2c; > > Use the standard register access code, don't open code things in your > driver unless there's a good reason to. Current best practice for most > I2C or SPI devices is to use regmap, see any recently added driver for > examples. What's "regmap" mean ? or do you mean drivers/mfd/* ? Could you tell me this ?
>> + unsigned char data; >> + unsigned int rate = priv->rate; >> + unsigned int channels = priv->ch; >> + >> + snd_card_codec_reg_read(priv, 0x30, &data); /* Read MICVIAS Voltage */ >> + >> + snd_card_codec_reg_write(priv, 0x10, 0x01); /* soft reset assert */ >> + snd_card_codec_reg_write(priv, 0x10, 0x00); /* soft reset negate */ >> + >> + snd_card_codec_reg_write(priv, 0x0c, 0x00); /* Stop clock */ > > Use snd_soc_update_bits() and the other standard register access > functions. Using snd_soc_update_bits(), need to register ".read" method. Is the same as the above snd_card_codec_reg_read ?
Thanks in advance, --- tomoya ROHM Co., Ltd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |