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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
