[alsa-devel] [PATCH 1/8] sound:asoc: Add support for STA529 Audio Codec
Mark Brown
broonie at opensource.wolfsonmicro.com
Tue Mar 20 16:25:49 CET 2012
On Tue, Mar 20, 2012 at 05:03:45PM +0530, Rajeev Kumar wrote:
> This patch adds the support for STA529 audio codec.
> Details of the audio codec can be seen here:
> http://www.st.com/internet/imag_video/product/159187.jsp
Please use subject lines appropriate for the subsystem. If your commits
look visually different to all the other commits in the log that's not a
good sign.
> +config SND_SOC_STA529
> + tristate
> + depends on I2C
> +
Drop the dependency, none of the other CODECs have this for a reason...
> +#include <linux/platform_device.h>
If you need this something went wrong...
> +static const u8 sta529_reg[STA529_CACHEREGNUM] = {
Use regmap for register I/O.
> +struct sta529 {
> + unsigned int sysclk;
> + enum snd_soc_control_type control_type;
> + void *control_data;
Your device only supports I2C, no need for the infrastructure for other
buses (though this will go away with regmap).
> +static const char *pwm_mode_text[] = { "binary", "headphone", "ternary",
> + "phase-shift"};
> +static const char *interface_mode_text[] = { "slave", "master"};
ALSA controls always use capitalisation.
> + case SND_SOC_BIAS_PREPARE:
> + snd_soc_update_bits(codec, STA529_FFXCFG0, POWER_CNTLMSAK,
> + POWER_UP);
> + snd_soc_update_bits(codec, STA529_MISC, 1, 0x01);
One of these has magic numbers and the other doesn't, and the magic
numbers are a little confusing as the mask is written in decimal but the
binary in hex.
> + switch (params_format(params)) {
> + case SNDRV_PCM_FORMAT_S16_LE:
> + pdata = 1;
> + break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + pdata = 2;
> + break;
> + case SNDRV_PCM_FORMAT_S32_LE:
> + pdata = 3;
> + break;
> + }
Return an error on unsupported sample sizes.
> + default:
> + printk(KERN_WARNING, "bad rate\n");
> + return -EINVAL;
> + }
dev_warn() - always use the dev_ prints where possible.
> + sta529_set_bias_level(codec, SND_SOC_BIAS_PREPARE);
> + mdelay(10);
Absolutely no - why are you doing this?
> +static int sta529_mute(struct snd_soc_dai *dai, int mute)
> +{
> + struct snd_soc_codec *codec = dai->codec;
> +
> + u8 mute_reg = snd_soc_read(codec, STA529_FFXCFG0) & ~CODEC_MUTE_VAL;
> +
> + if (mute)
> + mute_reg |= CODEC_MUTE_VAL;
> +
> + snd_soc_update_bits(codec, STA529_FFXCFG0, 0x80, 00);
You're always setting the same value here...
> + snd_soc_add_controls(codec, sta529_snd_controls,
> + ARRAY_SIZE(sta529_snd_controls));
> +
> + snd_soc_add_controls(codec, sta529_new_snd_controls,
> + ARRAY_SIZE(sta529_new_snd_controls));
Use the initialisers in the card struct.
> + sta529 = kzalloc(sizeof(struct sta529), GFP_KERNEL);
> + if (sta529 == NULL)
> + return -ENOMEM;
devm_kzalloc().
> +static int __init sta529_modinit(void)
module_i2c_driver().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20120320/a6102837/attachment.sig
More information about the Alsa-devel
mailing list