alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Sangsu Park <sangsu@gmail.com>
To: broonie@opensource.wolfsonmicro.com
Cc: sbkim73@samsung.com, alsa-devel@alsa-project.org, lrg@ti.com,
	sangsu4u.park@samsung.com
Subject: Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
Date: Tue, 27 Dec 2011 16:49:46 +0900	[thread overview]
Message-ID: <CANh+=Y8HVjkChnMQLzHmGbpbxRu3wxqbUDp2SQ3SpQhpC-0oug@mail.gmail.com> (raw)

> 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
[<c01d1ba4>] (__schedule+0x244/0x6cc) from [<c01d2db0>]
(__mutex_lock_slowpath+0x180/0x360)
[<c01d2db0>] (__mutex_lock_slowpath+0x180/0x360) from [<c01d2f9c>]
(mutex_lock+0xc/0x24)
[<c01d2f9c>] (mutex_lock+0xc/0x24) from [<c01c10c4>] (soc_pcm_open+0x38/0x48c)
[<c01c10c4>] (soc_pcm_open+0x38/0x48c) from [<c01c1114>]
(soc_pcm_open+0x88/0x48c)
[<c01c1114>] (soc_pcm_open+0x88/0x48c) from [<c01b0fcc>]
(snd_pcm_open_substream+0x60/0xa0)
[<c01b0fcc>] (snd_pcm_open_substream+0x60/0xa0) from [<c01b1134>]
(snd_pcm_open+0x128/0x270)
[<c01b1134>] (snd_pcm_open+0x128/0x270) from [<c01a4034>] (snd_open+0x130/0x2ec)
[<c01a4034>] (snd_open+0x130/0x2ec) from [<c00b00e4>] (chrdev_open+0x10c/0x1f0)
[<c00b00e4>] (chrdev_open+0x10c/0x1f0) from [<c00aad24>]
(__dentry_open+0x220/0x328)
[<c00aad24>] (__dentry_open+0x220/0x328) from [<c00aaef4>]
(nameidata_to_filp+0x60/0x68)
[<c00aaef4>] (nameidata_to_filp+0x60/0x68) from [<c00bb5d4>]
(do_last+0x214/0xcc0)
[<c00bb5d4>] (do_last+0x214/0xcc0) from [<c00bc13c>] (path_openat+0xbc/0x3a8)
[<c00bc13c>] (path_openat+0xbc/0x3a8) from [<c00bc510>] (do_filp_open+0x30/0x84)
[<c00bc510>] (do_filp_open+0x30/0x84) from [<c00aa954>] (do_sys_open+0xe4/0x184)
[<c00aa954>] (do_sys_open+0xe4/0x184) from [<c000f3c0>]
(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?

             reply	other threads:[~2011-12-27  7:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-27  7:49 Sangsu Park [this message]
2011-12-27 10:34 ` [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2011-12-27  8:06 Sangsu Park
2011-12-27 10:40 ` Mark Brown
2011-12-23  1:37 Sangbeom Kim
2011-12-23  9:42 ` Liam Girdwood
2011-12-23 10:21   ` Mark Brown
2011-12-23 10:24 ` Mark Brown
2011-12-23  0:26 Sangbeom Kim
2011-12-23 11:36 ` Lars-Peter Clausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANh+=Y8HVjkChnMQLzHmGbpbxRu3wxqbUDp2SQ3SpQhpC-0oug@mail.gmail.com' \
    --to=sangsu@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=lrg@ti.com \
    --cc=sangsu4u.park@samsung.com \
    --cc=sbkim73@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).