2011/12/3 Mark Brown broonie@opensource.wolfsonmicro.com:
On Fri, Dec 02, 2011 at 06:45:15PM +0900, Tomoya MORINAGA wrote:
+#define CODEC_DEV_ADDR 0x1A +static struct i2c_board_info ioh_hwmon_info[] = {
- {I2C_BOARD_INFO("ioh_i2c-0", CODEC_DEV_ADDR + 1)},
- {I2C_BOARD_INFO("ioh_i2c-1", CODEC_DEV_ADDR + 2)},
- {I2C_BOARD_INFO("ioh_i2c-2", CODEC_DEV_ADDR + 3)},
- {I2C_BOARD_INFO("ioh_i2c-3", CODEC_DEV_ADDR + 4)},
- {I2C_BOARD_INFO("ioh_i2c-4", CODEC_DEV_ADDR + 5)},
- {I2C_BOARD_INFO("ioh_i2c-5", CODEC_DEV_ADDR + 0)},
This looks completely wrong. I'd not expect to see any I2C_BOARD_INFO usage at all in a machine driver (that should be done by whatever enumerates the system as a whole)
Do you mean machine driver must not use I2C_BOARD_INFO ? Is this true ? Grepping I2C_BOARD_INFO at sound/soc, the following drivers use I2C_BOARD_INFO. pxa/raumfeld.c: I2C_BOARD_INFO("max9485", 0x63), pxa/magician.c: I2C_BOARD_INFO("uda1380", 0x18), s6000/s6105-ipcam.c: { I2C_BOARD_INFO("tlv320aic33", 0x18), }
and I'm not clear what the intended effect of the above is - the names look like the names of I2C controllers...
You are right. "ioh_i2c-0" must be "ml26124".
+static int ml7213_startup(struct snd_pcm_substream *substream) +{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_codec *codec = rtd->codec;
- struct i2c_client *i2c;
- struct i2c_adapter *adapter;
- mutex_lock(&codec->mutex);
- adapter = i2c_get_adapter(1);
- if (!adapter)
- return -ENODEV;
- i2c = i2c_new_device(adapter, &ioh_hwmon_info[substream->number]);
- if (!i2c) {
- mutex_unlock(&codec->mutex);
- return -1;
- }
- i2c_put_adapter(adapter);
- mutex_unlock(&codec->mutex);
No, this is definitely wrong - we certainly shouldn't be registering new devices whenever someone starts an audio stream. I'm struggling to understand what the intended effect is though.
Do you mean this registering must be at xx_init processing ? OK, I'll move this processing into init().
- ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
- SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS);
- if (ret < 0)
- return ret;
Use the dai_fmt field in the dai_link to set this.
Sorry, I can't understand your saying. Let me know in more detail.
+MODULE_AUTHOR("Tomoya MORINAGA tomoya.rohm@gmail.com"); +MODULE_DESCRIPTION("LAPIS Semiconductor ML7213 IOH ALSA SoC machine driver"); +MODULE_LICENSE("GPL");
Should have MODULE_ALIAS too.
Do you mean machine driver should have MODULE_ALIAS ? Grepping MODULE_ALIAS at sound/soc, it seems MODULE_ALIAS is used in platform driver only like below. pxa/pxa2xx-i2s.c:MODULE_ALIAS("platform:pxa2xx-i2s");
Does machine driver need like "MODULE_ALIAS("machine:ml7213CRB")" ?
Thanks in advance
tomoya