On Wed, Aug 12, 2009 at 12:32:52PM +0800, Barry Song wrote:
There has been an ad1836 driver in sound/blackfin based on traditional alsa. The new driver is based on asoc. The architecture of ad1836 codec driver is very much like ad1938.
This is mostly good, a few relatively minor issues though.
- SOC_DOUBLE_R("DAC3 Volume", AD1836_DAC_L3_VOL,
AD1836_DAC_R3_VOL, 0, 0x3FF, 0),
You've got an extra space in the volume control names which I'd expect to confuse user space.
{ "DAC1OUT", "DAC1 Switch", "DAC" },{ "DAC2OUT", "DAC2 Switch", "DAC" },
I'm surprised this works since the switches weren't declared as DAPM controls but if the core supports it that's OK.
+static int ad1836_set_tdm_slot(struct snd_soc_dai *dai,
unsigned int mask, int slots)+{
This needs updating for the new set_tdm_slot() API but given that there's only one possible configuration I'd suggest just removing it - the function does nothing other than check its arguments.
- reg = codec->read(codec, AD1836_DAC_CTRL1);
- reg = (reg & (~AD1836_DAC_WORD_LEN_MASK)) | word_len;
- codec->write(codec, AD1836_DAC_CTRL1, reg);
Not essential but snd_soc_update_bits() does the read/modify/write cycle for you.
+static struct spi_driver ad1836_spi_driver = {
- .driver = {
.name = "ad1836-spi",
Just "ad1836" - the fact that it's controlled via SPI is clear from the fact that this is a struct spi_driver.
- ret = snd_soc_register_codec(codec);
- if (ret != 0) {
dev_err(codec->dev, "Failed to register codec: %d\n", ret);return ret;- }
- ret = snd_soc_register_dai(&ad1836_dai);
- if (ret != 0) {
dev_err(codec->dev, "Failed to register DAI: %d\n", ret);snd_soc_unregister_codec(codec);return ret;- }
Should also be freeing the private data structure on error.
+struct snd_soc_codec_device soc_codec_dev_ad1836 = {
- .probe = ad1836_probe,
- .remove = ad1836_remove,
- /* The power management of ad1836 is very simple. There are
* only adc&dac 2 components to control. Dapm handles them.*/- .suspend = NULL,
- .resume = NULL,
The power control will be handled by DAPM but your driver still needs to restore things like the volume settings - when the driver is powered off the register settings will be forgotten.