- Alsa-devel - mailman.alsa-project.org

Re: [alsa-devel] [PATCH 1/4] ASOC: Blackfin driver for ALSA SoC framework
by Mark Brown 03 Sep '08

03 Sep '08

On Wed, Aug 27, 2008 at 05:39:25PM +0800, Bryan Wu wrote: > From: Cliff Cai <cliff.cai(a)analog.com> > > Signed-off-by: Cliff Cai <cliff.cai(a)analog.com> > Signed-off-by: Bryan Wu <cooloney(a)kernel.org> As with the other patches in the series this looks good but it has been affected by changes in the ASoC APIs and needs to be updated to reflect current ALSA. The topic/asoc branch of Takashi's tree: git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git is what's currently queued for 2.6.28. Other than that, checkpatch-style things and a few more minor issues the main things are the register write interface in /proc and the hardware parameters for the I2S driver. I've not flagged all the checkpatch stuff here. Can I also suggest splitting this up into a series of patches for easier review - for example, adding the SPORT support first, followed by the AC97 and I2S drivers and then the machine drivers? I'd certainly have appreciated having seen the SPORT support (which appears at the end of the patch) before looking at the AC97 and I2S stuff. > + depends on BLACKFIN && SND_SOC > + help > + Say Y or M if you want to add support for codecs attached to > + the Blackfin SPORT (synchronous serial ports) interface in slot 16 > + mode (pseudo AC97 interface). > + You will also need to select the audio interfaces to support below. > + > + Note: > + AC'97 codecs which do not implment the slot-16 mode will not function > + properly with this driver. This driver is known to work with the > + Analog Devices line of AC97 codecs. Your spelling of AC97 isn't consistent here. The kernel seems to prefer AC97, as do you. > + * File: sound/soc/blackfin/bf5xx-ac97-pcm.c > + * Author: Cliff Cai <Cliff.Cai(a)analog.com> > + * > + * Created: Tue June 06 2008 > + * Description: Driver for SSM2602 sound chip built in ADSP-BF52xC Hopefully the driver is more generic than that? > +static int bf5xx_pcm_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params) > +{ > + size_t size = bf5xx_pcm_hardware.buffer_bytes_max * sizeof(struct ac97_frame)/4; Checkpatch picks up on this and quite a few other places being over 80 columns. > +static int bf5xx_pcm_trigger(struct snd_pcm_substream *substream, int cmd) > +{ > + struct snd_pcm_runtime *runtime = substream->runtime; > + struct sport_device *sport = runtime->private_data; > + int ret = 0; > + > + pr_debug("%s %s\n", substream->stream?"Capture":"Playback", \ > + cmd?" start":" stop"); This looks wrong - there's rather more options for cmd, for example, and you're making assumptions about the value of SNDRV_PMC_STREAM_PLAYBACK. > +/*Need to allocate local buffer when enable MMAP for SPORT working in TMD mode(include AC97).*/ Indentation? > diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.h b/sound/soc/blackfin/bf5xx-ac97-pcm.h > new file mode 100644 > index 0000000..5831300 > --- /dev/null > +++ b/sound/soc/blackfin/bf5xx-ac97-pcm.h > +struct bf5xx_gpio { > + u32 sys; > + u32 rx; > + u32 tx; > + u32 clk; > + u32 frm; > +}; That should be a space before rx, not a tab. > diff --git a/sound/soc/blackfin/bf5xx-ac97.c b/sound/soc/blackfin/bf5xx-ac97.c > new file mode 100644 > index 0000000..685a494 > --- /dev/null > +++ b/sound/soc/blackfin/bf5xx-ac97.c > +#include <sound/driver.h> This header is obsolte and should be removed. > +static unsigned short bf5xx_ac97_read(struct snd_ac97 *ac97, > + unsigned short reg) > +{ > + struct ac97_frame out_frame[2], in_frame[2]; > + > + pr_debug("%s enter 0x%x\n", __func__, reg); > + > + /* When dma descriptor is enabled, the register should not be read */ > + if (sport_handle->tx_run || sport_handle->rx_run) { > + printk(KERN_ERR "Could you send a mail to author " > + "to report this?\n"); > + return -EFAULT; > + } Might be nice to say what to report and who the author is - by itself the printout is going to be rather obscure :) > +static int bf5xx_ac97_resume(struct platform_device *pdev, > + struct snd_soc_cpu_dai *dai) struct snd_soc_cpu_dai and struct snd_soc_codec_dai have been merged into a single struct snd_soc_dai in current versions. > + ret = sport_config_rx(sport_handle, IRFS, 0xF, 0, (16*16-1)); > + > + if (ret) { > + printk(KERN_ERR "SPORT is busy!\n"); > + return -EBUSY; > + } > + ret = sport_config_tx(sport_handle, ITFS, 0xF, 0, (16*16-1)); > + > + if (ret) { > + printk(KERN_ERR "SPORT is busy!\n"); > + return -EBUSY; > + } Shouldn't this undo the previous sport_confix_rx() on error? Also, the use of blank lines is a bit funny here - I'd expect the blank before rather than after the function calls. > +static struct proc_dir_entry *ac_entry; > + > +/* For test purpose, read a register from codec */ > +static int proc_write(struct file *file, const char __user *buffer, > + unsigned long count, void *data) > +{ I think it's probably better to drop this from mainline - it doesn't seem like something that we should be supporting long term and obviously it could do damage if used. If you do want to put it in mainline then it ought to go in debugfs rather than proc. > + /*SPORT works in TDM mode to simulate AC97 transfers*/ > + ret = sport_set_multichannel(sport_handle, 16, 0x1F, 1); > + > + if (ret) { > + printk(KERN_ERR "SPORT is busy!\n"); > + return -EBUSY; > + } > + ret = sport_config_rx(sport_handle, IRFS, 0xF, 0, (16*16-1)); > + > + if (ret) { > + printk(KERN_ERR "SPORT is busy!\n"); > + return -EBUSY; > + } Again, odd blank lines and presumably there needs to be something to undo things on error? > + * File: sound/soc/blackfin/bf5xx-ad1980.c > + * Author: Cliff Cai <Cliff.Cai(a)analog.com> > + * > + * Created: Tue June 06 2008 o> + * Description: Driver for SSM2602 sound chip built in ADSP-BF52xC This comment is wrong - it's for the AD1980. > +static int bf5xx_board_startup(struct snd_pcm_substream *substream) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct snd_soc_cpu_dai *cpu_dai = rtd->dai->cpu_dai; > + > + pr_debug("%s enter\n", __func__); > + cpu_dai->private_data = sport_handle; > + return 0; > +} > --- /dev/null > +++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c > @@ -0,0 +1,293 @@ > +/* > + * File: sound/soc/blackfin/bf5xx-i2s-pcm.c > + * Author: Cliff Cai <Cliff.Cai(a)analog.com> > + * > + * Created: Tue June 06 2008 > + * Description: Driver for SSM2602 sound chip built in ADSP-BF52xC This comment is wrong too :) > +#include <sound/driver.h> Obsolete. > + pr_debug("%s %s\n", substream->stream?"Capture":"Playback", \ > + cmd?" start":" stop"); Same comment as for AC97. Shouldn't more of this code be shared with the AC97 DMA driver? > +static int bf5xx_pcm_close(struct snd_pcm_substream *substream) > +{ > + pr_debug("%s enter\n", __func__); > + /*Nothing need to be cleared here*/ > + > + return 0; > +} You should just be able to remove this if it's empty. > +#include <sound/driver.h> Obsolete. > + /* TX and RX are not independent,they are enabled at the same time, > + * even if only one side is running.So,we need to configure both of them in advance. > + * CPU DAI format:I2S,word length:32 bit,slave mode. > + */ Are the RX and TX clocks independant? If not you should probably tell user space about the constraint in startup(), forcing the second stream that's opened to use the same configuration as the first - take a look at the fsl_ssi driver or the wm8903 driver for examples of how to do this. I'm also not seeing the code that configures the sample rate anywhere - but then it looks like the driver only support slave mode ATM? That's what the machine driver is using. There should still be a set_fmt() to document what's supported if nothing else. > +void decfrag(struct sport_device *sport, int *frag, int tx) > +{ > + --(*frag); > + if (tx == 1 && *frag == 0) > + *frag = sport->tx_frags; > + > + if (tx == 0 && *frag == 0) > + *frag = sport->rx_frags; > +} > +EXPORT_SYMBOL(decfrag); This symbol should be namespaced. > diff --git a/sound/soc/blackfin/bf5xx-ssm2602.c b/sound/soc/blackfin/bf5xx-ssm2602.c > new file mode 100644 > index 0000000..41e161f > --- /dev/null > +++ b/sound/soc/blackfin/bf5xx-ssm2602.c This driver needs to follow the ssm2602 codec driver so either the series needs reordering or at least this bit needs to be split out into another patch. > +#include <sound/driver.h> Obsolete. > + * WARNING - TODO > + * > + * This code assumes there is a variable clocksource for the SSM2602. > + * i.e. it supplies MCLK depending on rate. > + * > + * If you are using a crystal source then modify the below case > + * statement with a static frequency. > + * > + * If you are using the SPORT to generate clocking then this is > + * where to do it. > + */ > + > + switch (params_rate(params)) { > + case 8000: > + case 16000: > + case 48000: > + case 96000: > + case 11025: > + case 22050: > + case 44100: > + clk = 12000000; > + break; > + } I'm not sure that comment quite agrees with the code here...

2 2

[alsa-devel] [PATCH 1/1] ALSA: add dummy function to support shared mmap in nommu Blackfin arch (v3)
by Bryan Wu 03 Sep '08

03 Sep '08

From: Cliff Cai <cliff.cai(a)analog.com> v2-v3: - when it is mmu arch, we define the dummy function to NULL v1-v2: - only adding this dummy function in nommu arch Cc: Mike Frysinger <vapier.adi(a)gmail.com> Signed-off-by: Cliff Cai <cliff.cai(a)analog.com> Signed-off-by: Bryan Wu <cooloney(a)kernel.org> --- sound/core/pcm_native.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c49b9d9..e2068a4 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -3391,6 +3391,16 @@ out: } #endif /* CONFIG_SND_SUPPORT_OLD_API */ +#ifndef CONFIG_MMU +unsigned long dummy_get_unmapped_area(struct file *file, unsigned long addr, + unsigned long len, unsigned long pgoff, unsigned long flags) +{ + return 0; +} +#else +# define dummy_get_unmapped_area NULL +#endif + /* * Register section */ @@ -3407,6 +3417,7 @@ const struct file_operations snd_pcm_f_ops[2] = { .compat_ioctl = snd_pcm_ioctl_compat, .mmap = snd_pcm_mmap, .fasync = snd_pcm_fasync, + .get_unmapped_area = dummy_get_unmapped_area, }, { .owner = THIS_MODULE, @@ -3419,5 +3430,6 @@ const struct file_operations snd_pcm_f_ops[2] = { .compat_ioctl = snd_pcm_ioctl_compat, .mmap = snd_pcm_mmap, .fasync = snd_pcm_fasync, + .get_unmapped_area = dummy_get_unmapped_area, } }; -- 1.5.6

2 3

[alsa-devel] [PATCH 1/1] ALSA: add dummy function to support shared mmap in nommu Blackfin arch (v2)
by Bryan Wu 03 Sep '08

03 Sep '08

From: Cliff Cai <cliff.cai(a)analog.com> v1-v2: - only adding this dummy function in nommu arch Signed-off-by: Cliff Cai <cliff.cai(a)analog.com> Signed-off-by: Bryan Wu <cooloney(a)kernel.org> --- sound/core/pcm_native.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c49b9d9..1e88cde 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -3391,6 +3391,14 @@ out: } #endif /* CONFIG_SND_SUPPORT_OLD_API */ +#ifndef CONFIG_MMU +unsigned long dummy_get_unmapped_area(struct file *file, unsigned long addr, + unsigned long len, unsigned long pgoff, unsigned long flags) +{ + return 0; +} +#endif + /* * Register section */ @@ -3407,6 +3415,9 @@ const struct file_operations snd_pcm_f_ops[2] = { .compat_ioctl = snd_pcm_ioctl_compat, .mmap = snd_pcm_mmap, .fasync = snd_pcm_fasync, +#ifndef CONFIG_MMU + .get_unmapped_area = dummy_get_unmapped_area, +#endif }, { .owner = THIS_MODULE, @@ -3419,5 +3430,8 @@ const struct file_operations snd_pcm_f_ops[2] = { .compat_ioctl = snd_pcm_ioctl_compat, .mmap = snd_pcm_mmap, .fasync = snd_pcm_fasync, +#ifndef CONFIG_MMU + .get_unmapped_area = dummy_get_unmapped_area, +#endif } }; -- 1.5.6

