Skip to contentSkip to conversation

ASoC: Convert tlv320aic3x to a new-style i2c driver

PATCH In review Jean Delvare 3 participants 5 messages Sep 1, 2008

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ASoC: Convert tlv320aic3x to a new-style i2c driver

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

Cc Vladimir Barinov <[email protected]>
Cc Jarkko Nikula <[email protected]>
Cc Mark Brown <[email protected]>

---
Warning: this assumes that the davinci-evm and n810 have their codec chips on I2C bus 0, which may or may not be the case. Someone with these machines please test and report.

 sound/soc/codecs/tlv320aic3x.c  |  106 +++++++++++++++++++--------------------
 sound/soc/codecs/tlv320aic3x.h  |    1 
 sound/soc/davinci/davinci-evm.c |    1 
 sound/soc/omap/n810.c           |    1 
 4 files changed, 57 insertions(+), 52 deletions(-)
sound/soc/codecs/tlv320aic3x.c +54 -52
@@ -1172,71 +1172,39 @@ static struct snd_soc_device *aic3x_socd
  * AIC3X 2 wire address can be up to 4 devices with device addresses
  * 0x18, 0x19, 0x1A, 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 aic3x_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 aic3x_codec_probe(struct i2c_adapter *adap, int addr, int kind)
+static int aic3x_i2c_probe(struct i2c_client *i2c,
+			   const struct i2c_device_id *id)
 {
 	struct snd_soc_device *socdev = aic3x_socdev;
-	struct aic3x_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) {
-		printk(KERN_ERR "aic3x: failed to attach codec at addr %x\n",
-		       addr);
-		goto err;
-	}
-
 	ret = aic3x_init(socdev);
-	if (ret < 0) {
+	if (ret < 0)
 		printk(KERN_ERR "aic3x: failed to initialise AIC3X\n");
-		goto err;
-	}
-	return ret;
-
-err:
-	kfree(i2c);
 	return ret;
 }
 
-static int aic3x_i2c_detach(struct i2c_client *client)
+static int aic3x_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 aic3x_i2c_attach(struct i2c_adapter *adap)
-{
-	return i2c_probe(adap, &addr_data, aic3x_codec_probe);
-}
+static const struct i2c_device_id aic3x_i2c_id[] = {
+	{ "tlv320aic3x", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, aic3x_i2c_id);
 
 /* machine i2c codec control layer */
 static struct i2c_driver aic3x_i2c_driver = {
@@ -1244,13 +1212,9 @@ static struct i2c_driver aic3x_i2c_drive
 		.name = "aic3x I2C Codec",
 		.owner = THIS_MODULE,
 	},
-	.attach_adapter = aic3x_i2c_attach,
-	.detach_client = aic3x_i2c_detach,
-};
-
-static struct i2c_client client_template = {
-	.name = "AIC3X",
-	.driver = &aic3x_i2c_driver,
+	.probe = aic3x_i2c_probe,
+	.remove = aic3x_i2c_remove,
+	.id_table = aic3x_i2c_id,
 };
 
 static int aic3x_i2c_read(struct i2c_client *client, u8 *value, int len)
@@ -1258,6 +1222,46 @@ static int aic3x_i2c_read(struct i2c_cli
 	value[0] = i2c_smbus_read_byte_data(client, value[0]);
 	return (len == 1);
 }
