[PATCH] WM8750: Convert to new API

[PATCH] WM8750: Convert to new API

[Marek Vasut]

Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
around. Hugely inspired by WM8753 which was already converted.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 sound/soc/codecs/wm8750.c |  297 +++++++++++++++++++++------------------------
 1 files changed, 140 insertions(+), 157 deletions(-)

diff --git a/sound/soc/codecs/wm8750.c b/sound/soc/codecs/wm8750.c
index ee08408..f01513e 100644
--- a/sound/soc/codecs/wm8750.c
+++ b/sound/soc/codecs/wm8750.c
@@ -29,11 +29,6 @@
 
 #include "wm8750.h"
 
-/* codec private data */
-struct wm8750_priv {
-	unsigned int sysclk;
-};
-
 /*
  * wm8750 register cache
  * We can't read the WM8750 register space when we
@@ -53,6 +48,13 @@ static const u16 wm8750_reg[] = {
 	0x0079, 0x0079, 0x0079,          /* 40 */
 };
 
+/* codec private data */
+struct wm8750_priv {
+	unsigned int sysclk;
+	struct snd_soc_codec codec;
+	u16 reg_cache[ARRAY_SIZE(wm8750_reg)];
+};
+
 #define wm8750_reset(c)	snd_soc_write(c, WM8750_RESET, 0)
 
 /*
@@ -695,25 +697,90 @@ static int wm8750_resume(struct platform_device *pdev)
 	return 0;
 }
 
+static struct snd_soc_codec *wm8750_codec;
+
+static int wm8750_probe(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+	struct snd_soc_codec *codec;
+	int ret = 0;
+
+	if (!wm8750_codec) {
+		dev_err(&pdev->dev, "WM8750 codec not yet registered\n");
+		return -EINVAL;
+	}
+
+	socdev->card->codec = wm8750_codec;
+	codec = wm8750_codec;
+
+	/* register pcms */
+	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
+	if (ret < 0) {
+		printk(KERN_ERR "wm8750: failed to create pcms\n");
+		goto err;
+	}
+
+	snd_soc_add_controls(codec, wm8750_snd_controls,
+				ARRAY_SIZE(wm8750_snd_controls));
+	wm8750_add_widgets(codec);
+
+	return 0;
+
+err:
+	return ret;
+}
+
+/* power down chip */
+static int wm8750_remove(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+
+	snd_soc_free_pcms(socdev);
+	snd_soc_dapm_free(socdev);
+
+	return 0;
+}
+
+struct snd_soc_codec_device soc_codec_dev_wm8750 = {
+	.probe = 	wm8750_probe,
+	.remove = 	wm8750_remove,
+	.suspend = 	wm8750_suspend,
+	.resume =	wm8750_resume,
+};
+EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
+
 /*
  * initialise the WM8750 driver
  * register the mixer and dsp interfaces with the kernel
  */
-static int wm8750_init(struct snd_soc_device *socdev,
-		       enum snd_soc_control_type control)
+static int wm8750_register(struct wm8750_priv *wm8750,
+			enum snd_soc_control_type control)
 {
-	struct snd_soc_codec *codec = socdev->card->codec;
+	struct snd_soc_codec *codec = &wm8750->codec;
 	int reg, ret = 0;
 
+	if (wm8750_codec) {
+		dev_err(codec->dev, "Multiple WM8750 devices not supported\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	mutex_init(&codec->mutex);
+	INIT_LIST_HEAD(&codec->dapm_widgets);
+	INIT_LIST_HEAD(&codec->dapm_paths);
+
 	codec->name = "WM8750";
 	codec->owner = THIS_MODULE;
+	codec->bias_level = SND_SOC_BIAS_STANDBY;
 	codec->set_bias_level = wm8750_set_bias_level;
 	codec->dai = &wm8750_dai;
 	codec->num_dai = 1;
-	codec->reg_cache_size = ARRAY_SIZE(wm8750_reg);
-	codec->reg_cache = kmemdup(wm8750_reg, sizeof(wm8750_reg), GFP_KERNEL);
-	if (codec->reg_cache == NULL)
-		return -ENOMEM;
+	codec->private_data = wm8750;
+	codec->reg_cache_size = ARRAY_SIZE(wm8750->reg_cache) + 1;
+	codec->reg_cache = &wm8750->reg_cache;
+	codec->private_data = wm8750;
+
+	memcpy(codec->reg_cache, wm8750_reg, sizeof(wm8750->reg_cache));
 
 	ret = snd_soc_codec_set_cache_io(codec, 7, 9, control);
 	if (ret < 0) {
@@ -727,13 +794,6 @@ static int wm8750_init(struct snd_soc_device *socdev,
 		goto err;
 	}
 
-	/* register pcms */
-	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
-	if (ret < 0) {
-		printk(KERN_ERR "wm8750: failed to create pcms\n");
-		goto err;
-	}
-
 	/* charge output caps */
 	wm8750_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
@@ -755,19 +815,37 @@ static int wm8750_init(struct snd_soc_device *socdev,
 	reg = snd_soc_read(codec, WM8750_RINVOL);
 	snd_soc_write(codec, WM8750_RINVOL, reg | 0x0100);
 
-	snd_soc_add_controls(codec, wm8750_snd_controls,
-				ARRAY_SIZE(wm8750_snd_controls));
-	wm8750_add_widgets(codec);
-	return ret;
+	wm8750_codec = codec;
+
+	ret = snd_soc_register_codec(codec);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
+		goto err;
+	}
 
+	ret = snd_soc_register_dais(&wm8750_dai, 1);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register DAIs: %d\n", ret);
+		goto err_codec;
+	}
+
+	return 0;
+
+err_codec:
+	snd_soc_unregister_codec(codec);
 err:
-	kfree(codec->reg_cache);
+	kfree(wm8750);
 	return ret;
 }
 
-/* If the i2c layer weren't so broken, we could pass this kind of data
-   around */
-static struct snd_soc_device *wm8750_socdev;
+static void wm8750_unregister(struct wm8750_priv *wm8750)
+{
+	wm8750_set_bias_level(&wm8750->codec, SND_SOC_BIAS_OFF);
+	snd_soc_unregister_dais(&wm8750_dai, 1);
+	snd_soc_unregister_codec(&wm8750->codec);
+	kfree(wm8750);
+	wm8750_codec = NULL;
+}
 
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
 
@@ -781,24 +859,26 @@ static struct snd_soc_device *wm8750_socdev;
 static int wm8750_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
-	i2c_set_clientdata(i2c, codec);
-	codec->control_data = i2c;
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
 
-	ret = wm8750_init(socdev, SND_SOC_I2C);
-	if (ret < 0)
-		pr_err("failed to initialise WM8750\n");
+        codec = &wm8750->codec;
+        codec->control_data = i2c;
+        i2c_set_clientdata(i2c, wm8750);
 
-	return ret;
+        codec->dev = &i2c->dev;
+
+	return wm8750_register(wm8750, SND_SOC_I2C);
 }
 
 static int wm8750_i2c_remove(struct i2c_client *client)
 {
-	struct snd_soc_codec *codec = i2c_get_clientdata(client);
-	kfree(codec->reg_cache);
+        struct wm8750_priv *wm8750 = i2c_get_clientdata(client);
+        wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -817,66 +897,31 @@ static struct i2c_driver wm8750_i2c_driver = {
 	.remove =   wm8750_i2c_remove,
 	.id_table = wm8750_i2c_id,
 };
-
-static int wm8750_add_i2c_device(struct platform_device *pdev,
-				 const struct wm8750_setup_data *setup)
-{
-	struct i2c_board_info info;
-	struct i2c_adapter *adapter;
-	struct i2c_client *client;
-	int ret;
-
-	ret = i2c_add_driver(&wm8750_i2c_driver);
-	if (ret != 0) {
-		dev_err(&pdev->dev, "can't add i2c driver\n");
-		return ret;
-	}
-
-	memset(&info, 0, sizeof(struct i2c_board_info));
-	info.addr = setup->i2c_address;
-	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
-
-	adapter = i2c_get_adapter(setup->i2c_bus);
-	if (!adapter) {
-		dev_err(&pdev->dev, "can't get i2c adapter %d\n",
-			setup->i2c_bus);
-		goto err_driver;
-	}
-
-	client = i2c_new_device(adapter, &info);
-	i2c_put_adapter(adapter);
-	if (!client) {
-		dev_err(&pdev->dev, "can't add i2c device at 0x%x\n",
-			(unsigned int)info.addr);
-		goto err_driver;
-	}
-
-	return 0;
-
-err_driver:
-	i2c_del_driver(&wm8750_i2c_driver);
-	return -ENODEV;
-}
 #endif
 
 #if defined(CONFIG_SPI_MASTER)
 static int __devinit wm8750_spi_probe(struct spi_device *spi)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
+
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
 
+	codec = &wm8750->codec;
 	codec->control_data = spi;
+	codec->dev = &spi->dev;
 
-	ret = wm8750_init(socdev, SND_SOC_SPI);
-	if (ret < 0)
-		dev_err(&spi->dev, "failed to initialise WM8750\n");
+	dev_set_drvdata(&spi->dev, wm8750);
 
-	return ret;
+	return wm8750_register(wm8750, SND_SOC_SPI);
 }
 
 static int __devexit wm8750_spi_remove(struct spi_device *spi)
 {
+	struct wm8750_priv *wm8750 = dev_get_drvdata(&spi->dev);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -891,93 +936,31 @@ static struct spi_driver wm8750_spi_driver = {
 };
 #endif
 
-static int wm8750_probe(struct platform_device *pdev)
+static int __init wm8750_modinit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct wm8750_setup_data *setup = socdev->codec_data;
-	struct snd_soc_codec *codec;
-	struct wm8750_priv *wm8750;
 	int ret;
-
-	codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL);
-	if (codec == NULL)
-		return -ENOMEM;
-
-	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
-	if (wm8750 == NULL) {
-		kfree(codec);
-		return -ENOMEM;
-	}
-
-	codec->private_data = wm8750;
-	socdev->card->codec = codec;
-	mutex_init(&codec->mutex);
-	INIT_LIST_HEAD(&codec->dapm_widgets);
-	INIT_LIST_HEAD(&codec->dapm_paths);
-	wm8750_socdev = socdev;
-
-	ret = -ENODEV;
-
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	if (setup->i2c_address) {
-		ret = wm8750_add_i2c_device(pdev, setup);
-	}
+	ret = i2c_add_driver(&wm8750_i2c_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 I2C driver: %d\n", ret);
 #endif
 #if defined(CONFIG_SPI_MASTER)
-	if (setup->spi) {
-		ret = spi_register_driver(&wm8750_spi_driver);
-		if (ret != 0)
-			printk(KERN_ERR "can't add spi driver");
-	}
+	ret = spi_register_driver(&wm8750_spi_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 SPI driver: %d\n", ret);
 #endif
-
-	if (ret != 0) {
-		kfree(codec->private_data);
-		kfree(codec);
-	}
-	return ret;
+	return 0;
 }
+module_init(wm8750_modinit);
 
-/* power down chip */
-static int wm8750_remove(struct platform_device *pdev)
+static void __exit wm8750_exit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct snd_soc_codec *codec = socdev->card->codec;
-
-	if (codec->control_data)
-		wm8750_set_bias_level(codec, SND_SOC_BIAS_OFF);
-	snd_soc_free_pcms(socdev);
-	snd_soc_dapm_free(socdev);
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	i2c_unregister_device(codec->control_data);
 	i2c_del_driver(&wm8750_i2c_driver);
 #endif
 #if defined(CONFIG_SPI_MASTER)
 	spi_unregister_driver(&wm8750_spi_driver);
 #endif
-	kfree(codec->private_data);
-	kfree(codec);
-
-	return 0;
-}
-
-struct snd_soc_codec_device soc_codec_dev_wm8750 = {
-	.probe = 	wm8750_probe,
-	.remove = 	wm8750_remove,
-	.suspend = 	wm8750_suspend,
-	.resume =	wm8750_resume,
-};
-EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
-
-static int __init wm8750_modinit(void)
-{
-	return snd_soc_register_dai(&wm8750_dai);
-}
-module_init(wm8750_modinit);
-
-static void __exit wm8750_exit(void)
-{
-	snd_soc_unregister_dai(&wm8750_dai);
 }
 module_exit(wm8750_exit);
 
-- 
1.7.0

Re: [PATCH] WM8750: Convert to new API

[Liam Girdwood]

On Fri, 2010-03-26 at 15:19 +0100, Marek Vasut wrote:
> Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
> around. Hugely inspired by WM8753 which was already converted.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  sound/soc/codecs/wm8750.c |  297 +++++++++++++++++++++------------------------
>  1 files changed, 140 insertions(+), 157 deletions(-)

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH] WM8750: Convert to new API