2 2

[alsa-devel] [PATCH] ASoC: Convert wm8510 to a new-style i2c driver (v2)
by Jean Delvare 02 Sep '08

02 Sep '08

Convert the wm8510 codec driver to the new (standard) device driver binding model. Signed-off-by: Jean Delvare <khali(a)linux-fr.org> Cc: Geoffrey Wossum <gwossum(a)acm.org> Acked-by: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- As noticed by Mark, my 1st patch didn't apply properly. Here's a fixed version which should apply fine. Sorry for the noise. Warning: this assumes that the playpaq has its codec chip on I2C bus 0, which may or may not be the case. Someone with this machine please test and report. sound/soc/at32/playpaq_wm8510.c | 1 sound/soc/codecs/wm8510.c | 110 ++++++++++++++++++--------------------- sound/soc/codecs/wm8510.h | 1 3 files changed, 55 insertions(+), 57 deletions(-) --- linux-2.6.27-rc5.orig/sound/soc/at32/playpaq_wm8510.c 2008-09-02 16:42:48.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/at32/playpaq_wm8510.c 2008-09-02 16:43:28.000000000 +0200 @@ -377,6 +377,7 @@ static struct snd_soc_machine snd_soc_ma static struct wm8510_setup_data playpaq_wm8510_setup = { + .i2c_bus = 0, .i2c_address = 0x1a, }; --- linux-2.6.27-rc5.orig/sound/soc/codecs/wm8510.c 2008-09-02 16:42:48.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/codecs/wm8510.c 2008-09-02 16:43:28.000000000 +0200 @@ -665,88 +665,86 @@ static struct snd_soc_device *wm8510_soc /* * WM8510 2 wire address is 0x1a */ -#define I2C_DRIVERID_WM8510 0xfefe /* liam - need a proper id */ -static unsigned short normal_i2c[] = { 0, I2C_CLIENT_END }; - -/* Magic definition of all other variables and things */ -I2C_CLIENT_INSMOD; - -static struct i2c_driver wm8510_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 wm8510_codec_probe(struct i2c_adapter *adap, int addr, int kind) +static int wm8510_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) { struct snd_soc_device *socdev = wm8510_socdev; - struct wm8510_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 = wm8510_init(socdev); - if (ret < 0) { + if (ret < 0) pr_err("failed to initialise WM8510\n"); - goto err; - } - return ret; -err: - kfree(i2c); return ret; } -static int wm8510_i2c_detach(struct i2c_client *client) +static int wm8510_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 wm8510_i2c_attach(struct i2c_adapter *adap) -{ - return i2c_probe(adap, &addr_data, wm8510_codec_probe); -} +static const struct i2c_device_id wm8510_i2c_id[] = { + { "wm8510", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, wm8510_i2c_id); -/* corgi i2c codec control layer */ static struct i2c_driver wm8510_i2c_driver = { .driver = { .name = "WM8510 I2C Codec", .owner = THIS_MODULE, }, - .id = I2C_DRIVERID_WM8510, - .attach_adapter = wm8510_i2c_attach, - .detach_client = wm8510_i2c_detach, - .command = NULL, + .probe = wm8510_i2c_probe, + .remove = wm8510_i2c_remove, + .id_table = wm8510_i2c_id, }; -static struct i2c_client client_template = { - .name = "WM8510", - .driver = &wm8510_i2c_driver, -}; +static int wm8510_add_i2c_device(struct platform_device *pdev, + const struct wm8510_setup_data *setup) +{ + struct i2c_board_info info; + struct i2c_adapter *adapter; + struct i2c_client *client; + int ret; + + ret = i2c_add_driver(&wm8510_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, "wm8510", 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(&wm8510_i2c_driver); + return -ENODEV; +} #endif static int wm8510_probe(struct platform_device *pdev) @@ -771,11 +769,8 @@ static int wm8510_probe(struct platform_ wm8510_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(&wm8510_i2c_driver); - if (ret != 0) - printk(KERN_ERR "can't add i2c driver"); + ret = wm8510_add_i2c_device(pdev, setup); } #else /* Add other interfaces here */ @@ -798,6 +793,7 @@ static int wm8510_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(&wm8510_i2c_driver); #endif kfree(codec); --- linux-2.6.27-rc5.orig/sound/soc/codecs/wm8510.h 2008-09-02 16:42:48.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/codecs/wm8510.h 2008-09-02 16:43:28.000000000 +0200 @@ -94,6 +94,7 @@ #define WM8510_MCLKDIV_12 (7 << 5) struct wm8510_setup_data { + int i2c_bus; unsigned short i2c_address; }; -- Jean Delvare

