On Thu, Dec 30, 2010 at 04:42:31PM +0530, Vinod Koul wrote:
This patch adds the msic asoc codec driver. This driver currently supports only playback. Capture and jack detection to be added later
This looks mostly OK, but you probably want to run it through checkpatch.pl. For quite a few of the debug prints you've got here you're replicating stuff that's in the core as standard - if you turn on debug logs from the core you should get equivalent logging.
Some more specific issues below:
- switch (params_format(params)) {
- case SNDRV_PCM_FORMAT_S16_LE:
format = 3;break;- case SNDRV_PCM_FORMAT_S24_LE:
format = 0;break;- default:
return -EINVAL;- }
- snd_soc_update_bits(dai->codec, MSIC_PCM2C2, BIT(4)|BIT(5), format);
This won't work - both the mask and value for snd_soc_update_bits() are bitmasks so I'd expect the format for S16 to be 0x18 with that bitmask.
- snd_soc_update_bits(dai->codec, MSIC_PCM1C1, BIT(7), rate);
Similarly here.
- /*PCM interface */
- msic_write(codec, MSIC_PCM2RXSLOT01, 0x10);
- msic_write(codec, MSIC_PCM2RXSLOT23, 0x32);
- msic_write(codec, MSIC_PCM2RXSLOT45, 0x54);
For these it'd be better if you could provide some comments explaining what the configuration that's being set is. It's hard to tell if this is stuff that should be managed by the driver or not.
+static struct platform_driver intelmid_codec_driver = {
- .driver = {
.name = "mid-msic-codec",
+MODULE_ALIAS("msic-codec");
This should be platform:mid-msic-codec.