[Mark Brown]

On Fri, Mar 26, 2010 at 03:19:02PM +0100, Marek Vasut wrote:
> Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
> around. Hugely inspired by WM8753 which was already converted.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

This all looks good but we need the machine driver updates pulling out
the registration too.  Like I say, wm8731 had the same problem.

The use of snd_soc_register_dais() with just one DAI is a bit odd but
not a problem.

Re: [PATCH] WM8750: Convert to new API

[Marek Vasut]

Dne Po 29. března 2010 14:48:09 Mark Brown napsal(a):
> On Fri, Mar 26, 2010 at 03:19:02PM +0100, Marek Vasut wrote:
> > Register the WM8750 as a SPI or I2C device. This patch mostly shuffles
> > code around. Hugely inspired by WM8753 which was already converted.
> >
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> 
> This all looks good but we need the machine driver updates pulling out
> the registration too.  Like I say, wm8731 had the same problem.
> 
> The use of snd_soc_register_dais() with just one DAI is a bit odd but
> not a problem.
> 
ok, shall I send that in another patch and CC linux-arm ?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] WM8750: Convert to new API

[Mark Brown]

On Mon, Mar 29, 2010 at 11:16:11PM +0200, Marek Vasut wrote:

> ok, shall I send that in another patch and CC linux-arm ?

It should be merged into the same patch but yes, or to mitigate against
merge issues you could do the registration in the ASoC machine drivers
then merge to arch/arm in a second round later.

Re: [PATCH] WM8750: Convert to new API

[Marek Vasut]

Dne Út 30. března 2010 11:40:43 Mark Brown napsal(a):
> On Mon, Mar 29, 2010 at 11:16:11PM +0200, Marek Vasut wrote:
> > ok, shall I send that in another patch and CC linux-arm ?
> 
> It should be merged into the same patch but yes, or to mitigate against
> merge issues you could do the registration in the ASoC machine drivers
> then merge to arch/arm in a second round later.
> 
I'll send two patches and we can push them both through alsa. I doubt anyone 
will have problems with the arm stuff, those are not too used platform. As for 
the Z, I know the people who work on it so it should be fine.

Could you push this patch? I'll supply the rest tonight.

Cheers
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH] WM8750: Convert to new API

[Marek Vasut]

Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
around. Hugely inspired by WM8753 which was already converted.

Also, this patch fixes the Jive and Spitz machine.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 sound/soc/codecs/wm8750.c       |  295 ++++++++++++++++++---------------------
 sound/soc/pxa/spitz.c           |   43 +++++-
 sound/soc/s3c24xx/jive_wm8750.c |    5 -
 3 files changed, 175 insertions(+), 168 deletions(-)

diff --git a/sound/soc/codecs/wm8750.c b/sound/soc/codecs/wm8750.c
index 3e440c0..eea7ba6 100644
--- a/sound/soc/codecs/wm8750.c
+++ b/sound/soc/codecs/wm8750.c
@@ -30,11 +30,6 @@
 
 #include "wm8750.h"
 
-/* codec private data */
-struct wm8750_priv {
-	unsigned int sysclk;
-};
-
 /*
  * wm8750 register cache
  * We can't read the WM8750 register space when we
@@ -54,6 +49,13 @@ static const u16 wm8750_reg[] = {
 	0x0079, 0x0079, 0x0079,          /* 40 */
 };
 
+/* codec private data */
+struct wm8750_priv {
+	unsigned int sysclk;
+	struct snd_soc_codec codec;
+	u16 reg_cache[ARRAY_SIZE(wm8750_reg)];
+};
+
 #define wm8750_reset(c)	snd_soc_write(c, WM8750_RESET, 0)
 
 /*
@@ -696,25 +698,90 @@ static int wm8750_resume(struct platform_device *pdev)
 	return 0;
 }
 
+static struct snd_soc_codec *wm8750_codec;
+
+static int wm8750_probe(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+	struct snd_soc_codec *codec;
+	int ret = 0;
+
+	if (!wm8750_codec) {
+		dev_err(&pdev->dev, "WM8750 codec not yet registered\n");
+		return -EINVAL;
+	}
+
+	socdev->card->codec = wm8750_codec;
+	codec = wm8750_codec;
+
+	/* register pcms */
+	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
+	if (ret < 0) {
+		printk(KERN_ERR "wm8750: failed to create pcms\n");
+		goto err;
+	}
+
+	snd_soc_add_controls(codec, wm8750_snd_controls,
+				ARRAY_SIZE(wm8750_snd_controls));
+	wm8750_add_widgets(codec);
+
+	return 0;
+
+err:
+	return ret;
+}
+
+/* power down chip */
+static int wm8750_remove(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+
+	snd_soc_free_pcms(socdev);
+	snd_soc_dapm_free(socdev);
+
+	return 0;
+}
+
+struct snd_soc_codec_device soc_codec_dev_wm8750 = {
+	.probe		= wm8750_probe,
+	.remove		= wm8750_remove,
+	.suspend	= wm8750_suspend,
+	.resume		= wm8750_resume,
+};
+EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
+
 /*
  * initialise the WM8750 driver
  * register the mixer and dsp interfaces with the kernel
  */
