At Tue, 26 Aug 2008 15:28:56 +0200, Jean Delvare wrote:
Hi Mark,
On Tue, 26 Aug 2008 13:05:27 +0100, Mark Brown wrote:
The WM8903 is a high performance ultra-low power stereo CODEC optimised for portable audio applications. Features include:
* 5mW power consumption for DAC to headphone playback * Stereo DAC SNR 96dB typical, THD -86dB typical * Stereo ADC SNR 93dB typical, THD -80dB typical * Up to 3 single ended inputs per stereo channel * Up to 2 pseudo differential inputs per stereo channel * Up to 1 fully differential mic input per stereo channel * Digital Dynamic Range Controller (compressor/limiter) * Digital sidetone mixingSigned-off-by: Mark Brown broonie@opensource.wolfsonmicro.com
sound/soc/codecs/Kconfig | 4 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/wm8903.c | 1813 +++++++++++++++++++++++++++++++++++++++++++++ sound/soc/codecs/wm8903.h | 1463 ++++++++++++++++++++++++++++++++++++ 4 files changed, 3282 insertions(+), 0 deletions(-) create mode 100644 sound/soc/codecs/wm8903.c create mode 100644 sound/soc/codecs/wm8903.h
Here are minor issues I spotted in the i2c part of your driver:
(...) +static int wm8903_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)+{
- struct snd_soc_device *socdev = wm8903_socdev;
- struct snd_soc_codec *codec = socdev->codec;
- int ret;
- i2c_set_clientdata(i2c, codec);
- codec->control_data = i2c;
- ret = wm8903_init(socdev);
- if (ret < 0)
dev_err(&i2c->dev, "Device initialisation failed\n");- return ret;
+}
+static int wm8903_i2c_remove(struct i2c_client *client) +{
- struct snd_soc_codec *codec = i2c_get_clientdata(client);
- kfree(codec->reg_cache);
- return 0;
+}
+/* i2c codec control layer */ +static const struct i2c_device_id wm8903_i2c_id[] = {
{ "wm8903", 0 },{ }+}; +MODULE_DEVICE_TABLE(i2c, wm8903_i2c_id);
+static struct i2c_driver wm8903_i2c_driver = {
- .driver = {
.name = "WM8903",.owner = THIS_MODULE,- },
- .probe = wm8903_i2c_probe,
- .remove = wm8903_i2c_remove,
- .id_table = wm8903_i2c_id,
+};
+static struct i2c_client *wm8903_i2c_device;
I know that I am the one who came up with this idea, but on second thought, wm8903_i2c_device will always be equal to wm8903_socdev->codec->control_data, so we can easily do without it.
Indeed. It's a good cleanup.
+static int wm8903_probe(struct platform_device *pdev) +{
- struct snd_soc_device *socdev = platform_get_drvdata(pdev);
- struct wm8903_setup_data *setup;
- struct snd_soc_codec *codec;
- struct wm8903_priv *wm8903;
- struct i2c_board_info board_info;
- struct i2c_adapter *adapter;
- int ret = 0;
- setup = socdev->codec_data;
- if (!setup->i2c_address) {
dev_err(&pdev->dev, "No codec address provided\n");return -ENODEV;- }
- codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL);
- if (codec == NULL)
return -ENOMEM;- wm8903 = kzalloc(sizeof(struct wm8903_priv), GFP_KERNEL);
- if (wm8903 == NULL) {
ret = -ENOMEM;goto err_codec;- }
- codec->private_data = wm8903;
- socdev->codec = codec;
- mutex_init(&codec->mutex);
- INIT_LIST_HEAD(&codec->dapm_widgets);
- INIT_LIST_HEAD(&codec->dapm_paths);
- wm8903_socdev = socdev;
- codec->hw_write = (hw_write_t)i2c_master_send;
- ret = i2c_add_driver(&wm8903_i2c_driver);
- if (ret != 0) {
dev_err(&pdev->dev, "can't add i2c driver");Missing \n at end of string (looks like many other drivers have this problem - I found 11 other cases under sound/soc, I can send a patch fixing these if you want.)
It'd be appreciated. If Mark didn't work on it yet, I'll merge them.
goto err_priv;- } else {
memset(&board_info, 0, sizeof(board_info));strlcpy(board_info.type, "wm8903", I2C_NAME_SIZE);board_info.addr = setup->i2c_address;adapter = i2c_get_adapter(setup->i2c_bus);if (!adapter) {dev_err(&pdev->dev, "Can't get I2C bus %d\n",setup->i2c_bus);goto err_adapter;You are jumping to the error path but ret has value 0. This means that the function returns success when you really mean failure, and trouble is likely to arise after that.
Good catch.
Mark, could you post a fix patch? The original patch was already merged, so I'd like to apply on it.
thanks,
Takashi