From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sangsu Park Subject: Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs Date: Tue, 27 Dec 2011 16:49:46 +0900 Message-ID: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-gx0-f179.google.com (mail-gx0-f179.google.com [209.85.161.179]) by alsa0.perex.cz (Postfix) with ESMTP id 5DCF41038FA for ; Tue, 27 Dec 2011 08:49:47 +0100 (CET) Received: by ggnv5 with SMTP id v5so7783884ggn.38 for ; Mon, 26 Dec 2011 23:49:46 -0800 (PST) List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: broonie@opensource.wolfsonmicro.com Cc: sbkim73@samsung.com, alsa-devel@alsa-project.org, lrg@ti.com, sangsu4u.park@samsung.com List-Id: alsa-devel@alsa-project.org > On Fri, Dec 23, 2011 at 10:37:51AM +0900, Sangbeom Kim wrote: > > > + soc_pcm_ops = kzalloc(sizeof(*soc_pcm_ops), GFP_KERNEL); > > + if (soc_pcm_ops == NULL) { > > + snd_printk(KERN_ERR "Cannot allocate PCM OPS\n"); > > + return -ENOMEM; > > + } > > This patch is good and fixes a real problem but can you please change it > slightly so that instead of dynamically allocating the soc_pcm_ops we > just embed it directly in the runtime structure. Since every runtime > needs an ops structure we may as well just have it embedded directly and > not write the allocation, freeing and error handling code. Hi, I'm Sangsu Park. (sangsu4u.park@samsung.com) I'm using gmail because of company's firewall problems... I've implement your guide like follows. //-------------------------------------------------------------------------------------------------------------------------------- struct snd_soc_dai *codec_dai = rtd->codec_dai; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_pcm_ops *soc_pcm_ops = platform->driver->ops; struct snd_pcm *pcm; char new_name[64]; int ret = 0, playback = 0, capture = 0; if (playback) - snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops); + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, soc_pcm_ops); if (capture) - snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops); + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, soc_pcm_ops); --------------------------------------------------------------------------------------------------------------------------------// But, it seems to make mutex dead lock. When starting playback, it tells some messages. //-------------------------------------------------------------------------------------------------------------------------------- # aplay -twav Test.wav INFO: task aplay:1071 blocked for more than 120 seconds. "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. aplay D c01d1ba4 0 1071 1051 0x00000000 [] (__schedule+0x244/0x6cc) from [] (__mutex_lock_slowpath+0x180/0x360) [] (__mutex_lock_slowpath+0x180/0x360) from [] (mutex_lock+0xc/0x24) [] (mutex_lock+0xc/0x24) from [] (soc_pcm_open+0x38/0x48c) [] (soc_pcm_open+0x38/0x48c) from [] (soc_pcm_open+0x88/0x48c) [] (soc_pcm_open+0x88/0x48c) from [] (snd_pcm_open_substream+0x60/0xa0) [] (snd_pcm_open_substream+0x60/0xa0) from [] (snd_pcm_open+0x128/0x270) [] (snd_pcm_open+0x128/0x270) from [] (snd_open+0x130/0x2ec) [] (snd_open+0x130/0x2ec) from [] (chrdev_open+0x10c/0x1f0) [] (chrdev_open+0x10c/0x1f0) from [] (__dentry_open+0x220/0x328) [] (__dentry_open+0x220/0x328) from [] (nameidata_to_filp+0x60/0x68) [] (nameidata_to_filp+0x60/0x68) from [] (do_last+0x214/0xcc0) [] (do_last+0x214/0xcc0) from [] (path_openat+0xbc/0x3a8) [] (path_openat+0xbc/0x3a8) from [] (do_filp_open+0x30/0x84) [] (do_filp_open+0x30/0x84) from [] (do_sys_open+0xe4/0x184) [] (do_sys_open+0xe4/0x184) from [] (ret_fast_syscall+0x0/0x30) --------------------------------------------------------------------------------------------------------------------------------// "sound/core/" uses soc_pcm_ops using substream->ops, and "sound/soc/" uses soc_pcm_ops using platform->driver->ops. So, I think that they must have their own soc_pcm_ops object. How do you think about this?