-static int wm8750_init(struct snd_soc_device *socdev,
-		       enum snd_soc_control_type control)
+static int wm8750_register(struct wm8750_priv *wm8750,
+			enum snd_soc_control_type control)
 {
-	struct snd_soc_codec *codec = socdev->card->codec;
+	struct snd_soc_codec *codec = &wm8750->codec;
 	int reg, ret = 0;
 
+	if (wm8750_codec) {
+		dev_err(codec->dev, "Multiple WM8750 devices not supported\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	mutex_init(&codec->mutex);
+	INIT_LIST_HEAD(&codec->dapm_widgets);
+	INIT_LIST_HEAD(&codec->dapm_paths);
+
 	codec->name = "WM8750";
 	codec->owner = THIS_MODULE;
+	codec->bias_level = SND_SOC_BIAS_STANDBY;
 	codec->set_bias_level = wm8750_set_bias_level;
 	codec->dai = &wm8750_dai;
 	codec->num_dai = 1;
-	codec->reg_cache_size = ARRAY_SIZE(wm8750_reg);
-	codec->reg_cache = kmemdup(wm8750_reg, sizeof(wm8750_reg), GFP_KERNEL);
-	if (codec->reg_cache == NULL)
-		return -ENOMEM;
+	codec->private_data = wm8750;
+	codec->reg_cache_size = ARRAY_SIZE(wm8750->reg_cache) + 1;
+	codec->reg_cache = &wm8750->reg_cache;
+	codec->private_data = wm8750;
+
+	memcpy(codec->reg_cache, wm8750_reg, sizeof(wm8750->reg_cache));
 
 	ret = snd_soc_codec_set_cache_io(codec, 7, 9, control);
 	if (ret < 0) {
@@ -728,13 +795,6 @@ static int wm8750_init(struct snd_soc_device *socdev,
 		goto err;
 	}
 
-	/* register pcms */
-	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
-	if (ret < 0) {
-		printk(KERN_ERR "wm8750: failed to create pcms\n");
-		goto err;
-	}
-
 	/* charge output caps */
 	wm8750_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
@@ -756,19 +816,37 @@ static int wm8750_init(struct snd_soc_device *socdev,
 	reg = snd_soc_read(codec, WM8750_RINVOL);
 	snd_soc_write(codec, WM8750_RINVOL, reg | 0x0100);
 
-	snd_soc_add_controls(codec, wm8750_snd_controls,
-				ARRAY_SIZE(wm8750_snd_controls));
-	wm8750_add_widgets(codec);
-	return ret;
+	wm8750_codec = codec;
 
+	ret = snd_soc_register_codec(codec);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
+		goto err;
+	}
+
+	ret = snd_soc_register_dais(&wm8750_dai, 1);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register DAIs: %d\n", ret);
+		goto err_codec;
+	}
+
+	return 0;
+
+err_codec:
+	snd_soc_unregister_codec(codec);
 err:
-	kfree(codec->reg_cache);
+	kfree(wm8750);
 	return ret;
 }
 
-/* If the i2c layer weren't so broken, we could pass this kind of data
-   around */
-static struct snd_soc_device *wm8750_socdev;
+static void wm8750_unregister(struct wm8750_priv *wm8750)
+{
+	wm8750_set_bias_level(&wm8750->codec, SND_SOC_BIAS_OFF);
+	snd_soc_unregister_dais(&wm8750_dai, 1);
+	snd_soc_unregister_codec(&wm8750->codec);
+	kfree(wm8750);
+	wm8750_codec = NULL;
+}
 
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
 
@@ -782,24 +860,26 @@ static struct snd_soc_device *wm8750_socdev;
 static int wm8750_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
-	i2c_set_clientdata(i2c, codec);
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
+
+	codec = &wm8750->codec;
 	codec->control_data = i2c;
+	i2c_set_clientdata(i2c, wm8750);
 
-	ret = wm8750_init(socdev, SND_SOC_I2C);
-	if (ret < 0)
-		pr_err("failed to initialise WM8750\n");
+	codec->dev = &i2c->dev;
 
-	return ret;
+	return wm8750_register(wm8750, SND_SOC_I2C);
 }
 
 static int wm8750_i2c_remove(struct i2c_client *client)
 {
-	struct snd_soc_codec *codec = i2c_get_clientdata(client);
-	kfree(codec->reg_cache);
+	struct wm8750_priv *wm8750 = i2c_get_clientdata(client);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -818,66 +898,31 @@ static struct i2c_driver wm8750_i2c_driver = {
 	.remove =   wm8750_i2c_remove,
 	.id_table = wm8750_i2c_id,
 };
-
-static int wm8750_add_i2c_device(struct platform_device *pdev,
-				 const struct wm8750_setup_data *setup)
-{
-	struct i2c_board_info info;
-	struct i2c_adapter *adapter;
-	struct i2c_client *client;
-	int ret;
-
-	ret = i2c_add_driver(&wm8750_i2c_driver);
-	if (ret != 0) {
-		dev_err(&pdev->dev, "can't add i2c driver\n");
-		return ret;
-	}
-
-	memset(&info, 0, sizeof(struct i2c_board_info));
-	info.addr = setup->i2c_address;
-	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
-
-	adapter = i2c_get_adapter(setup->i2c_bus);
-	if (!adapter) {
-		dev_err(&pdev->dev, "can't get i2c adapter %d\n",
-			setup->i2c_bus);
-		goto err_driver;
-	}
-
-	client = i2c_new_device(adapter, &info);
-	i2c_put_adapter(adapter);
-	if (!client) {
-		dev_err(&pdev->dev, "can't add i2c device at 0x%x\n",
-			(unsigned int)info.addr);
-		goto err_driver;
-	}
-
-	return 0;
-
-err_driver:
-	i2c_del_driver(&wm8750_i2c_driver);
-	return -ENODEV;
-}
 #endif
 
 #if defined(CONFIG_SPI_MASTER)
 static int __devinit wm8750_spi_probe(struct spi_device *spi)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
+
+	codec = &wm8750->codec;
 	codec->control_data = spi;
+	codec->dev = &spi->dev;
 
-	ret = wm8750_init(socdev, SND_SOC_SPI);
-	if (ret < 0)
-		dev_err(&spi->dev, "failed to initialise WM8750\n");
+	dev_set_drvdata(&spi->dev, wm8750);
 
-	return ret;
+	return wm8750_register(wm8750, SND_SOC_SPI);
 }
 
 static int __devexit wm8750_spi_remove(struct spi_device *spi)
 {
+	struct wm8750_priv *wm8750 = dev_get_drvdata(&spi->dev);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -892,93 +937,31 @@ static struct spi_driver wm8750_spi_driver = {
 };
 #endif
 
-static int wm8750_probe(struct platform_device *pdev)
+static int __init wm8750_modinit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct wm8750_setup_data *setup = socdev->codec_data;
-	struct snd_soc_codec *codec;
-	struct wm8750_priv *wm8750;
 	int ret;
-
-	codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL);
-	if (codec == NULL)
-		return -ENOMEM;
-
-	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
-	if (wm8750 == NULL) {
-		kfree(codec);
-		return -ENOMEM;
-	}
-
-	codec->private_data = wm8750;
-	socdev->card->codec = codec;
-	mutex_init(&codec->mutex);
-	INIT_LIST_HEAD(&codec->dapm_widgets);
-	INIT_LIST_HEAD(&codec->dapm_paths);
-	wm8750_socdev = socdev;
-
-	ret = -ENODEV;
-
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	if (setup->i2c_address) {
-		ret = wm8750_add_i2c_device(pdev, setup);
-	}
+	ret = i2c_add_driver(&wm8750_i2c_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 I2C driver: %d\n", ret);
 #endif
 #if defined(CONFIG_SPI_MASTER)
-	if (setup->spi) {
-		ret = spi_register_driver(&wm8750_spi_driver);
-		if (ret != 0)
-			printk(KERN_ERR "can't add spi driver");
-	}
+	ret = spi_register_driver(&wm8750_spi_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 SPI driver: %d\n", ret);
 #endif
-
-	if (ret != 0) {
-		kfree(codec->private_data);
-		kfree(codec);
-	}
-	return ret;
+	return 0;
 }
+module_init(wm8750_modinit);
 
-/* power down chip */
-static int wm8750_remove(struct platform_device *pdev)
+static void __exit wm8750_exit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct snd_soc_codec *codec = socdev->card->codec;
-
-	if (codec->control_data)
-		wm8750_set_bias_level(codec, SND_SOC_BIAS_OFF);
-	snd_soc_free_pcms(socdev);
-	snd_soc_dapm_free(socdev);
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	i2c_unregister_device(codec->control_data);
 	i2c_del_driver(&wm8750_i2c_driver);
 #endif
 #if defined(CONFIG_SPI_MASTER)
 	spi_unregister_driver(&wm8750_spi_driver);
 #endif
-	kfree(codec->private_data);
-	kfree(codec);
-
-	return 0;
-}
-
-struct snd_soc_codec_device soc_codec_dev_wm8750 = {
-	.probe = 	wm8750_probe,
-	.remove = 	wm8750_remove,
-	.suspend = 	wm8750_suspend,
-	.resume =	wm8750_resume,
-};
-EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
-
-static int __init wm8750_modinit(void)
-{
-	return snd_soc_register_dai(&wm8750_dai);
-}
-module_init(wm8750_modinit);
-
-static void __exit wm8750_exit(void)
-{
-	snd_soc_unregister_dai(&wm8750_dai);
 }
 module_exit(wm8750_exit);
 
diff --git a/sound/soc/pxa/spitz.c b/sound/soc/pxa/spitz.c
index c4cd2ac..1941a35 100644
--- a/sound/soc/pxa/spitz.c
+++ b/sound/soc/pxa/spitz.c
@@ -322,19 +322,44 @@ static struct snd_soc_card snd_soc_spitz = {
 	.num_links = 1,
 };
 
-/* spitz audio private data */
-static struct wm8750_setup_data spitz_wm8750_setup = {
-	.i2c_bus = 0,
-	.i2c_address = 0x1b,
-};
-
 /* spitz audio subsystem */
 static struct snd_soc_device spitz_snd_devdata = {
 	.card = &snd_soc_spitz,
 	.codec_dev = &soc_codec_dev_wm8750,
-	.codec_data = &spitz_wm8750_setup,
 };
 
+/*
+ * FIXME: This is a temporary bodge to avoid cross-tree merge issues.
+ * New drivers should register the wm8750 I2C device in the machine
+ * setup code (under arch/arm for ARM systems).
+ */
+static int wm8750_i2c_register(void)
+{
+	struct i2c_board_info info;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+
+	memset(&info, 0, sizeof(struct i2c_board_info));
+	info.addr = 0x1b;
+	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
+
+	adapter = i2c_get_adapter(0);
+	if (!adapter) {
+		printk(KERN_ERR "can't get i2c adapter 0\n");
+		return -ENODEV;
+	}
+
+	client = i2c_new_device(adapter, &info);
+	i2c_put_adapter(adapter);
+	if (!client) {
+		printk(KERN_ERR "can't add i2c device at 0x%x\n",
+		(unsigned int)info.addr);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static struct platform_device *spitz_snd_device;
 
 static int __init spitz_init(void)
@@ -344,6 +369,10 @@ static int __init spitz_init(void)
 	if (!(machine_is_spitz() || machine_is_borzoi() || machine_is_akita()))
 		return -ENODEV;
 
+	ret = wm8750_i2c_setup();
+	if (ret != 0)
+		return ret;
+
 	spitz_snd_device = platform_device_alloc("soc-audio", -1);
 	if (!spitz_snd_device)
 		return -ENOMEM;
diff --git a/sound/soc/s3c24xx/jive_wm8750.c b/sound/soc/s3c24xx/jive_wm8750.c
index 59dc2c6..97d8ff3 100644
--- a/sound/soc/s3c24xx/jive_wm8750.c
+++ b/sound/soc/s3c24xx/jive_wm8750.c
@@ -152,15 +152,10 @@ static struct snd_soc_card snd_soc_machine_jive = {
 	.num_links	= 1,
 };
 
-/* jive audio private data */
-static struct wm8750_setup_data jive_wm8750_setup = {
-};
-
 /* jive audio subsystem */
 static struct snd_soc_device jive_snd_devdata = {
 	.card		= &snd_soc_machine_jive,
 	.codec_dev	= &soc_codec_dev_wm8750,
-	.codec_data	= &jive_wm8750_setup,
 };
 
 static struct platform_device *jive_snd_device;
-- 
1.7.0

Re: [PATCH] WM8750: Convert to new API

[Liam Girdwood]

On Thu, 2010-04-08 at 20:48 +0200, Marek Vasut wrote:
> Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
> around. Hugely inspired by WM8753 which was already converted.
> 
> Also, this patch fixes the Jive and Spitz machine.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH] WM8750: Convert to new API

[Mark Brown]

On Thu, Apr 08, 2010 at 08:48:51PM +0200, Marek Vasut wrote:
> Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
> around. Hugely inspired by WM8753 which was already converted.
> 
> Also, this patch fixes the Jive and Spitz machine.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

Applied, thanks.

[PATCH] WM8750: Convert to new API

[Marek Vasut]

Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
around. Hugely inspired by WM8753 which was already converted.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 sound/soc/codecs/wm8750.c       |  295 ++++++++++++++++++---------------------
 sound/soc/pxa/spitz.c           |   43 +++++-
 sound/soc/s3c24xx/jive_wm8750.c |   40 +++++-
 3 files changed, 210 insertions(+), 168 deletions(-)

diff --git a/sound/soc/codecs/wm8750.c b/sound/soc/codecs/wm8750.c
index 3e440c0..eea7ba6 100644
--- a/sound/soc/codecs/wm8750.c
+++ b/sound/soc/codecs/wm8750.c
@@ -30,11 +30,6 @@
 
 #include "wm8750.h"
 
-/* codec private data */
-struct wm8750_priv {
-	unsigned int sysclk;
-};
-
 /*
  * wm8750 register cache
  * We can't read the WM8750 register space when we
@@ -54,6 +49,13 @@ static const u16 wm8750_reg[] = {
 	0x0079, 0x0079, 0x0079,          /* 40 */
 };
 
+/* codec private data */
+struct wm8750_priv {
+	unsigned int sysclk;
+	struct snd_soc_codec codec;
+	u16 reg_cache[ARRAY_SIZE(wm8750_reg)];
+};
+
 #define wm8750_reset(c)	snd_soc_write(c, WM8750_RESET, 0)
 
 /*
@@ -696,25 +698,90 @@ static int wm8750_resume(struct platform_device *pdev)
 	return 0;
 }
 
+static struct snd_soc_codec *wm8750_codec;
+
+static int wm8750_probe(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+	struct snd_soc_codec *codec;
+	int ret = 0;
+
+	if (!wm8750_codec) {
+		dev_err(&pdev->dev, "WM8750 codec not yet registered\n");
+		return -EINVAL;
+	}
+
+	socdev->card->codec = wm8750_codec;
+	codec = wm8750_codec;
+
+	/* register pcms */
+	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
+	if (ret < 0) {
+		printk(KERN_ERR "wm8750: failed to create pcms\n");
+		goto err;
+	}
+
+	snd_soc_add_controls(codec, wm8750_snd_controls,
+				ARRAY_SIZE(wm8750_snd_controls));
+	wm8750_add_widgets(codec);
+
+	return 0;
+
+err:
+	return ret;
+}
+
+/* power down chip */
+static int wm8750_remove(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+
+	snd_soc_free_pcms(socdev);
+	snd_soc_dapm_free(socdev);
+
+	return 0;
+}
+
+struct snd_soc_codec_device soc_codec_dev_wm8750 = {
+	.probe		= wm8750_probe,
+	.remove		= wm8750_remove,
+	.suspend	= wm8750_suspend,
+	.resume		= wm8750_resume,
+};
+EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
+
 /*
  * initialise the WM8750 driver
  * register the mixer and dsp interfaces with the kernel
  */
