[PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Jean Delvare]

Convert the wm8731 codec driver to the new (standard) device driver
binding model.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
Warning: this assumes that the corgi, poodle and eti_b1 have their
codec chips on I2C bus 0, which may or may not be the case. Someone
with these machines please test and report.

Note that you can safely ignore patch failures in i2c-id.h.

 include/linux/i2c-id.h         |    1 
 sound/soc/at91/eti_b1_wm8731.c |    1 
 sound/soc/codecs/wm8731.c      |  111 +++++++++++++++++++---------------------
 sound/soc/codecs/wm8731.h      |    1 
 sound/soc/pxa/corgi.c          |    1 
 sound/soc/pxa/poodle.c         |    1 
 6 files changed, 59 insertions(+), 57 deletions(-)

--- linux-2.6.27-rc4.orig/include/linux/i2c-id.h	2008-08-26 16:41:12.000000000 +0200
+++ linux-2.6.27-rc4/include/linux/i2c-id.h	2008-08-26 17:02:29.000000000 +0200
@@ -64,7 +64,6 @@
 #define I2C_DRIVERID_SAA717X	80	/* saa717x video encoder	*/
 #define I2C_DRIVERID_DS1672	81	/* Dallas/Maxim DS1672 RTC	*/
 #define I2C_DRIVERID_TLV320AIC23B 87	/* TI TLV320AIC23B audio codec  */
-#define I2C_DRIVERID_WM8731	89	/* Wolfson WM8731 audio codec */
 #define I2C_DRIVERID_WM8753	91	/* Wolfson WM8753 audio codec */
 #define I2C_DRIVERID_LM4857 	92 	/* LM4857 Audio Amplifier */
 #define I2C_DRIVERID_VP27SMPX	93	/* Panasonic VP27s tuner internal MPX */
--- linux-2.6.27-rc4.orig/sound/soc/at91/eti_b1_wm8731.c	2008-08-26 16:40:44.000000000 +0200
+++ linux-2.6.27-rc4/sound/soc/at91/eti_b1_wm8731.c	2008-08-26 17:02:20.000000000 +0200
@@ -243,6 +243,7 @@ static struct snd_soc_machine snd_soc_ma
 };
 
 static struct wm8731_setup_data eti_b1_wm8731_setup = {
+	.i2c_bus = 0,
 	.i2c_address = 0x1a,
 };
 
--- linux-2.6.27-rc4.orig/sound/soc/codecs/wm8731.c	2008-08-26 16:40:44.000000000 +0200
+++ linux-2.6.27-rc4/sound/soc/codecs/wm8731.c	2008-08-26 17:11:50.000000000 +0200
@@ -570,86 +570,87 @@ static struct snd_soc_device *wm8731_soc
  *    low  = 0x1a
  *    high = 0x1b
  */
-static unsigned short normal_i2c[] = { 0, I2C_CLIENT_END };
 
-/* Magic definition of all other variables and things */
-I2C_CLIENT_INSMOD;
-
-static struct i2c_driver wm8731_i2c_driver;
-static struct i2c_client client_template;
-
-/* If the i2c layer weren't so broken, we could pass this kind of data
-   around */
-
-static int wm8731_codec_probe(struct i2c_adapter *adap, int addr, int kind)
+static int wm8731_i2c_probe(struct i2c_client *i2c,
+			    const struct i2c_device_id *id)
 {
 	struct snd_soc_device *socdev = wm8731_socdev;
-	struct wm8731_setup_data *setup = socdev->codec_data;
 	struct snd_soc_codec *codec = socdev->codec;
-	struct i2c_client *i2c;
 	int ret;
 
-	if (addr != setup->i2c_address)
-		return -ENODEV;
-
-	client_template.adapter = adap;
-	client_template.addr = addr;
-
-	i2c = kmemdup(&client_template, sizeof(client_template), GFP_KERNEL);
-	if (i2c == NULL)
-		return -ENOMEM;
-
 	i2c_set_clientdata(i2c, codec);
-	codec->control_data = i2c;
-
-	ret = i2c_attach_client(i2c);
-	if (ret < 0) {
-		pr_err("failed to attach codec at addr %x\n", addr);
-		goto err;
-	}
 
 	ret = wm8731_init(socdev);
-	if (ret < 0) {
+	if (ret < 0)
 		pr_err("failed to initialise WM8731\n");
-		goto err;
-	}
-	return ret;
 
-err:
-	kfree(i2c);
 	return ret;
 }
 
