On Thu, 2010-07-15 at 15:42 +0800, Axel Lin wrote:
I agree that it is good to release resources at the same level, where they have been acquired. But I prefer to follow the maintainer/author's coding style.
For the changes in other drivers, I'd like to see Liam and Mark's comments. Or if the driver maintainers request it, I can fix it.
Fwiw, the soon to be merged ASoC multi-component branch simplifies the codec probe() and remove() as follows (e.g. from I2C and SPI WM8750 codec) :-
static int wm8750_probe(struct snd_soc_codec *codec) { struct wm8750_priv *wm8750 = snd_soc_codec_get_drvdata(codec); int reg, ret;
codec->control_data = wm8750->control_data; ret = snd_soc_codec_set_cache_io(codec, 7, 9, wm8750->control_type); if (ret < 0) { printk(KERN_ERR "wm8750: failed to set cache I/O: %d\n", ret); return ret; }
ret = wm8750_reset(codec); if (ret < 0) { printk(KERN_ERR "wm8750: failed to reset: %d\n", ret); return ret; }
/* charge output caps */ wm8750_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
snd_soc_add_controls(codec, wm8750_snd_controls, ARRAY_SIZE(wm8750_snd_controls)); wm8750_add_widgets(codec); return ret; }
static int wm8750_remove(struct snd_soc_codec *codec) { wm8750_set_bias_level(codec, SND_SOC_BIAS_OFF); return 0; }
#if defined(CONFIG_SPI_MASTER) static int __devinit wm8750_spi_probe(struct spi_device *spi) { struct wm8750_priv *wm8750; int ret;
wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL); if (wm8750 == NULL) return -ENOMEM;
wm8750->control_data = spi; wm8750->control_type = SND_SOC_SPI; spi_set_drvdata(spi, wm8750);
ret = snd_soc_register_codec(&spi->dev, spi->chip_select, &soc_codec_dev_wm8750, &wm8750_dai, 1); if (ret < 0) kfree(wm8750); return ret; }
static int __devexit wm8750_spi_remove(struct spi_device *spi) { snd_soc_unregister_codec(&spi->dev, spi->chip_select); kfree(spi_get_drvdata(spi)); return 0; }
static struct spi_driver wm8750_spi_driver = { .driver = { .name = "wm8750 SPI Codec", .bus = &spi_bus_type, .owner = THIS_MODULE, }, .probe = wm8750_spi_probe, .remove = __devexit_p(wm8750_spi_remove), }; #endif /* CONFIG_SPI_MASTER */
#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) static __devinit int wm8750_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { struct wm8750_priv *wm8750; int ret;
wm8750 = kzalloc(sizeof(struct wm8750_priv), GFP_KERNEL); if (wm8750 == NULL) return -ENOMEM;
i2c_set_clientdata(i2c, wm8750); wm8750->control_data = i2c; wm8750->control_type = SND_SOC_I2C;
ret = snd_soc_register_codec(&i2c->dev, i2c->addr, &soc_codec_dev_wm8750, &wm8750_dai, 1); if (ret < 0) kfree(wm8750); return ret; }
static __devexit int wm8750_i2c_remove(struct i2c_client *client) { snd_soc_unregister_codec(&client->dev, client->addr); kfree(i2c_get_clientdata(client)); return 0; }
static const struct i2c_device_id wm8750_i2c_id[] = { { "wm8750", 0 }, { "wm8987", 0 }, { } }; MODULE_DEVICE_TABLE(i2c, wm8750_i2c_id);
static struct i2c_driver wm8750_i2c_driver = { .driver = { .name = "wm8750 I2C Codec", .owner = THIS_MODULE, }, .probe = wm8750_i2c_probe, .remove = __devexit_p(wm8750_i2c_remove), .id_table = wm8750_i2c_id, }; #endif
static int __init wm8750_modinit(void) { int ret = 0; #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) ret = i2c_add_driver(&wm8750_i2c_driver); if (ret != 0) { printk(KERN_ERR "Failed to register wm8750 I2C driver: %d\n", ret); } #endif #if defined(CONFIG_SPI_MASTER) ret = spi_register_driver(&wm8750_spi_driver); if (ret != 0) { printk(KERN_ERR "Failed to register wm8750 SPI driver: %d\n", ret); } #endif return ret; } module_init(wm8750_modinit);
static void __exit wm8750_exit(void) { #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) i2c_del_driver(&wm8750_i2c_driver); #endif #if defined(CONFIG_SPI_MASTER) spi_unregister_driver(&wm8750_spi_driver); #endif } module_exit(wm8750_exit);
So while this patch series is useful (from a minor bug fix pov), it will be overwritten shortly in the ASoC multi-component merge. Probably delaying the multi-component merge too (since the changes are in the same place).
Can we hold off this series for a week or two. I have one more update for multi-component and if we don't have multi-component upstreamed in that time frame we can apply this series.
Thanks
Liam