-static int wm8750_init(struct snd_soc_device *socdev,
-		       enum snd_soc_control_type control)
+static int wm8750_register(struct wm8750_priv *wm8750,
+			enum snd_soc_control_type control)
 {
-	struct snd_soc_codec *codec = socdev->card->codec;
+	struct snd_soc_codec *codec = &wm8750->codec;
 	int reg, ret = 0;
 
+	if (wm8750_codec) {
+		dev_err(codec->dev, "Multiple WM8750 devices not supported\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	mutex_init(&codec->mutex);
+	INIT_LIST_HEAD(&codec->dapm_widgets);
+	INIT_LIST_HEAD(&codec->dapm_paths);
+
 	codec->name = "WM8750";
 	codec->owner = THIS_MODULE;
+	codec->bias_level = SND_SOC_BIAS_STANDBY;
 	codec->set_bias_level = wm8750_set_bias_level;
 	codec->dai = &wm8750_dai;
 	codec->num_dai = 1;
-	codec->reg_cache_size = ARRAY_SIZE(wm8750_reg);
-	codec->reg_cache = kmemdup(wm8750_reg, sizeof(wm8750_reg), GFP_KERNEL);
-	if (codec->reg_cache == NULL)
-		return -ENOMEM;
+	codec->private_data = wm8750;
+	codec->reg_cache_size = ARRAY_SIZE(wm8750->reg_cache) + 1;
+	codec->reg_cache = &wm8750->reg_cache;
+	codec->private_data = wm8750;
+
+	memcpy(codec->reg_cache, wm8750_reg, sizeof(wm8750->reg_cache));
 
 	ret = snd_soc_codec_set_cache_io(codec, 7, 9, control);
 	if (ret < 0) {
@@ -728,13 +795,6 @@ static int wm8750_init(struct snd_soc_device *socdev,
 		goto err;
 	}
 
-	/* register pcms */
-	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
-	if (ret < 0) {
-		printk(KERN_ERR "wm8750: failed to create pcms\n");
-		goto err;
-	}
-
 	/* charge output caps */
 	wm8750_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
@@ -756,19 +816,37 @@ static int wm8750_init(struct snd_soc_device *socdev,
 	reg = snd_soc_read(codec, WM8750_RINVOL);
 	snd_soc_write(codec, WM8750_RINVOL, reg | 0x0100);
 
-	snd_soc_add_controls(codec, wm8750_snd_controls,
-				ARRAY_SIZE(wm8750_snd_controls));
-	wm8750_add_widgets(codec);
-	return ret;
+	wm8750_codec = codec;
 
+	ret = snd_soc_register_codec(codec);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
+		goto err;
+	}
+
+	ret = snd_soc_register_dais(&wm8750_dai, 1);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register DAIs: %d\n", ret);
+		goto err_codec;
+	}
+
+	return 0;
+
+err_codec:
+	snd_soc_unregister_codec(codec);
 err:
-	kfree(codec->reg_cache);
+	kfree(wm8750);
 	return ret;
 }
 
-/* If the i2c layer weren't so broken, we could pass this kind of data
-   around */
-static struct snd_soc_device *wm8750_socdev;
+static void wm8750_unregister(struct wm8750_priv *wm8750)
+{
+	wm8750_set_bias_level(&wm8750->codec, SND_SOC_BIAS_OFF);
+	snd_soc_unregister_dais(&wm8750_dai, 1);
+	snd_soc_unregister_codec(&wm8750->codec);
+	kfree(wm8750);
+	wm8750_codec = NULL;
+}
 
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
 
@@ -782,24 +860,26 @@ static struct snd_soc_device *wm8750_socdev;
 static int wm8750_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
-	i2c_set_clientdata(i2c, codec);
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
+
+	codec = &wm8750->codec;
 	codec->control_data = i2c;
+	i2c_set_clientdata(i2c, wm8750);
 
-	ret = wm8750_init(socdev, SND_SOC_I2C);
-	if (ret < 0)
-		pr_err("failed to initialise WM8750\n");
+	codec->dev = &i2c->dev;
 
-	return ret;
+	return wm8750_register(wm8750, SND_SOC_I2C);
 }
 
 static int wm8750_i2c_remove(struct i2c_client *client)
 {
-	struct snd_soc_codec *codec = i2c_get_clientdata(client);
-	kfree(codec->reg_cache);
+	struct wm8750_priv *wm8750 = i2c_get_clientdata(client);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -818,66 +898,31 @@ static struct i2c_driver wm8750_i2c_driver = {
 	.remove =   wm8750_i2c_remove,
 	.id_table = wm8750_i2c_id,
 };
-
-static int wm8750_add_i2c_device(struct platform_device *pdev,
-				 const struct wm8750_setup_data *setup)
-{
-	struct i2c_board_info info;
-	struct i2c_adapter *adapter;
-	struct i2c_client *client;
-	int ret;
-
-	ret = i2c_add_driver(&wm8750_i2c_driver);
-	if (ret != 0) {
-		dev_err(&pdev->dev, "can't add i2c driver\n");
-		return ret;
-	}
-
-	memset(&info, 0, sizeof(struct i2c_board_info));
-	info.addr = setup->i2c_address;
-	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
-
-	adapter = i2c_get_adapter(setup->i2c_bus);
-	if (!adapter) {
-		dev_err(&pdev->dev, "can't get i2c adapter %d\n",
-			setup->i2c_bus);
-		goto err_driver;
-	}
-
-	client = i2c_new_device(adapter, &info);
-	i2c_put_adapter(adapter);
-	if (!client) {
-		dev_err(&pdev->dev, "can't add i2c device at 0x%x\n",
-			(unsigned int)info.addr);
-		goto err_driver;
-	}
-
-	return 0;
-
-err_driver:
-	i2c_del_driver(&wm8750_i2c_driver);
-	return -ENODEV;
-}
 #endif
 
 #if defined(CONFIG_SPI_MASTER)
 static int __devinit wm8750_spi_probe(struct spi_device *spi)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
+
+	codec = &wm8750->codec;
 	codec->control_data = spi;
+	codec->dev = &spi->dev;
 
-	ret = wm8750_init(socdev, SND_SOC_SPI);
-	if (ret < 0)
-		dev_err(&spi->dev, "failed to initialise WM8750\n");
+	dev_set_drvdata(&spi->dev, wm8750);
 
-	return ret;
+	return wm8750_register(wm8750, SND_SOC_SPI);
 }
 
 static int __devexit wm8750_spi_remove(struct spi_device *spi)
 {
+	struct wm8750_priv *wm8750 = dev_get_drvdata(&spi->dev);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -892,93 +937,31 @@ static struct spi_driver wm8750_spi_driver = {
 };
 #endif
 
-static int wm8750_probe(struct platform_device *pdev)
+static int __init wm8750_modinit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct wm8750_setup_data *setup = socdev->codec_data;
-	struct snd_soc_codec *codec;
-	struct wm8750_priv *wm8750;
 	int ret;
-
-	codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL);
-	if (codec == NULL)
-		return -ENOMEM;
-
-	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
-	if (wm8750 == NULL) {
-		kfree(codec);
-		return -ENOMEM;
-	}
-
-	codec->private_data = wm8750;
-	socdev->card->codec = codec;
-	mutex_init(&codec->mutex);
-	INIT_LIST_HEAD(&codec->dapm_widgets);
-	INIT_LIST_HEAD(&codec->dapm_paths);
-	wm8750_socdev = socdev;
-
-	ret = -ENODEV;
-
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	if (setup->i2c_address) {
-		ret = wm8750_add_i2c_device(pdev, setup);
-	}
+	ret = i2c_add_driver(&wm8750_i2c_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 I2C driver: %d\n", ret);
 #endif
 #if defined(CONFIG_SPI_MASTER)
-	if (setup->spi) {
-		ret = spi_register_driver(&wm8750_spi_driver);
-		if (ret != 0)
-			printk(KERN_ERR "can't add spi driver");
-	}
+	ret = spi_register_driver(&wm8750_spi_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 SPI driver: %d\n", ret);
 #endif
-
-	if (ret != 0) {
-		kfree(codec->private_data);
-		kfree(codec);
-	}
-	return ret;
+	return 0;
 }
+module_init(wm8750_modinit);
 
-/* power down chip */
-static int wm8750_remove(struct platform_device *pdev)
+static void __exit wm8750_exit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct snd_soc_codec *codec = socdev->card->codec;
-
-	if (codec->control_data)
-		wm8750_set_bias_level(codec, SND_SOC_BIAS_OFF);
-	snd_soc_free_pcms(socdev);
-	snd_soc_dapm_free(socdev);
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	i2c_unregister_device(codec->control_data);
 	i2c_del_driver(&wm8750_i2c_driver);
 #endif
 #if defined(CONFIG_SPI_MASTER)
 	spi_unregister_driver(&wm8750_spi_driver);
 #endif
-	kfree(codec->private_data);
-	kfree(codec);
-
-	return 0;
-}
-
-struct snd_soc_codec_device soc_codec_dev_wm8750 = {
-	.probe = 	wm8750_probe,
-	.remove = 	wm8750_remove,
-	.suspend = 	wm8750_suspend,
-	.resume =	wm8750_resume,
-};
-EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
-
-static int __init wm8750_modinit(void)
-{
-	return snd_soc_register_dai(&wm8750_dai);
-}
-module_init(wm8750_modinit);
-
-static void __exit wm8750_exit(void)
-{
-	snd_soc_unregister_dai(&wm8750_dai);
 }
 module_exit(wm8750_exit);
 
diff --git a/sound/soc/pxa/spitz.c b/sound/soc/pxa/spitz.c
index c4cd2ac..1941a35 100644
--- a/sound/soc/pxa/spitz.c
+++ b/sound/soc/pxa/spitz.c
@@ -322,19 +322,44 @@ static struct snd_soc_card snd_soc_spitz = {
 	.num_links = 1,
 };
 
-/* spitz audio private data */
-static struct wm8750_setup_data spitz_wm8750_setup = {
-	.i2c_bus = 0,
-	.i2c_address = 0x1b,
-};
-
 /* spitz audio subsystem */
 static struct snd_soc_device spitz_snd_devdata = {
 	.card = &snd_soc_spitz,
 	.codec_dev = &soc_codec_dev_wm8750,
-	.codec_data = &spitz_wm8750_setup,
 };
 
+/*
+ * FIXME: This is a temporary bodge to avoid cross-tree merge issues.
+ * New drivers should register the wm8750 I2C device in the machine
+ * setup code (under arch/arm for ARM systems).
+ */
+static int wm8750_i2c_register(void)
+{
+	struct i2c_board_info info;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+
+	memset(&info, 0, sizeof(struct i2c_board_info));
+	info.addr = 0x1b;
+	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
+
+	adapter = i2c_get_adapter(0);
+	if (!adapter) {
+		printk(KERN_ERR "can't get i2c adapter 0\n");
+		return -ENODEV;
+	}
+
+	client = i2c_new_device(adapter, &info);
+	i2c_put_adapter(adapter);
+	if (!client) {
+		printk(KERN_ERR "can't add i2c device at 0x%x\n",
+		(unsigned int)info.addr);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static struct platform_device *spitz_snd_device;
 
 static int __init spitz_init(void)
@@ -344,6 +369,10 @@ static int __init spitz_init(void)
 	if (!(machine_is_spitz() || machine_is_borzoi() || machine_is_akita()))
 		return -ENODEV;
 
+	ret = wm8750_i2c_setup();
+	if (ret != 0)
+		return ret;
+
 	spitz_snd_device = platform_device_alloc("soc-audio", -1);
 	if (!spitz_snd_device)
 		return -ENOMEM;
diff --git a/sound/soc/s3c24xx/jive_wm8750.c b/sound/soc/s3c24xx/jive_wm8750.c
index 59dc2c6..b9d85c6 100644
--- a/sound/soc/s3c24xx/jive_wm8750.c
+++ b/sound/soc/s3c24xx/jive_wm8750.c
@@ -152,17 +152,43 @@ static struct snd_soc_card snd_soc_machine_jive = {
 	.num_links	= 1,
 };
 
-/* jive audio private data */
-static struct wm8750_setup_data jive_wm8750_setup = {
-};
-
 /* jive audio subsystem */
 static struct snd_soc_device jive_snd_devdata = {
 	.card		= &snd_soc_machine_jive,
 	.codec_dev	= &soc_codec_dev_wm8750,
-	.codec_data	= &jive_wm8750_setup,
 };
 
+/*
+ * FIXME: This is a temporary bodge to avoid cross-tree merge issues.
+ * New drivers should register the wm8750 I2C device in the machine
+ * setup code (under arch/arm for ARM systems).
+ */
+static int wm8750_i2c_register(void)
+{
+	struct i2c_board_info info;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+
+	memset(&info, 0, sizeof(struct i2c_board_info));
+	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
+
+	adapter = i2c_get_adapter(0);
+	if (!adapter) {
+		printk(KERN_ERR "can't get i2c adapter 0\n");
+		return -ENODEV;
+	}
+
+	client = i2c_new_device(adapter, &info);
+	i2c_put_adapter(adapter);
+	if (!client) {
+		printk(KERN_ERR "can't add i2c device at 0x%x\n",
+		(unsigned int)info.addr);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static struct platform_device *jive_snd_device;
 
 static int __init jive_init(void)
@@ -172,6 +198,10 @@ static int __init jive_init(void)
 	if (!machine_is_jive())
 		return 0;
 
+	ret = wm8750_i2c_setup();
+	if (ret != 0)
+		return ret;
+
 	printk("JIVE WM8750 Audio support\n");
 
 	jive_snd_device = platform_device_alloc("soc-audio", -1);
-- 
1.7.0

Re: [PATCH] WM8750: Convert to new API

[Mark Brown]

On Thu, Apr 08, 2010 at 04:59:09PM +0200, Marek Vasut wrote:

> @@ -172,6 +198,10 @@ static int __init jive_init(void)
>  	if (!machine_is_jive())
>  		return 0;
>  
> +	ret = wm8750_i2c_setup();
> +	if (ret != 0)
> +		return ret;
> +

Quoting from my mail of 15:58 (5 minutes ago!):

| The WM8750 on the JIVE is connected using SPI, not I2C.  You'll want to
| CC in the JIVE and S3C maintainer too (Ben Dooks for both).  Looking at
| the Jive machine file it's actually registering the SPI device in
| arch/arm already so there shouldn't be any need to add any registration
| code at all.

To repeat, the WM8750 on JIVE is connected using SPI, not I2C.  This
means you should not be registering an I2C device.

Re: [PATCH 2/3] WM8750: Fix SPITZ machine

[Mark Brown]

On Mon, Apr 05, 2010 at 06:13:37AM +0200, Marek Vasut wrote:
> This patch registers the WM8750 codec on a proper place on the SPITZ machine
> after the WM8750 driver was converted to new API. Moreover, the incorrect code

We also need this for the Jive machine.  FWIW I'm checking for affected
machines by doing 'grep -l wm8750 sound/soc/*/*.c'.

> is hereby removed from the aSoC driver.

ASoC.

> diff --git a/sound/soc/pxa/spitz.c b/sound/soc/pxa/spitz.c
> index c4cd2ac..d256f5f 100644
> --- a/sound/soc/pxa/spitz.c
> +++ b/sound/soc/pxa/spitz.c
> @@ -322,17 +322,10 @@ static struct snd_soc_card snd_soc_spitz = {
>  	.num_links = 1,
>  };
>  
> -/* spitz audio private data */
> -static struct wm8750_setup_data spitz_wm8750_setup = {
> -	.i2c_bus = 0,
> -	.i2c_address = 0x1b,
> -};
> -
>  /* spitz audio subsystem */
>  static struct snd_soc_device spitz_snd_devdata = {
>  	.card = &snd_soc_spitz,
>  	.codec_dev = &soc_codec_dev_wm8750,
> -	.codec_data = &spitz_wm8750_setup,
>  };

So, this is going to break spitz if not merged in with the patch to
update wm8750 since prior to the conversion of the driver the setup data
is required while after the conversion the setup data structure no
longer exists and will therefore cause compile failures.

As I've mentioned several times before I urge you to look at how this
transition was done for wm8731 for an example of how this sort of change
can be done without too much disruption.

[PATCH] WM8750: Convert to new API

[Marek Vasut]

Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
around. Hugely inspired by WM8753 which was already converted.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 sound/soc/codecs/wm8750.c       |  295 ++++++++++++++++++---------------------
 sound/soc/pxa/spitz.c           |   43 +++++-
 sound/soc/s3c24xx/jive_wm8750.c |   40 +++++-
 3 files changed, 210 insertions(+), 168 deletions(-)

diff --git a/sound/soc/codecs/wm8750.c b/sound/soc/codecs/wm8750.c
index 3e440c0..eea7ba6 100644
--- a/sound/soc/codecs/wm8750.c
+++ b/sound/soc/codecs/wm8750.c
@@ -30,11 +30,6 @@
 
 #include "wm8750.h"
 
-/* codec private data */
-struct wm8750_priv {
-	unsigned int sysclk;
-};
-
 /*
  * wm8750 register cache
  * We can't read the WM8750 register space when we
@@ -54,6 +49,13 @@ static const u16 wm8750_reg[] = {
 	0x0079, 0x0079, 0x0079,          /* 40 */
 };
 
+/* codec private data */
+struct wm8750_priv {
+	unsigned int sysclk;
+	struct snd_soc_codec codec;
+	u16 reg_cache[ARRAY_SIZE(wm8750_reg)];
+};
+
 #define wm8750_reset(c)	snd_soc_write(c, WM8750_RESET, 0)
 
 /*
@@ -696,25 +698,90 @@ static int wm8750_resume(struct platform_device *pdev)
 	return 0;
 }
 
+static struct snd_soc_codec *wm8750_codec;
+
+static int wm8750_probe(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+	struct snd_soc_codec *codec;
+	int ret = 0;
+
+	if (!wm8750_codec) {
+		dev_err(&pdev->dev, "WM8750 codec not yet registered\n");
+		return -EINVAL;
+	}
+
+	socdev->card->codec = wm8750_codec;
+	codec = wm8750_codec;
+
+	/* register pcms */
+	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
+	if (ret < 0) {
+		printk(KERN_ERR "wm8750: failed to create pcms\n");
+		goto err;
+	}
+
+	snd_soc_add_controls(codec, wm8750_snd_controls,
+				ARRAY_SIZE(wm8750_snd_controls));
+	wm8750_add_widgets(codec);
+
+	return 0;
+
+err:
+	return ret;
+}
+
+/* power down chip */
+static int wm8750_remove(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+
+	snd_soc_free_pcms(socdev);
+	snd_soc_dapm_free(socdev);
+
+	return 0;
+}
+
+struct snd_soc_codec_device soc_codec_dev_wm8750 = {
+	.probe		= wm8750_probe,
+	.remove		= wm8750_remove,
+	.suspend	= wm8750_suspend,
+	.resume		= wm8750_resume,
+};
+EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
+
 /*
  * initialise the WM8750 driver
  * register the mixer and dsp interfaces with the kernel
  */
-static int wm8750_init(struct snd_soc_device *socdev,
-		       enum snd_soc_control_type control)
+static int wm8750_register(struct wm8750_priv *wm8750,
+			enum snd_soc_control_type control)
 {
-	struct snd_soc_codec *codec = socdev->card->codec;
+	struct snd_soc_codec *codec = &wm8750->codec;
 	int reg, ret = 0;
 
+	if (wm8750_codec) {
+		dev_err(codec->dev, "Multiple WM8750 devices not supported\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	mutex_init(&codec->mutex);
+	INIT_LIST_HEAD(&codec->dapm_widgets);
+	INIT_LIST_HEAD(&codec->dapm_paths);
+
 	codec->name = "WM8750";
 	codec->owner = THIS_MODULE;
+	codec->bias_level = SND_SOC_BIAS_STANDBY;
 	codec->set_bias_level = wm8750_set_bias_level;
 	codec->dai = &wm8750_dai;
 	codec->num_dai = 1;
-	codec->reg_cache_size = ARRAY_SIZE(wm8750_reg);
-	codec->reg_cache = kmemdup(wm8750_reg, sizeof(wm8750_reg), GFP_KERNEL);
-	if (codec->reg_cache == NULL)
-		return -ENOMEM;
+	codec->private_data = wm8750;
+	codec->reg_cache_size = ARRAY_SIZE(wm8750->reg_cache) + 1;
+	codec->reg_cache = &wm8750->reg_cache;
+	codec->private_data = wm8750;
+
+	memcpy(codec->reg_cache, wm8750_reg, sizeof(wm8750->reg_cache));
 
 	ret = snd_soc_codec_set_cache_io(codec, 7, 9, control);
 	if (ret < 0) {
@@ -728,13 +795,6 @@ static int wm8750_init(struct snd_soc_device *socdev,
 		goto err;
 	}
 
-	/* register pcms */
-	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
-	if (ret < 0) {
-		printk(KERN_ERR "wm8750: failed to create pcms\n");
-		goto err;
-	}
-
 	/* charge output caps */
 	wm8750_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
@@ -756,19 +816,37 @@ static int wm8750_init(struct snd_soc_device *socdev,
 	reg = snd_soc_read(codec, WM8750_RINVOL);
 	snd_soc_write(codec, WM8750_RINVOL, reg | 0x0100);
 
-	snd_soc_add_controls(codec, wm8750_snd_controls,
-				ARRAY_SIZE(wm8750_snd_controls));
-	wm8750_add_widgets(codec);
-	return ret;
+	wm8750_codec = codec;
 
+	ret = snd_soc_register_codec(codec);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
+		goto err;
+	}
+
+	ret = snd_soc_register_dais(&wm8750_dai, 1);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register DAIs: %d\n", ret);
+		goto err_codec;
+	}
+
+	return 0;
+
+err_codec:
+	snd_soc_unregister_codec(codec);
 err:
-	kfree(codec->reg_cache);
+	kfree(wm8750);
 	return ret;
 }
 
-/* If the i2c layer weren't so broken, we could pass this kind of data
-   around */
-static struct snd_soc_device *wm8750_socdev;
+static void wm8750_unregister(struct wm8750_priv *wm8750)
+{
+	wm8750_set_bias_level(&wm8750->codec, SND_SOC_BIAS_OFF);
+	snd_soc_unregister_dais(&wm8750_dai, 1);
+	snd_soc_unregister_codec(&wm8750->codec);
+	kfree(wm8750);
+	wm8750_codec = NULL;
+}
 
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
 
@@ -782,24 +860,26 @@ static struct snd_soc_device *wm8750_socdev;
 static int wm8750_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
-	i2c_set_clientdata(i2c, codec);
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
+
+	codec = &wm8750->codec;
 	codec->control_data = i2c;
+	i2c_set_clientdata(i2c, wm8750);
 
-	ret = wm8750_init(socdev, SND_SOC_I2C);
-	if (ret < 0)
-		pr_err("failed to initialise WM8750\n");
+	codec->dev = &i2c->dev;
 
-	return ret;
+	return wm8750_register(wm8750, SND_SOC_I2C);
 }
 
 static int wm8750_i2c_remove(struct i2c_client *client)
 {
-	struct snd_soc_codec *codec = i2c_get_clientdata(client);
-	kfree(codec->reg_cache);
+	struct wm8750_priv *wm8750 = i2c_get_clientdata(client);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -818,66 +898,31 @@ static struct i2c_driver wm8750_i2c_driver = {
 	.remove =   wm8750_i2c_remove,
 	.id_table = wm8750_i2c_id,
 };
-
-static int wm8750_add_i2c_device(struct platform_device *pdev,
-				 const struct wm8750_setup_data *setup)
-{
-	struct i2c_board_info info;
-	struct i2c_adapter *adapter;
-	struct i2c_client *client;
-	int ret;
-
-	ret = i2c_add_driver(&wm8750_i2c_driver);
-	if (ret != 0) {
-		dev_err(&pdev->dev, "can't add i2c driver\n");
-		return ret;
-	}
-
-	memset(&info, 0, sizeof(struct i2c_board_info));
-	info.addr = setup->i2c_address;
-	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
-
-	adapter = i2c_get_adapter(setup->i2c_bus);
-	if (!adapter) {
-		dev_err(&pdev->dev, "can't get i2c adapter %d\n",
-			setup->i2c_bus);
-		goto err_driver;
-	}
-
-	client = i2c_new_device(adapter, &info);
-	i2c_put_adapter(adapter);
-	if (!client) {
-		dev_err(&pdev->dev, "can't add i2c device at 0x%x\n",
-			(unsigned int)info.addr);
-		goto err_driver;
-	}
-
-	return 0;
-
-err_driver:
-	i2c_del_driver(&wm8750_i2c_driver);
-	return -ENODEV;
-}
 #endif
 
 #if defined(CONFIG_SPI_MASTER)
 static int __devinit wm8750_spi_probe(struct spi_device *spi)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
+
+	codec = &wm8750->codec;
 	codec->control_data = spi;
+	codec->dev = &spi->dev;
 
-	ret = wm8750_init(socdev, SND_SOC_SPI);
-	if (ret < 0)
-		dev_err(&spi->dev, "failed to initialise WM8750\n");
+	dev_set_drvdata(&spi->dev, wm8750);
 
-	return ret;
+	return wm8750_register(wm8750, SND_SOC_SPI);
 }
 
 static int __devexit wm8750_spi_remove(struct spi_device *spi)
 {
+	struct wm8750_priv *wm8750 = dev_get_drvdata(&spi->dev);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -892,93 +937,31 @@ static struct spi_driver wm8750_spi_driver = {
 };
 #endif
 
-static int wm8750_probe(struct platform_device *pdev)
+static int __init wm8750_modinit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct wm8750_setup_data *setup = socdev->codec_data;
-	struct snd_soc_codec *codec;
-	struct wm8750_priv *wm8750;
 	int ret;
-
-	codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL);
-	if (codec == NULL)
-		return -ENOMEM;
-
-	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
-	if (wm8750 == NULL) {
-		kfree(codec);
-		return -ENOMEM;
-	}
-
-	codec->private_data = wm8750;
-	socdev->card->codec = codec;
-	mutex_init(&codec->mutex);
-	INIT_LIST_HEAD(&codec->dapm_widgets);
-	INIT_LIST_HEAD(&codec->dapm_paths);
-	wm8750_socdev = socdev;
-
-	ret = -ENODEV;
-
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	if (setup->i2c_address) {
-		ret = wm8750_add_i2c_device(pdev, setup);
-	}
+	ret = i2c_add_driver(&wm8750_i2c_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 I2C driver: %d\n", ret);
 #endif
 #if defined(CONFIG_SPI_MASTER)
-	if (setup->spi) {
-		ret = spi_register_driver(&wm8750_spi_driver);
-		if (ret != 0)
-			printk(KERN_ERR "can't add spi driver");
-	}
+	ret = spi_register_driver(&wm8750_spi_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 SPI driver: %d\n", ret);
 #endif
-
-	if (ret != 0) {
-		kfree(codec->private_data);
-		kfree(codec);
-	}
-	return ret;
+	return 0;
 }
+module_init(wm8750_modinit);
 
-/* power down chip */
-static int wm8750_remove(struct platform_device *pdev)
+static void __exit wm8750_exit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct snd_soc_codec *codec = socdev->card->codec;
-
-	if (codec->control_data)
-		wm8750_set_bias_level(codec, SND_SOC_BIAS_OFF);
-	snd_soc_free_pcms(socdev);
-	snd_soc_dapm_free(socdev);
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	i2c_unregister_device(codec->control_data);
 	i2c_del_driver(&wm8750_i2c_driver);
 #endif
 #if defined(CONFIG_SPI_MASTER)
 	spi_unregister_driver(&wm8750_spi_driver);
 #endif
-	kfree(codec->private_data);
-	kfree(codec);
-
-	return 0;
-}
-
-struct snd_soc_codec_device soc_codec_dev_wm8750 = {
-	.probe = 	wm8750_probe,
-	.remove = 	wm8750_remove,
-	.suspend = 	wm8750_suspend,
-	.resume =	wm8750_resume,
-};
-EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
-
-static int __init wm8750_modinit(void)
-{
-	return snd_soc_register_dai(&wm8750_dai);
-}
-module_init(wm8750_modinit);
-
-static void __exit wm8750_exit(void)
-{
-	snd_soc_unregister_dai(&wm8750_dai);
 }
 module_exit(wm8750_exit);
 
diff --git a/sound/soc/pxa/spitz.c b/sound/soc/pxa/spitz.c
index c4cd2ac..1941a35 100644
--- a/sound/soc/pxa/spitz.c
+++ b/sound/soc/pxa/spitz.c
@@ -322,19 +322,44 @@ static struct snd_soc_card snd_soc_spitz = {
 	.num_links = 1,
 };
 
-/* spitz audio private data */
-static struct wm8750_setup_data spitz_wm8750_setup = {
-	.i2c_bus = 0,
-	.i2c_address = 0x1b,
-};
-
 /* spitz audio subsystem */
 static struct snd_soc_device spitz_snd_devdata = {
 	.card = &snd_soc_spitz,
 	.codec_dev = &soc_codec_dev_wm8750,
-	.codec_data = &spitz_wm8750_setup,
 };
 
+/*
+ * FIXME: This is a temporary bodge to avoid cross-tree merge issues.
+ * New drivers should register the wm8750 I2C device in the machine
+ * setup code (under arch/arm for ARM systems).
+ */
+static int wm8750_i2c_register(void)
+{
+	struct i2c_board_info info;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+
+	memset(&info, 0, sizeof(struct i2c_board_info));
+	info.addr = 0x1b;
+	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
+
+	adapter = i2c_get_adapter(0);
+	if (!adapter) {
+		printk(KERN_ERR "can't get i2c adapter 0\n");
+		return -ENODEV;
+	}
+
+	client = i2c_new_device(adapter, &info);
+	i2c_put_adapter(adapter);
+	if (!client) {
+		printk(KERN_ERR "can't add i2c device at 0x%x\n",
+		(unsigned int)info.addr);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static struct platform_device *spitz_snd_device;
 
 static int __init spitz_init(void)
@@ -344,6 +369,10 @@ static int __init spitz_init(void)
 	if (!(machine_is_spitz() || machine_is_borzoi() || machine_is_akita()))
 		return -ENODEV;
 
+	ret = wm8750_i2c_setup();
+	if (ret != 0)
+		return ret;
+
 	spitz_snd_device = platform_device_alloc("soc-audio", -1);
 	if (!spitz_snd_device)
 		return -ENOMEM;
diff --git a/sound/soc/s3c24xx/jive_wm8750.c b/sound/soc/s3c24xx/jive_wm8750.c
index 59dc2c6..b9d85c6 100644
--- a/sound/soc/s3c24xx/jive_wm8750.c
+++ b/sound/soc/s3c24xx/jive_wm8750.c
@@ -152,17 +152,43 @@ static struct snd_soc_card snd_soc_machine_jive = {
 	.num_links	= 1,
 };
 
-/* jive audio private data */
-static struct wm8750_setup_data jive_wm8750_setup = {
-};
-
 /* jive audio subsystem */
 static struct snd_soc_device jive_snd_devdata = {
 	.card		= &snd_soc_machine_jive,
 	.codec_dev	= &soc_codec_dev_wm8750,
-	.codec_data	= &jive_wm8750_setup,
 };
 
+/*
+ * FIXME: This is a temporary bodge to avoid cross-tree merge issues.
+ * New drivers should register the wm8750 I2C device in the machine
+ * setup code (under arch/arm for ARM systems).
+ */
+static int wm8750_i2c_register(void)
+{
+	struct i2c_board_info info;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+
+	memset(&info, 0, sizeof(struct i2c_board_info));
+	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
+
+	adapter = i2c_get_adapter(0);
+	if (!adapter) {
+		printk(KERN_ERR "can't get i2c adapter 0\n");
+		return -ENODEV;
+	}
+
+	client = i2c_new_device(adapter, &info);
+	i2c_put_adapter(adapter);
+	if (!client) {
+		printk(KERN_ERR "can't add i2c device at 0x%x\n",
+		(unsigned int)info.addr);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static struct platform_device *jive_snd_device;
 
 static int __init jive_init(void)
@@ -172,6 +198,10 @@ static int __init jive_init(void)
 	if (!machine_is_jive())
 		return 0;
 
+	ret = wm8750_i2c_setup();
+	if (ret != 0)
+		return ret;
+
 	printk("JIVE WM8750 Audio support\n");
 
 	jive_snd_device = platform_device_alloc("soc-audio", -1);
-- 
1.7.0

Re: [PATCH] WM8750: Convert to new API

[Mark Brown]

On Thu, Apr 08, 2010 at 04:51:49PM +0200, Marek Vasut wrote:

> @@ -172,6 +198,10 @@ static int __init jive_init(void)
>  	if (!machine_is_jive())
>  		return 0;
>  
> +	ret = wm8750_i2c_setup();
> +	if (ret != 0)
> +		return ret;
> +

The WM8750 on the JIVE is connected using SPI, not I2C.  You'll want to
CC in the JIVE and S3C maintainer too (Ben Dooks for both).  Looking at
the Jive machine file it's actually registering the SPI device in
arch/arm already so there shouldn't be any need to add any registration
code at all.

[PATCH] WM8750: Convert to new API

[Marek Vasut]

Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
around. Hugely inspired by WM8753 which was already converted.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 sound/soc/codecs/wm8750.c |  411 +++++++++++++++++++++++++--------------------
 1 files changed, 232 insertions(+), 179 deletions(-)

diff --git a/sound/soc/codecs/wm8750.c b/sound/soc/codecs/wm8750.c
index 475c67a..c7ee6d2 100644
--- a/sound/soc/codecs/wm8750.c
+++ b/sound/soc/codecs/wm8750.c
@@ -29,13 +29,6 @@
 
 #include "wm8750.h"
 
-#define WM8750_VERSION "0.12"
-
-/* codec private data */
-struct wm8750_priv {
-	unsigned int sysclk;
-};
-
 /*
  * wm8750 register cache
  * We can't read the WM8750 register space when we
@@ -55,6 +48,59 @@ static const u16 wm8750_reg[] = {
 	0x0079, 0x0079, 0x0079,          /* 40 */
 };
 
+/* codec private data */
+struct wm8750_priv {
+	unsigned int sysclk;
+	struct snd_soc_codec codec;
+	u16 reg_cache[ARRAY_SIZE(wm8750_reg)];
+};
+
+/*
+ * read wm8750 register cache
+ */
+static inline unsigned int wm8750_read_reg_cache(struct snd_soc_codec *codec,
+	unsigned int reg)
+{
+	u16 *cache = codec->reg_cache;
+	if (reg < 1 || reg >= (ARRAY_SIZE(wm8750_reg) + 1))
+		return -1;
+	return cache[reg - 1];
+}
+
+/*
+ * write wm8750 register cache
+ */
+static inline void wm8750_write_reg_cache(struct snd_soc_codec *codec,
+	unsigned int reg, unsigned int value)
+{
+	u16 *cache = codec->reg_cache;
+	if (reg < 1 || reg >= (ARRAY_SIZE(wm8750_reg) + 1))
+		return;
+	cache[reg - 1] = value;
+}
+
+/*
+ * write to the WM8750 register space
+ */
+static int wm8750_write(struct snd_soc_codec *codec, unsigned int reg,
+	unsigned int value)
+{
+	u8 data[2];
+
+	/* data is
+	 *   D15..D9 WM8750 register offset
+	 *   D8...D0 register data
+	 */
+	data[0] = (reg << 1) | ((value >> 8) & 0x0001);
+	data[1] = value & 0x00ff;
+
+	wm8750_write_reg_cache(codec, reg, value);
+	if (codec->hw_write(codec->control_data, data, 2) == 2)
+		return 0;
+	else
+		return -EIO;
+}
+
 #define wm8750_reset(c)	snd_soc_write(c, WM8750_RESET, 0)
 
 /*
@@ -688,6 +734,11 @@ static int wm8750_resume(struct platform_device *pdev)
 	for (i = 0; i < ARRAY_SIZE(wm8750_reg); i++) {
 		if (i == WM8750_RESET)
 			continue;
+
+		/* No point in writing hardware default values back */
+		if (cache[i] == wm8750_reg[i])
+			continue;
+
 		data[0] = (i << 1) | ((cache[i] >> 8) & 0x0001);
 		data[1] = cache[i] & 0x00ff;
 		codec->hw_write(codec->control_data, data, 2);
@@ -706,31 +757,111 @@ static int wm8750_resume(struct platform_device *pdev)
 	return 0;
 }
 
+static struct snd_soc_codec *wm8750_codec;
+
+static int wm8750_probe(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+	struct snd_soc_codec *codec;
+	int ret = 0;
+
+	if (!wm8750_codec) {
+		dev_err(&pdev->dev, "WM8750 codec not yet registered\n");
+		return -EINVAL;
+	}
+
+	socdev->card->codec = wm8750_codec;
+	codec = wm8750_codec;
+
+	/* register pcms */
+	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
+	if (ret < 0) {
+		printk(KERN_ERR "wm8750: failed to create pcms\n");
+		goto err;
+	}
+
+	snd_soc_add_controls(codec, wm8750_snd_controls,
+				ARRAY_SIZE(wm8750_snd_controls));
+	wm8750_add_widgets(codec);
+
+	return 0;
+
+err:
+	return ret;
+}
+
+/*
+ * This function forces any delayed work to be queued and run.
+ */
+static int run_delayed_work(struct delayed_work *dwork)
+{
+	int ret;
+
+	/* cancel any work waiting to be queued. */
+	ret = cancel_delayed_work(dwork);
+
+	/* if there was any work waiting then we run it now and
+	 * wait for it's completion */
+	if (ret) {
+		schedule_delayed_work(dwork, 0);
+		flush_scheduled_work();
+	}
+	return ret;
+}
+
+/* power down chip */
+static int wm8750_remove(struct platform_device *pdev)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+
+	snd_soc_free_pcms(socdev);
+	snd_soc_dapm_free(socdev);
+
+	return 0;
+}
+
+struct snd_soc_codec_device soc_codec_dev_wm8750 = {
+	.probe = 	wm8750_probe,
+	.remove = 	wm8750_remove,
+	.suspend = 	wm8750_suspend,
+	.resume =	wm8750_resume,
+};
+EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
+
 /*
  * initialise the WM8750 driver
  * register the mixer and dsp interfaces with the kernel
  */
-static int wm8750_init(struct snd_soc_device *socdev,
-		       enum snd_soc_control_type control)
+static int wm8750_register(struct wm8750_priv *wm8750)
 {
-	struct snd_soc_codec *codec = socdev->card->codec;
+	struct snd_soc_codec *codec = &wm8750->codec;
 	int reg, ret = 0;
 
+	if (wm8750_codec) {
+		dev_err(codec->dev, "Multiple WM8750 devices not supported\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	mutex_init(&codec->mutex);
+	INIT_LIST_HEAD(&codec->dapm_widgets);
+	INIT_LIST_HEAD(&codec->dapm_paths);
+
 	codec->name = "WM8750";
 	codec->owner = THIS_MODULE;
+	codec->read = wm8750_read_reg_cache;
+	codec->write = wm8750_write;
+	codec->bias_level = SND_SOC_BIAS_STANDBY;
 	codec->set_bias_level = wm8750_set_bias_level;
 	codec->dai = &wm8750_dai;
 	codec->num_dai = 1;
-	codec->reg_cache_size = ARRAY_SIZE(wm8750_reg);
-	codec->reg_cache = kmemdup(wm8750_reg, sizeof(wm8750_reg), GFP_KERNEL);
-	if (codec->reg_cache == NULL)
-		return -ENOMEM;
+	codec->private_data = wm8750;
+	codec->reg_cache_size = ARRAY_SIZE(wm8750->reg_cache) + 1;
+	codec->reg_cache = &wm8750->reg_cache;
+	codec->private_data = wm8750;
 
-	ret = snd_soc_codec_set_cache_io(codec, 7, 9, control);
-	if (ret < 0) {
-		printk(KERN_ERR "wm8750: failed to set cache I/O: %d\n", ret);
-		goto err;
-	}
+	memcpy(codec->reg_cache, wm8750_reg, sizeof(wm8750->reg_cache));
+	INIT_DELAYED_WORK(&codec->delayed_work, wm8750_work);
 
 	ret = wm8750_reset(codec);
 	if (ret < 0) {
@@ -738,13 +869,6 @@ static int wm8750_init(struct snd_soc_device *socdev,
 		goto err;
 	}
 
-	/* register pcms */
-	ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1);
-	if (ret < 0) {
-		printk(KERN_ERR "wm8750: failed to create pcms\n");
-		goto err;
-	}
-
 	/* charge output caps */
 	wm8750_set_bias_level(codec, SND_SOC_BIAS_PREPARE);
 	codec->bias_level = SND_SOC_BIAS_STANDBY;
@@ -768,19 +892,39 @@ static int wm8750_init(struct snd_soc_device *socdev,
 	reg = snd_soc_read(codec, WM8750_RINVOL);
 	snd_soc_write(codec, WM8750_RINVOL, reg | 0x0100);
 
-	snd_soc_add_controls(codec, wm8750_snd_controls,
-				ARRAY_SIZE(wm8750_snd_controls));
-	wm8750_add_widgets(codec);
-	return ret;
+	wm8750_codec = codec;
+
+	ret = snd_soc_register_codec(codec);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
+		goto err;
+	}
+
+	ret = snd_soc_register_dais(&wm8750_dai, 1);
+	if (ret != 0) {
+		dev_err(codec->dev, "Failed to register DAIs: %d\n", ret);
+		goto err_codec;
+	}
+
+	return 0;
 
+err_codec:
+	run_delayed_work(&codec->delayed_work);
+	snd_soc_unregister_codec(codec);
 err:
-	kfree(codec->reg_cache);
+	kfree(wm8750);
 	return ret;
 }
 
-/* If the i2c layer weren't so broken, we could pass this kind of data
-   around */
-static struct snd_soc_device *wm8750_socdev;
+static void wm8750_unregister(struct wm8750_priv *wm8750)
+{
+	wm8750_set_bias_level(&wm8750->codec, SND_SOC_BIAS_OFF);
+	run_delayed_work(&wm8750->codec.delayed_work);
+	snd_soc_unregister_dais(&wm8750_dai, 1);
+	snd_soc_unregister_codec(&wm8750->codec);
+	kfree(wm8750);
+	wm8750_codec = NULL;
+}
 
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
 
@@ -794,24 +938,27 @@ static struct snd_soc_device *wm8750_socdev;
 static int wm8750_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
 
-	i2c_set_clientdata(i2c, codec);
-	codec->control_data = i2c;
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
 
-	ret = wm8750_init(socdev, SND_SOC_I2C);
-	if (ret < 0)
-		pr_err("failed to initialise WM8750\n");
+        codec = &wm8750->codec;
+        codec->hw_write = (hw_write_t)i2c_master_send;
+        codec->control_data = i2c;
+        i2c_set_clientdata(i2c, wm8750);
 
-	return ret;
+        codec->dev = &i2c->dev;
+
+	return wm8750_register(wm8750);
 }
 
 static int wm8750_i2c_remove(struct i2c_client *client)
 {
-	struct snd_soc_codec *codec = i2c_get_clientdata(client);
-	kfree(codec->reg_cache);
+        struct wm8750_priv *wm8750 = i2c_get_clientdata(client);
+        wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -830,66 +977,56 @@ static struct i2c_driver wm8750_i2c_driver = {
 	.remove =   wm8750_i2c_remove,
 	.id_table = wm8750_i2c_id,
 };
+#endif
 
-static int wm8750_add_i2c_device(struct platform_device *pdev,
-				 const struct wm8750_setup_data *setup)
+#if defined(CONFIG_SPI_MASTER)
+static int wm8750_spi_write(struct spi_device *spi, const char *data, int len)
 {
-	struct i2c_board_info info;
-	struct i2c_adapter *adapter;
-	struct i2c_client *client;
-	int ret;
+	struct spi_transfer t;
+	struct spi_message m;
+	u8 msg[2];
 
-	ret = i2c_add_driver(&wm8750_i2c_driver);
-	if (ret != 0) {
-		dev_err(&pdev->dev, "can't add i2c driver\n");
-		return ret;
-	}
+	if (len <= 0)
+		return 0;
 
-	memset(&info, 0, sizeof(struct i2c_board_info));
-	info.addr = setup->i2c_address;
-	strlcpy(info.type, "wm8750", I2C_NAME_SIZE);
+	msg[0] = data[0];
+	msg[1] = data[1];
 
-	adapter = i2c_get_adapter(setup->i2c_bus);
-	if (!adapter) {
-		dev_err(&pdev->dev, "can't get i2c adapter %d\n",
-			setup->i2c_bus);
-		goto err_driver;
-	}
+	spi_message_init(&m);
+	memset(&t, 0, (sizeof t));
 
-	client = i2c_new_device(adapter, &info);
-	i2c_put_adapter(adapter);
-	if (!client) {
-		dev_err(&pdev->dev, "can't add i2c device at 0x%x\n",
-			(unsigned int)info.addr);
-		goto err_driver;
-	}
+	t.tx_buf = &msg[0];
+	t.len = len;
 
-	return 0;
+	spi_message_add_tail(&t, &m);
+	spi_sync(spi, &m);
 
-err_driver:
-	i2c_del_driver(&wm8750_i2c_driver);
-	return -ENODEV;
+	return len;
 }
-#endif
 
-#if defined(CONFIG_SPI_MASTER)
 static int __devinit wm8750_spi_probe(struct spi_device *spi)
 {
-	struct snd_soc_device *socdev = wm8750_socdev;
-	struct snd_soc_codec *codec = socdev->card->codec;
-	int ret;
+	struct snd_soc_codec *codec;
+	struct wm8750_priv *wm8750;
+
+	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
+	if (wm8750 == NULL)
+		return -ENOMEM;
 
+	codec = &wm8750->codec;
+	codec->hw_write = (hw_write_t)wm8750_spi_write;
 	codec->control_data = spi;
+	codec->dev = &spi->dev;
 
-	ret = wm8750_init(socdev, SND_SOC_SPI);
-	if (ret < 0)
-		dev_err(&spi->dev, "failed to initialise WM8750\n");
+	dev_set_drvdata(&spi->dev, wm8750);
 
-	return ret;
+	return wm8750_register(wm8750);
 }
 
 static int __devexit wm8750_spi_remove(struct spi_device *spi)
 {
+	struct wm8750_priv *wm8750 = dev_get_drvdata(&spi->dev);
+	wm8750_unregister(wm8750);
 	return 0;
 }
 
@@ -904,115 +1041,31 @@ static struct spi_driver wm8750_spi_driver = {
 };
 #endif
 
-static int wm8750_probe(struct platform_device *pdev)
+static int __init wm8750_modinit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct wm8750_setup_data *setup = socdev->codec_data;
-	struct snd_soc_codec *codec;
-	struct wm8750_priv *wm8750;
 	int ret;
-
-	pr_info("WM8750 Audio Codec %s", WM8750_VERSION);
-	codec = kzalloc(sizeof(struct snd_soc_codec), GFP_KERNEL);
-	if (codec == NULL)
-		return -ENOMEM;
-
-	wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL);
-	if (wm8750 == NULL) {
-		kfree(codec);
-		return -ENOMEM;
-	}
-
-	codec->private_data = wm8750;
-	socdev->card->codec = codec;
-	mutex_init(&codec->mutex);
-	INIT_LIST_HEAD(&codec->dapm_widgets);
-	INIT_LIST_HEAD(&codec->dapm_paths);
-	wm8750_socdev = socdev;
-	INIT_DELAYED_WORK(&codec->delayed_work, wm8750_work);
-
-	ret = -ENODEV;
-
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	if (setup->i2c_address) {
-		ret = wm8750_add_i2c_device(pdev, setup);
-	}
+	ret = i2c_add_driver(&wm8750_i2c_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 I2C driver: %d\n", ret);
 #endif
 #if defined(CONFIG_SPI_MASTER)
-	if (setup->spi) {
-		ret = spi_register_driver(&wm8750_spi_driver);
-		if (ret != 0)
-			printk(KERN_ERR "can't add spi driver");
-	}
+	ret = spi_register_driver(&wm8750_spi_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM8750 SPI driver: %d\n", ret);
 #endif
-
-	if (ret != 0) {
-		kfree(codec->private_data);
-		kfree(codec);
-	}
-	return ret;
-}
-
-/*
- * This function forces any delayed work to be queued and run.
- */
-static int run_delayed_work(struct delayed_work *dwork)
-{
-	int ret;
-
-	/* cancel any work waiting to be queued. */
-	ret = cancel_delayed_work(dwork);
-
-	/* if there was any work waiting then we run it now and
-	 * wait for it's completion */
-	if (ret) {
-		schedule_delayed_work(dwork, 0);
-		flush_scheduled_work();
-	}
-	return ret;
+	return 0;
 }
+module_init(wm8750_modinit);
 
-/* power down chip */
-static int wm8750_remove(struct platform_device *pdev)
+static void __exit wm8750_exit(void)
 {
-	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
-	struct snd_soc_codec *codec = socdev->card->codec;
-
-	if (codec->control_data)
-		wm8750_set_bias_level(codec, SND_SOC_BIAS_OFF);
-	run_delayed_work(&codec->delayed_work);
-	snd_soc_free_pcms(socdev);
-	snd_soc_dapm_free(socdev);
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	i2c_unregister_device(codec->control_data);
 	i2c_del_driver(&wm8750_i2c_driver);
 #endif
 #if defined(CONFIG_SPI_MASTER)
 	spi_unregister_driver(&wm8750_spi_driver);
 #endif
-	kfree(codec->private_data);
-	kfree(codec);
-
-	return 0;
-}
-
-struct snd_soc_codec_device soc_codec_dev_wm8750 = {
-	.probe = 	wm8750_probe,
-	.remove = 	wm8750_remove,
-	.suspend = 	wm8750_suspend,
-	.resume =	wm8750_resume,
-};
-EXPORT_SYMBOL_GPL(soc_codec_dev_wm8750);
-
-static int __init wm8750_modinit(void)
-{
-	return snd_soc_register_dai(&wm8750_dai);
-}
-module_init(wm8750_modinit);
-
-static void __exit wm8750_exit(void)
-{
-	snd_soc_unregister_dai(&wm8750_dai);
 }
 module_exit(wm8750_exit);
 
-- 
1.7.0

Re: [PATCH] WM8750: Convert to new API

[Liam Girdwood]

On Fri, 2010-03-26 at 03:23 +0100, Marek Vasut wrote:
> Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
> around. Hugely inspired by WM8753 which was already converted.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  sound/soc/codecs/wm8750.c |  411 +++++++++++++++++++++++++--------------------
>  1 files changed, 232 insertions(+), 179 deletions(-)
> 

This looks mostly fine. Just a comment on the codec IO.

> diff --git a/sound/soc/codecs/wm8750.c b/sound/soc/codecs/wm8750.c
> index 475c67a..c7ee6d2 100644
> --- a/sound/soc/codecs/wm8750.c
> +++ b/sound/soc/codecs/wm8750.c
> @@ -29,13 +29,6 @@
>  
>  #include "wm8750.h"
>  
> -#define WM8750_VERSION "0.12"
> -
> -/* codec private data */
> -struct wm8750_priv {
> -	unsigned int sysclk;
> -};
> -
>  /*
>   * wm8750 register cache
>   * We can't read the WM8750 register space when we
> @@ -55,6 +48,59 @@ static const u16 wm8750_reg[] = {
>  	0x0079, 0x0079, 0x0079,          /* 40 */
>  };
>  
> +/* codec private data */
> +struct wm8750_priv {
> +	unsigned int sysclk;
> +	struct snd_soc_codec codec;
> +	u16 reg_cache[ARRAY_SIZE(wm8750_reg)];
> +};
> +
> +/*
> + * read wm8750 register cache
> + */
> +static inline unsigned int wm8750_read_reg_cache(struct snd_soc_codec *codec,
> +	unsigned int reg)
> +{
> +	u16 *cache = codec->reg_cache;
> +	if (reg < 1 || reg >= (ARRAY_SIZE(wm8750_reg) + 1))
> +		return -1;
> +	return cache[reg - 1];
> +}
> +
> +/*
> + * write wm8750 register cache
> + */
> +static inline void wm8750_write_reg_cache(struct snd_soc_codec *codec,
> +	unsigned int reg, unsigned int value)
> +{
> +	u16 *cache = codec->reg_cache;
> +	if (reg < 1 || reg >= (ARRAY_SIZE(wm8750_reg) + 1))
> +		return;
> +	cache[reg - 1] = value;
> +}
> +
> +/*
> + * write to the WM8750 register space
> + */
> +static int wm8750_write(struct snd_soc_codec *codec, unsigned int reg,
> +	unsigned int value)
> +{
> +	u8 data[2];
> +
> +	/* data is
> +	 *   D15..D9 WM8750 register offset
> +	 *   D8...D0 register data
> +	 */
> +	data[0] = (reg << 1) | ((value >> 8) & 0x0001);
> +	data[1] = value & 0x00ff;
> +
> +	wm8750_write_reg_cache(codec, reg, value);
> +	if (codec->hw_write(codec->control_data, data, 2) == 2)
> +		return 0;
> +	else
> +		return -EIO;
> +}
> +

We should probably use the codec IO helpers in soc-cache.c for codec
read/write() e.g.

snd_soc_codec_set_cache_io(codec, 7, 9, wm8750->control_type);

This can set up the WM8750 for 7 address bits and 9 data bits and mean
less code in the driver.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH] WM8750: Convert to new API

[Mark Brown]

On Fri, Mar 26, 2010 at 03:23:13AM +0100, Marek Vasut wrote:

> Register the WM8750 as a SPI or I2C device. This patch mostly shuffles code
> around. Hugely inspired by WM8753 which was already converted.

Looks good - a few smallish points below, but basically it's OK.  Thanks
for sorting out git send-email, it really is much easier to work with.

It'd be best to also do the register cache thing like Liam said.  Some
other stuff...

> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  sound/soc/codecs/wm8750.c |  411 +++++++++++++++++++++++++--------------------

You also need to push out registration of the WM8750 out into the
machine drivers using it (spitz and jive).  Look at how the transition
was done for wm8731 for an example.

> -#define WM8750_VERSION "0.12"

This is already done in for-2.6.35 - in general you should always try to
make sure your patches apply against -next (which for ASoC has the
current for-2.6.xx branch in it).

> @@ -688,6 +734,11 @@ static int wm8750_resume(struct platform_device *pdev)
>  	for (i = 0; i < ARRAY_SIZE(wm8750_reg); i++) {
>  		if (i == WM8750_RESET)
>  			continue;
> +
> +		/* No point in writing hardware default values back */
> +		if (cache[i] == wm8750_reg[i])
> +			continue;
> +
>  		data[0] = (i << 1) | ((cache[i] >> 8) & 0x0001);
>  		data[1] = cache[i] & 0x00ff;
>  		codec->hw_write(codec->control_data, data, 2);

This should be done separately since while it's useful it's not really
related to the rest of your change.  This is something that frequently
comes up with your patches.  The general idea here is that each patch
should do exactly one thing which is fully described in the changelog.
This makes each individual patch simpler, making review easier (since
it's much clearer what each individual line in the diff is supposed to
do) and makes trawling the source with things like git bisect or git
annotate more useful for similar reasons.

> +/*
> + * This function forces any delayed work to be queued and run.
> + */
> +static int run_delayed_work(struct delayed_work *dwork)
> +{
> +	int ret;

This is also gone in current for-2.6.35, it shouldn't be so important
any more with current ASoC.