-static int wm8731_i2c_detach(struct i2c_client *client)
+static int wm8731_i2c_remove(struct i2c_client *client)
 {
 	struct snd_soc_codec *codec = i2c_get_clientdata(client);
-	i2c_detach_client(client);
 	kfree(codec->reg_cache);
-	kfree(client);
 	return 0;
 }
 
-static int wm8731_i2c_attach(struct i2c_adapter *adap)
-{
-	return i2c_probe(adap, &addr_data, wm8731_codec_probe);
-}
+static const struct i2c_device_id wm8731_i2c_id[] = {
+	{ "wm8731", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, wm8731_i2c_id);
 
-/* corgi i2c codec control layer */
 static struct i2c_driver wm8731_i2c_driver = {
 	.driver = {
 		.name = "WM8731 I2C Codec",
 		.owner = THIS_MODULE,
 	},
-	.id =             I2C_DRIVERID_WM8731,
-	.attach_adapter = wm8731_i2c_attach,
-	.detach_client =  wm8731_i2c_detach,
-	.command =        NULL,
+	.probe =    wm8731_i2c_probe,
+	.remove =   wm8731_i2c_remove,
+	.id_table = wm8731_i2c_id,
 };
 
-static struct i2c_client client_template = {
-	.name =   "WM8731",
-	.driver = &wm8731_i2c_driver,
-};
+static int wm8731_add_i2c_device(struct platform_device *pdev,
+				 const struct wm8731_setup_data *setup)
+{
+	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
+	struct i2c_board_info info;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+	int ret;
+
+	ret = i2c_add_driver(&wm8731_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, "wm8731", 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;
+	}
+	socdev->codec->control_data = client;
+
+	return 0;
+
+err_driver:
+	i2c_del_driver(&wm8731_i2c_driver);
+	return -ENODEV;
+}
 #endif
 
 static int wm8731_probe(struct platform_device *pdev)
@@ -682,11 +683,8 @@ static int wm8731_probe(struct platform_
 	wm8731_socdev = socdev;
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
 	if (setup->i2c_address) {
-		normal_i2c[0] = setup->i2c_address;
 		codec->hw_write = (hw_write_t)i2c_master_send;
-		ret = i2c_add_driver(&wm8731_i2c_driver);
-		if (ret != 0)
-			printk(KERN_ERR "can't add i2c driver");
+		ret = wm8731_add_i2c_device(pdev, setup);
 	}
 #else
 	/* Add other interfaces here */
@@ -711,6 +709,7 @@ static int wm8731_remove(struct platform
 	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(&wm8731_i2c_driver);
 #endif
 	kfree(codec->private_data);
--- linux-2.6.27-rc4.orig/sound/soc/codecs/wm8731.h	2008-08-26 16:40:44.000000000 +0200
+++ linux-2.6.27-rc4/sound/soc/codecs/wm8731.h	2008-08-26 17:02:20.000000000 +0200
@@ -35,6 +35,7 @@
 #define WM8731_DAI		0
 
 struct wm8731_setup_data {
+	int            i2c_bus;
 	unsigned short i2c_address;
 };
 
--- linux-2.6.27-rc4.orig/sound/soc/pxa/corgi.c	2008-08-26 16:40:44.000000000 +0200
+++ linux-2.6.27-rc4/sound/soc/pxa/corgi.c	2008-08-26 17:02:20.000000000 +0200
@@ -330,6 +330,7 @@ static struct snd_soc_machine snd_soc_ma
 
 /* corgi audio private data */
 static struct wm8731_setup_data corgi_wm8731_setup = {
+	.i2c_bus = 0,
 	.i2c_address = 0x1b,
 };
 
--- linux-2.6.27-rc4.orig/sound/soc/pxa/poodle.c	2008-08-26 16:40:44.000000000 +0200
+++ linux-2.6.27-rc4/sound/soc/pxa/poodle.c	2008-08-26 17:02:20.000000000 +0200
@@ -284,6 +284,7 @@ static struct snd_soc_machine snd_soc_ma
 
 /* poodle audio private data */
 static struct wm8731_setup_data poodle_wm8731_setup = {
+	.i2c_bus = 0,
 	.i2c_address = 0x1b,
 };
 


-- 
Jean Delvare

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Mark Brown]

On Tue, Aug 26, 2008 at 06:24:37PM +0200, Jean Delvare wrote:
> Convert the wm8731 codec driver to the new (standard) device driver
> binding model.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

for the wm8731 driver - again, I can't test the machine drivers.

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Manuel Lauss]

Hi Jean,

On Tue, Aug 26, 2008 at 06:24:37PM +0200, Jean Delvare wrote:
> Convert the wm8731 codec driver to the new (standard) device driver
> binding model.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> Warning: this assumes that the corgi, poodle and eti_b1 have their
> codec chips on I2C bus 0, which may or may not be the case. Someone
> with these machines please test and report.
> 
> Note that you can safely ignore patch failures in i2c-id.h.
> 
>  include/linux/i2c-id.h         |    1 
>  sound/soc/at91/eti_b1_wm8731.c |    1 
>  sound/soc/codecs/wm8731.c      |  111 +++++++++++++++++++---------------------
>  sound/soc/codecs/wm8731.h      |    1 
>  sound/soc/pxa/corgi.c          |    1 
>  sound/soc/pxa/poodle.c         |    1 
>  6 files changed, 59 insertions(+), 57 deletions(-)

>  static struct wm8731_setup_data eti_b1_wm8731_setup = {
> +	.i2c_bus = 0,
>  	.i2c_address = 0x1a,
>  };
[...]  
> --- linux-2.6.27-rc4.orig/sound/soc/codecs/wm8731.c	2008-08-26 16:40:44.000000000 +0200
> +++ linux-2.6.27-rc4/sound/soc/codecs/wm8731.c	2008-08-26 17:11:50.000000000 +0200
[...]
> +static int wm8731_add_i2c_device(struct platform_device *pdev,
> +				 const struct wm8731_setup_data *setup)
> +{
> +	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
> +	struct i2c_board_info info;
> +	struct i2c_adapter *adapter;
> +	struct i2c_client *client;
> +	int ret;
> +
> +	ret = i2c_add_driver(&wm8731_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, "wm8731", I2C_NAME_SIZE);

I don't know about other wm8731 users, but I'd prefer to let the board
code register the codec i2c device.  Then you could also get rid
of "struct wm8731_setup_data" altogether.

The patch works on a MIPS testboard.

Thanks!
	Manuel Lauss

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Jean Delvare]

On Tue, 26 Aug 2008 21:20:43 +0200, Manuel Lauss wrote:
> Hi Jean,
> 
> On Tue, Aug 26, 2008 at 06:24:37PM +0200, Jean Delvare wrote:
> > Convert the wm8731 codec driver to the new (standard) device driver
> > binding model.
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > ---
> > Warning: this assumes that the corgi, poodle and eti_b1 have their
> > codec chips on I2C bus 0, which may or may not be the case. Someone
> > with these machines please test and report.
> > 
> > Note that you can safely ignore patch failures in i2c-id.h.
> > 
> >  include/linux/i2c-id.h         |    1 
> >  sound/soc/at91/eti_b1_wm8731.c |    1 
> >  sound/soc/codecs/wm8731.c      |  111 +++++++++++++++++++---------------------
> >  sound/soc/codecs/wm8731.h      |    1 
> >  sound/soc/pxa/corgi.c          |    1 
> >  sound/soc/pxa/poodle.c         |    1 
> >  6 files changed, 59 insertions(+), 57 deletions(-)
> 
> >  static struct wm8731_setup_data eti_b1_wm8731_setup = {
> > +	.i2c_bus = 0,
> >  	.i2c_address = 0x1a,
> >  };
> [...]  
> > --- linux-2.6.27-rc4.orig/sound/soc/codecs/wm8731.c	2008-08-26 16:40:44.000000000 +0200
> > +++ linux-2.6.27-rc4/sound/soc/codecs/wm8731.c	2008-08-26 17:11:50.000000000 +0200
> [...]
> > +static int wm8731_add_i2c_device(struct platform_device *pdev,
> > +				 const struct wm8731_setup_data *setup)
> > +{
> > +	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
> > +	struct i2c_board_info info;
> > +	struct i2c_adapter *adapter;
> > +	struct i2c_client *client;
> > +	int ret;
> > +
> > +	ret = i2c_add_driver(&wm8731_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, "wm8731", I2C_NAME_SIZE);
> 
> I don't know about other wm8731 users, but I'd prefer to let the board
> code register the codec i2c device.  Then you could also get rid
> of "struct wm8731_setup_data" altogether.

Yes, I agree, and I think that Mark has plans for changes in this
direction. But for now my personal goal was to move the drivers away
from i2c_attach_client and friends as this old API is going away soon.
Moving the place where the i2c device is created can happen later as
platforms maintainers and/or ASoC maintainers see fit.

> The patch works on a MIPS testboard.

Great, thanks for testing and reporting!

-- 
Jean Delvare

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Mark Brown]

On Tue, Aug 26, 2008 at 09:20:43PM +0200, Manuel Lauss wrote:

> I don't know about other wm8731 users, but I'd prefer to let the board
> code register the codec i2c device.  Then you could also get rid
> of "struct wm8731_setup_data" altogether.

Yes, that would have been the ideal thing with the old I2C API too IIRC.
Unfortunately ASoC v1 requires that all the devices comprising the ASoC
system be registered in one fell swoop.  Once enough of v2 has been
merged to allow the codec drivers to register their DAIs independantly
of everything else the codecs can be instantiated from wherever the
platform feels like doing it.

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Manuel Lauss]