+
+static int aic3x_add_i2c_device(struct platform_device *pdev,
+				 const struct aic3x_setup_data *setup)
+{
+	struct i2c_board_info info;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client;
+	int ret;
+
+	ret = i2c_add_driver(&aic3x_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, "tlv320aic3x", 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(&aic3x_i2c_driver);
+	return -ENODEV;
+}
 #endif
 
 static int aic3x_probe(struct platform_device *pdev)
@@ -1290,12 +1294,9 @@ static int aic3x_probe(struct platform_d
 	aic3x_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;
 		codec->hw_read = (hw_read_t) aic3x_i2c_read;
-		ret = i2c_add_driver(&aic3x_i2c_driver);
-		if (ret != 0)
-			printk(KERN_ERR "can't add i2c driver");
+		ret = aic3x_add_i2c_device(pdev, setup);
 	}
 #else
 	/* Add other interfaces here */
@@ -1320,6 +1321,7 @@ static int aic3x_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(&aic3x_i2c_driver);
 #endif
 	kfree(codec->private_data);
sound/soc/codecs/tlv320aic3x.h +1 -0
@@ -224,6 +224,7 @@ int aic3x_get_gpio(struct snd_soc_codec 
 int aic3x_headset_detected(struct snd_soc_codec *codec);
 
 struct aic3x_setup_data {
+	int i2c_bus;
 	unsigned short i2c_address;
 	unsigned int gpio_func[2];
 };
sound/soc/davinci/davinci-evm.c +1 -0
@@ -136,6 +136,7 @@ static struct snd_soc_machine snd_soc_ma
 
 /* evm audio private data */
 static struct aic3x_setup_data evm_aic3x_setup = {
+	.i2c_bus = 0,
 	.i2c_address = 0x1b,
 };
 
sound/soc/omap/n810.c +1 -0
@@ -290,6 +290,7 @@ static struct snd_soc_machine snd_soc_ma
 
 /* Audio private data */
 static struct aic3x_setup_data n810_aic33_setup = {
+	.i2c_bus = 0,
 	.i2c_address = 0x18,
 	.gpio_func[0] = AIC3X_GPIO1_FUNC_DISABLED,
 	.gpio_func[1] = AIC3X_GPIO2_FUNC_DIGITAL_MIC_INPUT,
Signature · 1 line
Jean Delvare
#1 lore raw
Quoted 12 lines · On Mon, 1 Sep 2008 11:21:31 +0200 "ext Jean Delvare" <[email protected]> wrote:

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

Cc Vladimir Barinov <[email protected]>
Cc Jarkko Nikula <[email protected]>
Cc Mark Brown <[email protected]>

---
Warning: this assumes that the davinci-evm and n810 have their codec chips on I2C bus 0, which may or may not be the case. Someone with these machines please test and report.

...

Review

Quoted 5 lines

linux-2.6.27-rc5/sound/soc/omap/n810.c 2008-09-01
08:30:10.000000000 +0200 @@ -290,6 +290,7 @@ static struct snd_soc_machine snd_soc_ma /* Audio private data */
static struct aic3x_setup_data n810_aic33_setup = {

(quoted diff) +1 -0
+	.i2c_bus = 0,

Thanks. In N810 it's connected into second i2c bus so this makes it working

+ .i2c_bus = 2,

Jarkko

#2 lore raw
Quoted 6 lines · Jarkko Nikula

"ext Jean Delvare" <[email protected]> wrote:

Thanks. In N810 it's connected into second i2c bus so this makes it working

+ .i2c_bus = 2,

Should that be 1? Normally the buses are numbered from zero...

#3 lore raw
Quoted 10 lines · On Mon, 1 Sep 2008 12:35:09 +0100 "ext Mark Brown" <[email protected]>…
Quoted 6 lines · Jarkko Nikula

"ext Jean Delvare" <[email protected]> wrote:

Thanks. In N810 it's connected into second i2c bus so this makes it working

+ .i2c_bus = 2,

Should that be 1? Normally the buses are numbered from zero...

Good question. By some historical reason we are registering busses starting from number one in OMAP and short hack below (in linux-omap tree) trying to register them starting from zero didn't work out.

But yeah, that's another issue in linux-omap and Jean's patch is working on N810 if changing this i2c_bus to 2.

Jarkko

arch/arm/mach-omap2/board-n800.c +4 -4
@@ -691,15 +691,15 @@ void __init nokia_n800_common_init(void)
 				ARRAY_SIZE(n810_spi_board_info));
 	}
 	omap_serial_init();
-	omap_register_i2c_bus(1, 400, n800_i2c_board_info_1,
+	omap_register_i2c_bus(0, 400, n800_i2c_board_info_1,
 			      ARRAY_SIZE(n800_i2c_board_info_1));
-	omap_register_i2c_bus(2, 400, n8x0_i2c_board_info_2,
+	omap_register_i2c_bus(1, 400, n8x0_i2c_board_info_2,
 			      ARRAY_SIZE(n800_i2c_board_info_2));
 	if (machine_is_nokia_n800())
-		i2c_register_board_info(2, n800_i2c_board_info_2,
+		i2c_register_board_info(1, n800_i2c_board_info_2,
 			ARRAY_SIZE(n800_i2c_board_info_2));
 	if (machine_is_nokia_n810())
-		i2c_register_board_info(2, n810_i2c_board_info_2,
+		i2c_register_board_info(1, n810_i2c_board_info_2,
 			ARRAY_SIZE(n810_i2c_board_info_2));
 		
 	mipid_dev_init();
arch/arm/plat-omap/i2c.c +0 -0
index 89a6ab0..83ef7cf 100644
@@ -134,7 +134,7 @@ int __init omap_register_i2c_bus(int bus_id, u32

clkrate, else if (cpu_is_omap34xx())
ports = 3;

- BUG_ON(bus_id < 1 || bus_id > ports);
+// BUG_ON(bus_id < 1 || bus_id > ports);

if (info) {
err = i2c_register_board_info(bus_id, info, len);
@@ -142,10 +142,10 @@ int __init omap_register_i2c_bus(int bus_id, u32 clkrate, return err;
}

- pdev = &omap_i2c_devices[bus_id - 1];
+ pdev = &omap_i2c_devices[bus_id /*- 1*/];
*(u32 *)pdev->dev.platform_data = clkrate;

- if (bus_id == 1) {
+ if (bus_id == /*1*/0) {
res = pdev->resource;
if (cpu_class_is_omap1()) {
base = OMAP1_I2C_BASE;
@@ -159,6 +159,6 @@ int __init omap_register_i2c_bus(int bus_id, u32 clkrate, res[1].start = irq;
}

- omap_i2c_mux_pins(bus_id - 1);
+ omap_i2c_mux_pins(bus_id /*- 1*/);
return platform_device_register(pdev);
}

#4 lore raw
Quoted 11 lines · Jarkko Nikula
Quoted 8 lines · On Mon, 1 Sep 2008 12:35:09 +0100 "ext Mark Brown" <[email protected]>…
Quoted 6 lines · Jarkko Nikula

"ext Jean Delvare" <[email protected]> wrote:

Thanks. In N810 it's connected into second i2c bus so this makes it working

+ .i2c_bus = 2,

Quoted 8 lines
Quoted 2 lines

Should that be 1? Normally the buses are numbered from zero...

Good question. By some historical reason we are registering busses starting from number one in OMAP and short hack below (in linux-omap tree) trying to register them starting from zero didn't work out.

But yeah, that's another issue in linux-omap and Jean's patch is working on N810 if changing this i2c_bus to 2.

OK, thanks Jarkko for testing and reporting. I'll make it 2 for the time being so that things keep working, and it can be changed later if needed.

Signature · 1 line
Jean Delvare