[PATCH] ASoC: dapm: Replace siggen widget type with mic widgets

[PATCH] ASoC: dapm: Replace siggen widget type with mic widgets

[Charles Keepax]

The siggen widget type doesn't really require its own special handling.
It can be handled identically to a mic widget, ie. it is a source of a
signal and causes things to power up when an output is connected.

Additionally, several places (cs42l52, cs42l56, wm8962, arizona_haptics)
tie siggens in to the input framework and the pattern for this is
usually to use snd_soc_dapm_enable_pin/disable_pin on the siggen widget.
However, as siggen widgets present as always on (the power check always
returns 1), this leads to some odd interactions with DAPM.
Enabling/disabling the widget then connecting it to a path works as
expected, however once in a path enabling the widget has no effect.
dapm_widget_set_power will return immediately as the current power state
of the widget will return 1, meaning we never check peer power states.
Treating the widget as a mic widget causes the expected behaviour in all
cases.

This does cause a minor cosmetic side effect that siggen widgets no
longer show as always on through debugfs, although in the case described
above that is more representive of their behaviour.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 include/sound/soc-dapm.h | 2 +-
 sound/soc/soc-dapm.c     | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 9706946..67f176f 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -47,7 +47,7 @@ struct device;
 
 /* platform domain */
 #define SND_SOC_DAPM_SIGGEN(wname) \
-{	.id = snd_soc_dapm_siggen, .name = wname, .kcontrol_news = NULL, \
+{	.id = snd_soc_dapm_mic, .name = wname, .kcontrol_news = NULL, \
 	.num_kcontrols = 0, .reg = SND_SOC_NOPM }
 #define SND_SOC_DAPM_SINK(wname) \
 {	.id = snd_soc_dapm_sink, .name = wname, .kcontrol_news = NULL, \
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index a8156b5..ea6f477 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -1817,12 +1817,9 @@ static int dapm_power_widgets(struct snd_soc_card *card, int event)
 			/* Supplies and micbiases only bring the
 			 * context up to STANDBY as unless something
 			 * else is active and passing audio they
-			 * generally don't require full power.  Signal
-			 * generators are virtual pins and have no
-			 * power impact themselves.
+			 * generally don't require full power.
 			 */
 			switch (w->id) {
-			case snd_soc_dapm_siggen:
 			case snd_soc_dapm_vmid:
 				break;
 			case snd_soc_dapm_supply:
@@ -3354,7 +3351,6 @@ snd_soc_dapm_new_control_unlocked(struct snd_soc_dapm_context *dapm,
 		w->power_check = dapm_generic_check_power;
 		break;
 	case snd_soc_dapm_vmid:
-	case snd_soc_dapm_siggen:
 		w->is_ep = SND_SOC_DAPM_EP_SOURCE;
 		w->power_check = dapm_always_on_check_power;
 		break;
-- 
2.1.4

Re: [PATCH] ASoC: dapm: Replace siggen widget type with mic widgets

[Lars-Peter Clausen]

On 12/01/2015 06:32 PM, Charles Keepax wrote:
> The siggen widget type doesn't really require its own special handling.
> It can be handled identically to a mic widget, ie. it is a source of a
> signal and causes things to power up when an output is connected.
> 
> Additionally, several places (cs42l52, cs42l56, wm8962, arizona_haptics)
> tie siggens in to the input framework and the pattern for this is
> usually to use snd_soc_dapm_enable_pin/disable_pin on the siggen widget.
> However, as siggen widgets present as always on (the power check always
> returns 1), this leads to some odd interactions with DAPM.
> Enabling/disabling the widget then connecting it to a path works as
> expected, however once in a path enabling the widget has no effect.
> dapm_widget_set_power will return immediately as the current power state
> of the widget will return 1, meaning we never check peer power states.
> Treating the widget as a mic widget causes the expected behaviour in all
> cases.

Sounds sensible, but I'd keep the siggen type and just change the
power_check callback. snd_soc_dapm_mic widgets are a bit special in that
they also influence how input and output widgets behave, we don't want the
same behavior for siggen widgets, since siggen widgets are internal to CODEC
while mic widgets are usually external.

Re: [PATCH] ASoC: dapm: Replace siggen widget type with mic widgets

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 910 bytes --]

On Tue, Dec 01, 2015 at 07:57:11PM +0100, Lars-Peter Clausen wrote:
> On 12/01/2015 06:32 PM, Charles Keepax wrote:

> > The siggen widget type doesn't really require its own special handling.
> > It can be handled identically to a mic widget, ie. it is a source of a
> > signal and causes things to power up when an output is connected.

> > However, as siggen widgets present as always on (the power check always
> > returns 1), this leads to some odd interactions with DAPM.

> Sounds sensible, but I'd keep the siggen type and just change the
> power_check callback. snd_soc_dapm_mic widgets are a bit special in that
> they also influence how input and output widgets behave, we don't want the
> same behavior for siggen widgets, since siggen widgets are internal to CODEC
> while mic widgets are usually external.

Yeah, the siggen widgets were created because mics are funky - best to
just fix the bug.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: dapm: Replace siggen widget type with mic widgets

[Charles Keepax]

On Tue, Dec 01, 2015 at 07:36:13PM +0000, Mark Brown wrote:
> On Tue, Dec 01, 2015 at 07:57:11PM +0100, Lars-Peter Clausen wrote:
> > On 12/01/2015 06:32 PM, Charles Keepax wrote:
> 
> > > The siggen widget type doesn't really require its own special handling.
> > > It can be handled identically to a mic widget, ie. it is a source of a
> > > signal and causes things to power up when an output is connected.
> 
> > > However, as siggen widgets present as always on (the power check always
> > > returns 1), this leads to some odd interactions with DAPM.
> 
> > Sounds sensible, but I'd keep the siggen type and just change the
> > power_check callback. snd_soc_dapm_mic widgets are a bit special in that
> > they also influence how input and output widgets behave, we don't want the
> > same behavior for siggen widgets, since siggen widgets are internal to CODEC
> > while mic widgets are usually external.
> 
> Yeah, the siggen widgets were created because mics are funky - best to
> just fix the bug.

Thanks guys, will respin today.

Thanks,
Charles