Hi Mark,

On Tue, Aug 26, 2008 at 08:37:28PM +0100, Mark Brown wrote:
> On Tue, Aug 26, 2008 at 09:20:43PM +0200, Manuel Lauss wrote:
> 
> > I don't know about other wm8731 users, but I'd prefer to let the board
> > code register the codec i2c device.  Then you could also get rid
> > of "struct wm8731_setup_data" altogether.
> 
> Yes, that would have been the ideal thing with the old I2C API too IIRC.
> Unfortunately ASoC v1 requires that all the devices comprising the ASoC
> system be registered in one fell swoop.  Once enough of v2 has been
> merged to allow the codec drivers to register their DAIs independantly
> of everything else the codecs can be instantiated from wherever the
> platform feels like doing it.

I modified Jean's newest patch a bit to just register the i2c driver and
leave the actual device registration to the board code.  It works as
intended!  The only difference is that udev loads the wm8731 module at boot
but that is expected due to the set owner field in struct i2c_driver;
after loading the platform sound module all is working well.
(tried with asoc all-modular and all-builtin).

Best regards,
	Manuel Lauss

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Jean Delvare]

Hi Manuel,

On Thu, 28 Aug 2008 19:25:40 +0200, Manuel Lauss wrote:
> Hi Mark,
> 
> On Tue, Aug 26, 2008 at 08:37:28PM +0100, Mark Brown wrote:
> > On Tue, Aug 26, 2008 at 09:20:43PM +0200, Manuel Lauss wrote:
> > 
> > > I don't know about other wm8731 users, but I'd prefer to let the board
> > > code register the codec i2c device.  Then you could also get rid
> > > of "struct wm8731_setup_data" altogether.
> > 
> > Yes, that would have been the ideal thing with the old I2C API too IIRC.
> > Unfortunately ASoC v1 requires that all the devices comprising the ASoC
> > system be registered in one fell swoop.  Once enough of v2 has been
> > merged to allow the codec drivers to register their DAIs independantly
> > of everything else the codecs can be instantiated from wherever the
> > platform feels like doing it.
> 
> I modified Jean's newest patch a bit to just register the i2c driver and
> leave the actual device registration to the board code.  It works as
> intended!