2 2

[alsa-devel] [PATCH 0/4] ARM: OMAP2: Add support for Gumstix Overo
by sakoman＠gmail.com 02 Sep '08

02 Sep '08

This patch series adds support for the Gumstix Overo board. Since it includes an ASoC driver I am copying both linux-omap and alsa-devel The first patch adds the basic board files, the second adds an ASoC driver for the TWL4030 codec, the third an ASoC machine driver for the Overo, and the final patch is the Overo defconfig

9 20

[alsa-devel] |PATCH] ASoC: Convert wm8510 to a new-style i2c driver
by Jean Delvare 02 Sep '08

02 Sep '08

Convert the wm8510 codec driver to the new (standard) device driver binding model. Signed-off-by: Jean Delvare <khali(a)linux-fr.org> Cc: Geoffrey Wossum <gwossum(a)acm.org> Cc: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- Warning: this assumes that the playpaq has its codec chip on I2C bus 0, which may or may not be the case. Someone with this machine please test and report. sound/soc/at32/playpaq_wm8510.c | 1 sound/soc/codecs/wm8510.c | 108 +++++++++++++++++++-------------------- sound/soc/codecs/wm8510.h | 1 3 files changed, 55 insertions(+), 55 deletions(-) --- linux-2.6.27-rc5.orig/sound/soc/at32/playpaq_wm8510.c 2008-09-01 09:59:13.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/at32/playpaq_wm8510.c 2008-09-01 09:59:21.000000000 +0200 @@ -377,6 +377,7 @@ static struct snd_soc_machine snd_soc_ma static struct wm8510_setup_data playpaq_wm8510_setup = { + .i2c_bus = 0, .i2c_address = 0x1a, }; --- linux-2.6.27-rc5.orig/sound/soc/codecs/wm8510.c 2008-09-01 09:59:13.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/codecs/wm8510.c 2008-09-01 09:59:21.000000000 +0200 @@ -665,85 +665,85 @@ static struct snd_soc_device *wm8510_soc /* * WM8510 2 wire address is 0x1a */ -static unsigned short normal_i2c[] = { 0, I2C_CLIENT_END }; -/* Magic definition of all other variables and things */ -I2C_CLIENT_INSMOD; - -static struct i2c_driver wm8510_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 wm8510_codec_probe(struct i2c_adapter *adap, int addr, int kind) +static int wm8510_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) { struct snd_soc_device *socdev = wm8510_socdev; - struct wm8510_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 = wm8510_init(socdev); - if (ret < 0) { + if (ret < 0) pr_err("failed to initialise WM8510\n"); - goto err; - } - return ret; - -err: - kfree(i2c); return ret; } -static int wm8510_i2c_detach(struct i2c_client *client) +static int wm8510_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 wm8510_i2c_attach(struct i2c_adapter *adap) -{ - return i2c_probe(adap, &addr_data, wm8510_codec_probe); -} +static const struct i2c_device_id wm8510_i2c_id[] = { + { "wm8510", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, wm8510_i2c_id); -/* corgi i2c codec control layer */ static struct i2c_driver wm8510_i2c_driver = { .driver = { .name = "WM8510 I2C Codec", .owner = THIS_MODULE, }, - .attach_adapter = wm8510_i2c_attach, - .detach_client = wm8510_i2c_detach, - .command = NULL, + .probe = wm8510_i2c_probe, + .remove = wm8510_i2c_remove, + .id_table = wm8510_i2c_id, }; -static struct i2c_client client_template = { - .name = "WM8510", - .driver = &wm8510_i2c_driver, -}; +static int wm8510_add_i2c_device(struct platform_device *pdev, + const struct wm8510_setup_data *setup) +{ + struct i2c_board_info info; + struct i2c_adapter *adapter; + struct i2c_client *client; + int ret; + + ret = i2c_add_driver(&wm8510_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, "wm8510", 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(&wm8510_i2c_driver); + return -ENODEV; +} #endif static int wm8510_probe(struct platform_device *pdev) @@ -768,11 +768,8 @@ static int wm8510_probe(struct platform_ wm8510_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(&wm8510_i2c_driver); - if (ret != 0) - printk(KERN_ERR "can't add i2c driver"); + ret = wm8510_add_i2c_device(pdev, setup);; } #else /* Add other interfaces here */ @@ -795,6 +792,7 @@ static int wm8510_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(&wm8510_i2c_driver); #endif kfree(codec); --- linux-2.6.27-rc5.orig/sound/soc/codecs/wm8510.h 2008-09-01 09:59:13.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/codecs/wm8510.h 2008-09-01 09:59:21.000000000 +0200 @@ -94,6 +94,7 @@ #define WM8510_MCLKDIV_12 (7 << 5) struct wm8510_setup_data { + int i2c_bus; unsigned short i2c_address; }; -- Jean Delvare

3 4

[alsa-devel] [PATCH 0/4] Blackfin supports for ALSA/ASOC
by Bryan Wu 02 Sep '08

02 Sep '08

Hi folks, It's very happy to release this ASOC Blackfin ports. Please review our drivers and feel free to ask us update it. In this patchset, we post ASOC Blackfin supports, SSM2602 audio codec driver and other 2 ALSA/ASOC patches. Thanks a lot -Bryan

8 21

[alsa-devel] get rid of controls with snd_ctl_remove
by Harald Radke 02 Sep '08

02 Sep '08

Hi there! Please be patient with me, I am quite inexperienced with alsa programming, so this might be a dumb question: We have a WM8750 working in our PDA, the codec is working so far but I want to remove some of the control since they aren't needed. Instead of copying the wm8750.c file and adjusting it, I want to write another module which basically get rid of those controls, something like: [...] static const char* unused_controls[] = {"AAAA", "BBBB,"CCCC"}; struct snd_kcontrol *ctl; list_for_each_entry(ctl, &card->controls, list) for(i=0; i < ARRAY_SIZE(unused_controls); i++) if (!strcmp(unused_controls[i],ctl->id.name)) { down_write(&card->controls_rwsem); snd_ctl_remove(card,ctl); up_write(&card->controls_rwsem); break; } unfortunatly this doesn't work (module segfaults...."of course" you will say *g*) My question: is it possible at all and I am just not deep nuff in understanding the sound system or do I have to adjust the original code? Thx Harry

4 14

[alsa-devel] [PATCH 2/3] ASoC: Convert wm8753 to a new-style i2c driver
by Jean Delvare 01 Sep '08

01 Sep '08

Convert the wm8753 codec driver to the new (standard) i2c device driver binding model. Signed-off-by: Jean Delvare <khali(a)linux-fr.org> Cc: Ville Syrjala <syrjala(a)sci.fi> Cc: Frank Mandarino <fmandarino(a)endrelia.com> Cc: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- Warning: this assumes that the neo1973 has its codec chip on I2C bus 0, which may or may not be the case. Someone with this machine please test and report. include/linux/i2c-id.h | 1 sound/soc/codecs/wm8753.c | 106 ++++++++++++++++++------------------ sound/soc/codecs/wm8753.h | 1 sound/soc/s3c24xx/neo1973_wm8753.c | 1 4 files changed, 55 insertions(+), 54 deletions(-) --- linux-2.6.27-rc5.orig/include/linux/i2c-id.h 2008-09-01 11:06:04.000000000 +0200 +++ linux-2.6.27-rc5/include/linux/i2c-id.h 2008-09-01 12:46:57.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_WM8753 91 /* Wolfson WM8753 audio codec */ #define I2C_DRIVERID_LM4857 92 /* LM4857 Audio Amplifier */ #define I2C_DRIVERID_VP27SMPX 93 /* Panasonic VP27s tuner internal MPX */ #define I2C_DRIVERID_CS4270 94 /* Cirrus Logic 4270 audio codec */ --- linux-2.6.27-rc5.orig/sound/soc/codecs/wm8753.c 2008-08-29 09:32:25.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/codecs/wm8753.c 2008-09-01 12:49:46.000000000 +0200 @@ -1637,84 +1637,86 @@ static struct snd_soc_device *wm8753_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 wm8753_i2c_driver; -static struct i2c_client client_template; - -static int wm8753_codec_probe(struct i2c_adapter *adap, int addr, int kind) +static int wm8753_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) { struct snd_soc_device *socdev = wm8753_socdev; - struct wm8753_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) - 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 = wm8753_init(socdev); - if (ret < 0) { + if (ret < 0) pr_err("failed to initialise WM8753\n"); - goto err; - } - - return ret; -err: - kfree(i2c); return ret; } -static int wm8753_i2c_detach(struct i2c_client *client) +static int wm8753_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 wm8753_i2c_attach(struct i2c_adapter *adap) -{ - return i2c_probe(adap, &addr_data, wm8753_codec_probe); -} +static const struct i2c_device_id wm8753_i2c_id[] = { + { "wm8753", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, wm8753_i2c_id); -/* corgi i2c codec control layer */ static struct i2c_driver wm8753_i2c_driver = { .driver = { .name = "WM8753 I2C Codec", .owner = THIS_MODULE, }, - .id = I2C_DRIVERID_WM8753, - .attach_adapter = wm8753_i2c_attach, - .detach_client = wm8753_i2c_detach, - .command = NULL, + .probe = wm8753_i2c_probe, + .remove = wm8753_i2c_remove, + .id_table = wm8753_i2c_id, }; -static struct i2c_client client_template = { - .name = "WM8753", - .driver = &wm8753_i2c_driver, -}; +static int wm8753_add_i2c_device(struct platform_device *pdev, + const struct wm8753_setup_data *setup) +{ + struct i2c_board_info info; + struct i2c_adapter *adapter; + struct i2c_client *client; + int ret; + + ret = i2c_add_driver(&wm8753_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, "wm8753", 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(&wm8753_i2c_driver); + return -ENODEV; +} #endif static int wm8753_probe(struct platform_device *pdev) @@ -1748,11 +1750,8 @@ static int wm8753_probe(struct platform_ #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(&wm8753_i2c_driver); - if (ret != 0) - printk(KERN_ERR "can't add i2c driver"); + ret = wm8753_add_i2c_device(pdev, setup); } #else /* Add other interfaces here */ @@ -1796,6 +1795,7 @@ static int wm8753_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(&wm8753_i2c_driver); #endif kfree(codec->private_data); --- linux-2.6.27-rc5.orig/sound/soc/codecs/wm8753.h 2008-08-05 18:18:18.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/codecs/wm8753.h 2008-09-01 12:44:52.000000000 +0200 @@ -79,6 +79,7 @@ #define WM8753_ADCTL2 0x3f struct wm8753_setup_data { + int i2c_bus; unsigned short i2c_address; }; --- linux-2.6.27-rc5.orig/sound/soc/s3c24xx/neo1973_wm8753.c 2008-08-23 22:18:36.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/s3c24xx/neo1973_wm8753.c 2008-09-01 12:45:10.000000000 +0200 @@ -586,6 +586,7 @@ static struct snd_soc_machine neo1973 = }; static struct wm8753_setup_data neo1973_wm8753_setup = { + .i2c_bus = 0, .i2c_address = 0x1a, }; -- Jean Delvare

2 1

[alsa-devel] [PATCH 3/3] ASoC: Convert neo1973/lm4857 to a new-style i2c driver
by Jean Delvare 01 Sep '08

01 Sep '08

Convert the lm4857 driver in neo1973_wm8753 to the new (standard) i2c device driver binding model. I assumed that the LM4857 was always on the same I2C bus as the WM8753 codec. Signed-off-by: Jean Delvare <khali(a)linux-fr.org> Cc: Tim Niemeyer <reddog(a)mastersword.de> Cc: Graeme Gregory <graeme(a)openmoko.org> Cc: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- I couldn't even test-build that one. Tim, Graeme, can any of you please review and test this patch? Thanks. include/linux/i2c-id.h | 1 sound/soc/s3c24xx/neo1973_wm8753.c | 110 ++++++++++++++++++------------------ 2 files changed, 58 insertions(+), 53 deletions(-) --- linux-2.6.27-rc5.orig/include/linux/i2c-id.h 2008-09-01 14:31:28.000000000 +0200 +++ linux-2.6.27-rc5/include/linux/i2c-id.h 2008-09-01 14:46:19.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_LM4857 92 /* LM4857 Audio Amplifier */ #define I2C_DRIVERID_VP27SMPX 93 /* Panasonic VP27s tuner internal MPX */ #define I2C_DRIVERID_CS4270 94 /* Cirrus Logic 4270 audio codec */ #define I2C_DRIVERID_M52790 95 /* Mitsubishi M52790SP/FP AV switch */ --- linux-2.6.27-rc5.orig/sound/soc/s3c24xx/neo1973_wm8753.c 2008-09-01 14:31:30.000000000 +0200 +++ linux-2.6.27-rc5/sound/soc/s3c24xx/neo1973_wm8753.c 2008-09-01 15:02:39.000000000 +0200 @@ -597,54 +597,20 @@ static struct snd_soc_device neo1973_snd .codec_data = &neo1973_wm8753_setup, }; -static struct i2c_client client_template; - -static const unsigned short normal_i2c[] = { 0x7C, I2C_CLIENT_END }; - -/* Magic definition of all other variables and things */ -I2C_CLIENT_INSMOD; - -static int lm4857_amp_probe(struct i2c_adapter *adap, int addr, int kind) +static int lm4857_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) { - int ret; - DBG("Entered %s\n", __func__); - client_template.adapter = adap; - client_template.addr = addr; - - i2c = kmemdup(&client_template, sizeof(client_template), GFP_KERNEL); - if (i2c == NULL) - return -ENOMEM; - - ret = i2c_attach_client(i2c); - if (ret < 0) { - printk(KERN_ERR "LM4857 failed to attach at addr %x\n", addr); - goto exit_err; - } - lm4857_write_regs(); - return ret; - -exit_err: - kfree(i2c); - return ret; -} - -static int lm4857_i2c_detach(struct i2c_client *client) -{ - DBG("Entered %s\n", __func__); - - i2c_detach_client(client); - kfree(client); return 0; } -static int lm4857_i2c_attach(struct i2c_adapter *adap) +static int lm4857_i2c_remove(struct i2c_client *client) { DBG("Entered %s\n", __func__); - return i2c_probe(adap, &addr_data, lm4857_amp_probe); + return 0; } static u8 lm4857_state; @@ -682,27 +648,67 @@ static void lm4857_shutdown(struct i2c_c lm4857_write_regs(); } -/* corgi i2c codec control layer */ +static const struct i2c_device_id lm4857_i2c_id[] = { + { "neo1973_lm4857", 0 } + { } +}; + static struct i2c_driver lm4857_i2c_driver = { .driver = { .name = "LM4857 I2C Amp", .owner = THIS_MODULE, }, - .id = I2C_DRIVERID_LM4857, .suspend = lm4857_suspend, .resume = lm4857_resume, .shutdown = lm4857_shutdown, - .attach_adapter = lm4857_i2c_attach, - .detach_client = lm4857_i2c_detach, - .command = NULL, -}; - -static struct i2c_client client_template = { - .name = "LM4857", - .driver = &lm4857_i2c_driver, + .probe = lm4857_i2c_probe, + .remove = lm4857_i2c_remove, + .id_table = lm4857_i2c_id, }; static struct platform_device *neo1973_snd_device; +static struct i2c_client *lm4857_client; + +static int __init neo1973_add_lm4857_device(struct platform_device *pdev, + int i2c_bus, + unsigned short i2c_address) +{ + struct i2c_board_info info; + struct i2c_adapter *adapter; + struct i2c_client *client; + int ret; + + ret = i2c_add_driver(&lm4857_i2c_driver); + if (ret != 0) { + dev_err(&pdev->dev, "can't add lm4857 driver\n"); + return ret; + } + + memset(&info, 0, sizeof(struct i2c_board_info)); + info.addr = i2c_address; + strlcpy(info.type, "neo1973_lm4857", I2C_NAME_SIZE); + + adapter = i2c_get_adapter(i2c_bus); + if (!adapter) { + dev_err(&pdev->dev, "can't get i2c adapter %d\n", i2c_bus); + goto err_driver; + } + + client = i2c_new_device(adapter, &info); + i2c_put_adapter(adapter); + if (!client) { + dev_err(&pdev->dev, "can't add lm4857 device at 0x%x\n", + (unsigned int)info.addr); + goto err_driver; + } + + lm4857_client = client; + return 0; + +err_driver: + i2c_del_driver(&lm4857_i2c_driver); + return -ENODEV; +} static int __init neo1973_init(void) { @@ -723,11 +729,10 @@ static int __init neo1973_init(void) return ret; } - ret = i2c_add_driver(&lm4857_i2c_driver); - if (ret != 0) { - printk(KERN_ERR "can't add i2c driver"); + ret = neo1973_add_lm4857_device(neo1973_snd_device, + neo1973_wm8753_setup, 0x7C); + if (ret != 0) platform_device_unregister(neo1973_snd_device); - } return ret; } @@ -736,6 +741,7 @@ static void __exit neo1973_exit(void) { DBG("Entered %s\n", __func__); + i2c_unregister_device(lm4857_client); i2c_del_driver(&lm4857_i2c_driver); platform_device_unregister(neo1973_snd_device); } -- Jean Delvare

1 0