On Tue, Jul 14, 2009 at 03:39:37PM +0900, Joonyoung Shim wrote:
Overall this looks great, thanks. Clearly it's not an ideal approach but it gets the job done and it can be adapted to use core features as they're added.
A couple of relatively nitpicky issues below:
+++ b/sound/soc/codecs/Kconfig @@ -188,3 +188,7 @@ config SND_SOC_WM9712
config SND_SOC_WM9713 tristate
+# Amp +config SND_SOC_MAX9877
- tristate
I'd add it to SND_SOC_ALL_CODECS too - while it's not strictly a CODEC the point of the Kconfig option is to get build coverage and this driver will benefit from that as much as others.
+static const char *max9877_osc_mode[] = {
- "1176KHz",
- "1100KHz",
- "700KHz",
+};
Would this be better as platform data for the device?
+static const struct snd_kcontrol_new max9877_controls[] = {
- SOC_SINGLE_EXT_TLV("MAX9877 Amp HP Left Playback Volume",
MAX9877_HPL_VOLUME, 0, 31, 0,max9877_get_reg, max9877_set_reg, max9877_output_tlv),- SOC_SINGLE_EXT_TLV("MAX9877 Amp HP Right Playback Volume",
MAX9877_HPR_VOLUME, 0, 31, 0,max9877_get_reg, max9877_set_reg, max9877_output_tlv),
Ideally these would be a SOC_DOUBLE_EXT_TLV() but we don't have that yet - would you mind sending a patch for that?
+/* This function is called from ASoC machine driver */ +int max9877_add_controls(struct snd_soc_codec *codec) +{
- return snd_soc_add_controls(codec, max9877_controls,
ARRAY_SIZE(max9877_controls));+}
This should have an EXPORT_SYMBOL_GPL() to allow it to be used from machine drivers which are built modular.