Great, thanks for testing this. Could you please post an incremental
patch going on top of mine? So that I see what it looks like and also
so that both patches can be pushed upstream.

> The only difference is that udev loads the wm8731 module at boot
> but that is expected due to the set owner field in struct i2c_driver;

What makes the wm8731 driver load at boot is the module alias that is
now exported by the wm8731 module ("i2c:wm8731"). When you create the
wm8731 I2C device from your platform code, udev and the surrounding
scripts notice it and basically call "modprobe i2c:wm8731".

This is one of the various benefits of the new i2c device driver
binding model.

> after loading the platform sound module all is working well.
> (tried with asoc all-modular and all-builtin).

-- 
Jean Delvare

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Manuel Lauss]

On Thu, Aug 28, 2008 at 07:37:54PM +0200, Jean Delvare wrote:
> Hi Manuel,
> 
> On Thu, 28 Aug 2008 19:25:40 +0200, Manuel Lauss wrote:
> > Hi Mark,
> > 
> > On Tue, Aug 26, 2008 at 08:37:28PM +0100, Mark Brown wrote:
> > > On Tue, Aug 26, 2008 at 09:20:43PM +0200, Manuel Lauss wrote:
> > > 
> > > > I don't know about other wm8731 users, but I'd prefer to let the board
> > > > code register the codec i2c device.  Then you could also get rid
> > > > of "struct wm8731_setup_data" altogether.
> > > 
> > > Yes, that would have been the ideal thing with the old I2C API too IIRC.
> > > Unfortunately ASoC v1 requires that all the devices comprising the ASoC
> > > system be registered in one fell swoop.  Once enough of v2 has been
> > > merged to allow the codec drivers to register their DAIs independantly
> > > of everything else the codecs can be instantiated from wherever the
> > > platform feels like doing it.
> > 
> > I modified Jean's newest patch a bit to just register the i2c driver and
> > leave the actual device registration to the board code.  It works as
> > intended!
> 
> Great, thanks for testing this. Could you please post an incremental
> patch going on top of mine? So that I see what it looks like and also
> so that both patches can be pushed upstream.

