All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Liam Girdwood <lrg@ti.com>,
	alsa-devel@alsa-project.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Help request - ASoC recursion issue
Date: Mon, 23 Jul 2012 15:05:19 +0100	[thread overview]
Message-ID: <500D5A1F.9000808@linaro.org> (raw)

Hi Mark,

I was wondering if I may bother you for some help. I've been having 
serious issues with testing the new mop500 sound system you have in your 
ASoC for-next branch. I've fixed a few issues and will be submitting 
patches shortly. The most serious issue I came across was with 
recursion. Let me show you:

So here we setup the power_check function pointer with 
'dapm_supply_check_power()'.

> static struct snd_soc_dapm_widget *
> snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
> 			 const struct snd_soc_dapm_widget *widget)
> {
<snip>
> 	case snd_soc_dapm_regulator_supply:
> 	case snd_soc_dapm_clock_supply:
> 		w->power_check = dapm_supply_check_power;
> 		break;
<snip>
> }

Later we call 'dapm_widget_power_check()' which calls into the function 
pointer we know to be 'dapm_supply_check_power()'.

 > static int dapm_widget_power_check(struct snd_soc_dapm_widget *w)
 > {
 > 	if (w->power_checked)
 > 		return w->new_power;
 >
 > 	if (w->force)
 > 		w->new_power = 1;
 > 	else
 > 		w->new_power = w->power_check(w);
 >
 > 	w->power_checked = true;
 >
 > 	return w->new_power;
 > }

The problem seems to be that 'dapm_supply_check_power()' then calls back 
into 'dapm_widget_power_check()'. Then round and round we go!

> /* Check to see if a power supply is needed */
> static int dapm_supply_check_power(struct snd_soc_dapm_widget *w)
> {
<snip>
> 	/* Check if one of our outputs is connected */
> 	list_for_each_entry(path, &w->sinks, list_source) {
> 		DAPM_UPDATE_STAT(w, neighbour_checks);
>
> 		if (path->weak)
> 			continue;
>
> 		if (path->connected &&
> 		    !path->connected(path->source, path->sink))
> 			continue;
>
> 		if (!path->sink)
> 			continue;
>
> 		if (dapm_widget_power_check(path->sink))     /* <-- Doh! */
> 			return 1;
> 	}
<snip>
> }

Can you shed some light on what the correct solution might be?

Any help would be gratefully received.

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

WARNING: multiple messages have this Message-ID (diff)
From: lee.jones@linaro.org (Lee Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: Help request - ASoC recursion issue
Date: Mon, 23 Jul 2012 15:05:19 +0100	[thread overview]
Message-ID: <500D5A1F.9000808@linaro.org> (raw)

Hi Mark,

I was wondering if I may bother you for some help. I've been having 
serious issues with testing the new mop500 sound system you have in your 
ASoC for-next branch. I've fixed a few issues and will be submitting 
patches shortly. The most serious issue I came across was with 
recursion. Let me show you:

So here we setup the power_check function pointer with 
'dapm_supply_check_power()'.

> static struct snd_soc_dapm_widget *
> snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
> 			 const struct snd_soc_dapm_widget *widget)
> {
<snip>
> 	case snd_soc_dapm_regulator_supply:
> 	case snd_soc_dapm_clock_supply:
> 		w->power_check = dapm_supply_check_power;
> 		break;
<snip>
> }

Later we call 'dapm_widget_power_check()' which calls into the function 
pointer we know to be 'dapm_supply_check_power()'.

 > static int dapm_widget_power_check(struct snd_soc_dapm_widget *w)
 > {
 > 	if (w->power_checked)
 > 		return w->new_power;
 >
 > 	if (w->force)
 > 		w->new_power = 1;
 > 	else
 > 		w->new_power = w->power_check(w);
 >
 > 	w->power_checked = true;
 >
 > 	return w->new_power;
 > }

The problem seems to be that 'dapm_supply_check_power()' then calls back 
into 'dapm_widget_power_check()'. Then round and round we go!

> /* Check to see if a power supply is needed */
> static int dapm_supply_check_power(struct snd_soc_dapm_widget *w)
> {
<snip>
> 	/* Check if one of our outputs is connected */
> 	list_for_each_entry(path, &w->sinks, list_source) {
> 		DAPM_UPDATE_STAT(w, neighbour_checks);
>
> 		if (path->weak)
> 			continue;
>
> 		if (path->connected &&
> 		    !path->connected(path->source, path->sink))
> 			continue;
>
> 		if (!path->sink)
> 			continue;
>
> 		if (dapm_widget_power_check(path->sink))     /* <-- Doh! */
> 			return 1;
> 	}
<snip>
> }

Can you shed some light on what the correct solution might be?

Any help would be gratefully received.

Kind regards,
Lee

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

             reply	other threads:[~2012-07-23 14:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-23 14:05 Lee Jones [this message]
2012-07-23 14:05 ` Help request - ASoC recursion issue Lee Jones
2012-07-23 14:39 ` Mark Brown
2012-07-23 14:39   ` Mark Brown
2012-07-23 14:39   ` Mark Brown
2012-07-23 14:50 ` Lee Jones
2012-07-23 14:50   ` Lee Jones
2012-07-23 14:56   ` Lee Jones
2012-07-23 14:56     ` Lee Jones
2012-07-23 18:12     ` Will Deacon
2012-07-23 18:12       ` Will Deacon
2012-07-24  7:26       ` Lee Jones
2012-07-24  7:26         ` Lee Jones
2012-07-24 22:46         ` Mark Brown
2012-07-24 22:46           ` Mark Brown
2012-07-24 22:46           ` Mark Brown

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=500D5A1F.9000808@linaro.org \
    --to=lee.jones@linaro.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lrg@ti.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.