Attached below.  Although I'm no longer convinced it's a good idea after
all; I don't see how multiple asoc devices with this codec could be
realized (which afaik is planned with asoc2).

Thanks,
	Manuel Lauss
 
------ 8< -------------  8< ------------

From: Manuel Lauss <mano@roarinelk.homelinux.net>
Subject: [PATCH] wm8731: boards register i2c codec devices now!

Don't register the i2c device with the core in the driver source,
let the board code do that.  Now we can also get rid of wm8731_setup_data.
---
 I didn't want to touch the arch/arm/mach-pxa/{corgi,poodle}.c files, but
 if a variant of this patch somehow makes into the kernel, someone also
 needs to add i2c_board_info with codec device information.

 sound/soc/codecs/wm8731.c |   49 +++-----------------------------------------
 sound/soc/codecs/wm8731.h |    5 ----
 sound/soc/pxa/corgi.c     |    7 ------
 sound/soc/pxa/poodle.c    |    7 ------
 4 files changed, 4 insertions(+), 64 deletions(-)

diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c
index 5814f9b..1ef8e6a 100644
--- a/sound/soc/codecs/wm8731.c
+++ b/sound/soc/codecs/wm8731.c
@@ -580,6 +580,7 @@ static int wm8731_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, codec);
 	codec->control_data = i2c;
+	codec->hw_write = (hw_write_t)i2c_master_send;
 
 	ret = wm8731_init(socdev);
 	if (ret < 0)
@@ -610,46 +611,6 @@ static struct i2c_driver wm8731_i2c_driver = {
 	.remove =   wm8731_i2c_remove,
 	.id_table = wm8731_i2c_id,
 };
-
-static int wm8731_add_i2c_device(struct platform_device *pdev,
-				 const struct wm8731_setup_data *setup)
-{
-	struct i2c_board_info info;
-	struct i2c_adapter *adapter;
-	struct i2c_client *client;
-	int ret;
-
-	ret = i2c_add_driver(&wm8731_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, "wm8731", 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(&wm8731_i2c_driver);
-	return -ENODEV;
-}
 #endif
 
 static int wm8731_probe(struct platform_device *pdev)
@@ -681,10 +642,9 @@ static int wm8731_probe(struct platform_device *pdev)
 
 	wm8731_socdev = socdev;
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
-	if (setup->i2c_address) {
-		codec->hw_write = (hw_write_t)i2c_master_send;
-		ret = wm8731_add_i2c_device(pdev, setup);
-	}
+	ret = i2c_add_driver(&wm8731_i2c_driver);
+	if (ret != 0)
+		dev_err(&pdev->dev, "can't add i2c driver\n");
 #else
 	/* Add other interfaces here */
 #endif
@@ -708,7 +668,6 @@ static int wm8731_remove(struct platform_device *pdev)
 	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(&wm8731_i2c_driver);
 #endif
 	kfree(codec->private_data);
diff --git a/sound/soc/codecs/wm8731.h b/sound/soc/codecs/wm8731.h
index 0f81239..cd7b806 100644
--- a/sound/soc/codecs/wm8731.h
+++ b/sound/soc/codecs/wm8731.h
@@ -34,11 +34,6 @@
 #define WM8731_SYSCLK	0
 #define WM8731_DAI		0
 
-struct wm8731_setup_data {
-	int            i2c_bus;
-	unsigned short i2c_address;
-};
-
 extern struct snd_soc_dai wm8731_dai;
 extern struct snd_soc_codec_device soc_codec_dev_wm8731;
 
diff --git a/sound/soc/pxa/corgi.c b/sound/soc/pxa/corgi.c
index 72b7a51..4837bdb 100644
--- a/sound/soc/pxa/corgi.c
+++ b/sound/soc/pxa/corgi.c
@@ -328,18 +328,11 @@ static struct snd_soc_machine snd_soc_machine_corgi = {
 	.num_links = 1,
 };
 
-/* corgi audio private data */
-static struct wm8731_setup_data corgi_wm8731_setup = {
-	.i2c_bus = 0,
-	.i2c_address = 0x1b,
-};
-
 /* corgi audio subsystem */
 static struct snd_soc_device corgi_snd_devdata = {
 	.machine = &snd_soc_machine_corgi,
 	.platform = &pxa2xx_soc_platform,
 	.codec_dev = &soc_codec_dev_wm8731,
-	.codec_data = &corgi_wm8731_setup,
 };
 
 static struct platform_device *corgi_snd_device;
diff --git a/sound/soc/pxa/poodle.c b/sound/soc/pxa/poodle.c
index f84f7d8..a6adb46 100644
--- a/sound/soc/pxa/poodle.c
+++ b/sound/soc/pxa/poodle.c
@@ -282,18 +282,11 @@ static struct snd_soc_machine snd_soc_machine_poodle = {
 	.num_links = 1,
 };
 
-/* poodle audio private data */
-static struct wm8731_setup_data poodle_wm8731_setup = {
-	.i2c_bus = 0,
-	.i2c_address = 0x1b,
-};
-
 /* poodle audio subsystem */
 static struct snd_soc_device poodle_snd_devdata = {
 	.machine = &snd_soc_machine_poodle,
 	.platform = &pxa2xx_soc_platform,
 	.codec_dev = &soc_codec_dev_wm8731,
-	.codec_data = &poodle_wm8731_setup,
 };
 
 static struct platform_device *poodle_snd_device;
-- 
1.6.0

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Mark Brown]

On Thu, Aug 28, 2008 at 08:42:52PM +0200, Manuel Lauss wrote:

> Attached below.  Although I'm no longer convinced it's a good idea after
> all; I don't see how multiple asoc devices with this codec could be
> realized (which afaik is planned with asoc2).

You can't have multiple ASoC devices.  You also can't have more than one
codec within the one ASoC device that you can instantiate at the moment.
Loading the codec driver normally can be made to work (the PowerPC guys
are doing it at the minute) but it's fragile since it doesn't play with
the assumptions the ASoC code is currently making.

I'd be more comfortable with leaving the I2C driver registration done
where it is currently until ASoC at least supports dynamic registration
of codec drivers explicitly purely on a risk avoidance basis.

Re: [PATCH] ASoC: Convert wm8731 to a new-style i2c driver (testers wanted)

[Sedji Gaouaou]

Hi Manuel,
>  
> diff --git a/sound/soc/pxa/corgi.c b/sound/soc/pxa/corgi.c
> index 72b7a51..4837bdb 100644
> --- a/sound/soc/pxa/corgi.c
> +++ b/sound/soc/pxa/corgi.c
> @@ -328,18 +328,11 @@ static struct snd_soc_machine snd_soc_machine_corgi = {
>  	.num_links = 1,
>  };
>  
> -/* corgi audio private data */
> -static struct wm8731_setup_data corgi_wm8731_setup = {
> -	.i2c_bus = 0,
> -	.i2c_address = 0x1b,
> -};
> -
>  /* corgi audio subsystem */
>  static struct snd_soc_device corgi_snd_devdata = {
>  	.machine = &snd_soc_machine_corgi,
>  	.platform = &pxa2xx_soc_platform,
>  	.codec_dev = &soc_codec_dev_wm8731,
> -	.codec_data = &corgi_wm8731_setup,
>  };
>  
>  static struct platform_device *corgi_snd_device;
> diff --git a/sound/soc/pxa/poodle.c b/sound/soc/pxa/poodle.c
> index f84f7d8..a6adb46 100644
> --- a/sound/soc/pxa/poodle.c
> +++ b/sound/soc/pxa/poodle.c
> @@ -282,18 +282,11 @@ static struct snd_soc_machine snd_soc_machine_poodle = {
>  	.num_links = 1,
>  };
>  
> -/* poodle audio private data */
> -static struct wm8731_setup_data poodle_wm8731_setup = {
> -	.i2c_bus = 0,
> -	.i2c_address = 0x1b,
> -};
> -
>  /* poodle audio subsystem */
>  static struct snd_soc_device poodle_snd_devdata = {
>  	.machine = &snd_soc_machine_poodle,
>  	.platform = &pxa2xx_soc_platform,
>  	.codec_dev = &soc_codec_dev_wm8731,
> -	.codec_data = &poodle_wm8731_setup,
>  };
>  
>  static struct platform_device *poodle_snd_device;

Shouldn't it be the same for the file sound/soc/at91/eti_b1_wm8731.c ??

Regards,
Sedji