[RFC][PATCH 0/3] ASoC: remove card->pop_time

[RFC][PATCH 0/3] ASoC: remove card->pop_time

[Kuninori Morimoto]

Hi Mark, OMAP member

I'm now trying to cleanup snd_soc_card, and noticed that card->pop_time is used
only from OMAP1 board-ams-delta.

card->pop_time user drivers are...

	> git grep -w -l pop_time sound/soc include/sound
	include/sound/soc.h
(A)	sound/soc/codecs/cx20442.c
	sound/soc/soc-card.c
	sound/soc/soc-dapm.c
(B)	sound/soc/ti/ams-delta.c

Except ASoC framework, pop_time user is only cx20442 (A) / ams-delta (B).
And board-ams-delta (Z) is using it.

(Z)	linux/arch/arm/mach-omap1/board-ams-delta.c

	static struct platform_device ams_delta_audio_device = {
(A)		.name   = "ams-delta-audio",
	};

	static struct platform_device cx20442_codec_device = {
(B)		.name   = "cx20442-codec",
	};

	static struct platform_device *ams_delta_devices[] __initdata = {
		...
(A)		&ams_delta_audio_device,
		...
(B)		&cx20442_codec_device,
		...
	};

With off-list talk with Mark, this card->pop_time is is much less of a
problem. So let's remove it. But it needs Acked-by from OMAP1 member.
I added [RFC] on Subject.

Kuninori Morimoto (3):
  ASoC: ti: amx-delta: don't use card->pop_time
  ASoC: cx20442: don't use card->pop_time
  ASoC: soc-card: remvoe card->pop_time

 include/sound/soc.h        |  2 --
 sound/soc/codecs/cx20442.c |  3 --
 sound/soc/soc-core.c       |  3 --
 sound/soc/soc-dapm.c       | 43 ------------------------
 sound/soc/ti/ams-delta.c   | 67 +-------------------------------------
 5 files changed, 1 insertion(+), 117 deletions(-)

-- 
2.43.0

[RFC][PATCH 1/3] ASoC: ti: amx-delta: don't use card->pop_time

[Kuninori Morimoto]

Very few user uses card->pop_time and pop/click stuff is much less of a
problem. Let's don't use it.

card->pop_time is used at 2 parts.

1st one is at ams_delta_set_audio_mode() (A).
It is pair as set/get (A)(B). (A) did nothing if card->pop_time was 0 (C).
So I guess, this driver is assuming to be used with card->pop_time.
This patch removes (C).

(A)	static int ams_delta_set_audio_mode(...)
	{
		...
(C)		if (!cx20442_codec->card->pop_time)
			return -EUNATCH;
		...
	}

	static const struct snd_kcontrol_new ams_delta_audio_controls[] = {
		SOC_ENUM_EXT("Audio Mode", ams_delta_audio_enum,
			ams_delta_get_audio_mode, ams_delta_set_audio_mode),
	};		(B)			  (A)

2nd one is at cx81801_receive() (a).
It has 2 parts (b)(c). Because of above guess, Let's remove (b) part.

(a)	static void cx81801_receive(...)
	{
 ^		if (!component->card->pop_time) {
(b)			...
 v			return;
		}
 ^		...
(c)
 v		return;
	}

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/ti/ams-delta.c | 67 +---------------------------------------
 1 file changed, 1 insertion(+), 66 deletions(-)

diff --git a/sound/soc/ti/ams-delta.c b/sound/soc/ti/ams-delta.c
index ba173d9fcba91..4f3b0cf1e68d8 100644
--- a/sound/soc/ti/ams-delta.c
+++ b/sound/soc/ti/ams-delta.c
@@ -104,10 +104,6 @@ static int ams_delta_set_audio_mode(struct snd_kcontrol *kcontrol,
 	unsigned short pins;
 	int pin, changed = 0;
 
-	/* Refuse any mode changes if we are not able to control the codec. */
-	if (!cx20442_codec->card->pop_time)
-		return -EUNATCH;
-
 	if (ucontrol->value.enumerated.item[0] >= control->items)
 		return -EINVAL;
 
@@ -220,32 +216,6 @@ static struct snd_soc_jack_gpio ams_delta_hook_switch_gpios[] = {
 	}
 };
 
-/* After we are able to control the codec over the modem,
- * the hook switch can be used for dynamic DAPM reconfiguration. */
-static struct snd_soc_jack_pin ams_delta_hook_switch_pins[] = {
-	/* Handset */
-	{
-		.pin = "Mouthpiece",
-		.mask = SND_JACK_MICROPHONE,
-	},
-	{
-		.pin = "Earpiece",
-		.mask = SND_JACK_HEADPHONE,
-	},
-	/* Handsfree */
-	{
-		.pin = "Microphone",
-		.mask = SND_JACK_MICROPHONE,
-		.invert = 1,
-	},
-	{
-		.pin = "Speaker",
-		.mask = SND_JACK_HEADPHONE,
-		.invert = 1,
-	},
-};
-
-
 /*
  * Modem line discipline, required for making above controls functional.
  * Activated from userspace with ldattach, possibly invoked from udev rule.
@@ -260,21 +230,6 @@ static bool ams_delta_muted;
 static DEFINE_SPINLOCK(ams_delta_lock);
 static struct gpio_desc *gpiod_modem_codec;
 
-static void cx81801_timeout(struct timer_list *unused)
-{
-	int muted;
-
-	spin_lock(&ams_delta_lock);
-	cx81801_cmd_pending = 0;
-	muted = ams_delta_muted;
-	spin_unlock(&ams_delta_lock);
-
-	/* Reconnect the codec DAI back from the modem to the CPU DAI
-	 * only if digital mute still off */
-	if (!muted)
-		gpiod_set_value(gpiod_modem_codec, 0);
-}
-
 /* Line discipline .open() */
 static int cx81801_open(struct tty_struct *tty)
 {
@@ -341,31 +296,11 @@ static void cx81801_receive(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 {
 	struct snd_soc_component *component = tty->disc_data;
 	const unsigned char *c;
-	int apply, ret;
+	int apply;
 
 	if (!component)
 		return;
 
-	if (!component->card->pop_time) {
-		/* First modem response, complete setup procedure */
-
-		/* Initialize timer used for config pulse generation */
-		timer_setup(&cx81801_timer, cx81801_timeout, 0);
-
-		v253_ops.receive_buf(tty, cp, fp, count);
-
-		/* Link hook switch to DAPM pins */
-		ret = snd_soc_jack_add_pins(&ams_delta_hook_switch,
-					ARRAY_SIZE(ams_delta_hook_switch_pins),
-					ams_delta_hook_switch_pins);
-		if (ret)
-			dev_warn(component->dev,
-				"Failed to link hook switch to DAPM pins, "
-				"will continue with hook switch unlinked.\n");
-
-		return;
-	}
-
 	v253_ops.receive_buf(tty, cp, fp, count);
 
 	for (c = &cp[count - 1]; c >= cp; c--) {
-- 
2.43.0

[RFC][PATCH 2/3] ASoC: cx20442: don't use card->pop_time

[Kuninori Morimoto]

Very few user uses card->pop_time and pop/click stuff is much less of a
problem. Let's don't use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/codecs/cx20442.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/codecs/cx20442.c b/sound/soc/codecs/cx20442.c
index c0dc69ce4d0e7..daf3235eb9337 100644
--- a/sound/soc/codecs/cx20442.c
+++ b/sound/soc/codecs/cx20442.c
@@ -248,7 +248,6 @@ static void v253_close(struct tty_struct *tty)
 
 	/* Prevent the codec driver from further accessing the modem */
 	cx20442->tty = NULL;
-	component->card->pop_time = 0;
 }
 
 /* Line discipline .hangup() */
@@ -274,7 +273,6 @@ static void v253_receive(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 
 		/* Set up codec driver access to modem controls */
 		cx20442->tty = tty;
-		component->card->pop_time = 1;
 	}
 }
 
@@ -375,7 +373,6 @@ static int cx20442_component_probe(struct snd_soc_component *component)
 	cx20442->tty = NULL;
 
 	snd_soc_component_set_drvdata(component, cx20442);
-	component->card->pop_time = 0;
 
 	return 0;
 }
-- 
2.43.0

[RFC][PATCH 3/3] ASoC: soc-card: remvoe card->pop_time

[Kuninori Morimoto]

card->pop_time is mainly used at soc-dapm.c, and actually used was only
2 parts.

	static void dapm_seq_run_coalesced(...)
	{
		...
		if (reg >= 0) {
			...
=>			dapm_pop_wait(card->pop_time);
			...
		}
		...
	}

	static int dapm_power_widgets(...)
	{
		...
=>		dapm_pop_wait(card->pop_time);
		...
	}

No one is using it, Let's remove.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |  2 --
 sound/soc/soc-core.c |  3 ---
 sound/soc/soc-dapm.c | 43 -------------------------------------------
 3 files changed, 48 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 5e3eb617d8323..453548988d381 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1081,8 +1081,6 @@ struct snd_soc_card {
 #ifdef CONFIG_PM_SLEEP
 	struct work_struct deferred_resume_work;
 #endif
-	u32 pop_time;
-
 	/* bit field */
 	unsigned int instantiated:1;
 	unsigned int topology_shortname_created:1;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 3fecf9fc903c0..d72cd66cdcf94 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -194,9 +194,6 @@ static void soc_init_card_debugfs(struct snd_soc_card *card)
 	card->debugfs_card_root = debugfs_create_dir(card->name,
 						     snd_soc_debugfs_root);
 
-	debugfs_create_u32("dapm_pop_time", 0644, card->debugfs_card_root,
-			   &card->pop_time);
-
 	snd_soc_dapm_debugfs_init(snd_soc_card_to_dapm(card), card->debugfs_card_root);
 }
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index d6192204e613d..9b86ba284d038 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -161,33 +161,6 @@ static void dapm_assert_locked(struct snd_soc_dapm_context *dapm)
 		snd_soc_dapm_mutex_assert_held(dapm);
 }
 
-static void dapm_pop_wait(u32 pop_time)
-{
-	if (pop_time)
-		schedule_timeout_uninterruptible(msecs_to_jiffies(pop_time));
-}
-
-__printf(3, 4)
-static void dapm_pop_dbg(struct device *dev, u32 pop_time, const char *fmt, ...)
-{
-	va_list args;
-	char *buf;
-
-	if (!pop_time)
-		return;
-
-	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (buf == NULL)
-		return;
-
-	va_start(args, fmt);
-	vsnprintf(buf, PAGE_SIZE, fmt, args);
-	dev_info(dev, "%s", buf);
-	va_end(args);
-
-	kfree(buf);
-}
-
 struct snd_soc_dapm_context *snd_soc_dapm_alloc(struct device *dev)
 {
 	return devm_kzalloc(dev, sizeof(struct snd_soc_dapm_context), GFP_KERNEL);
@@ -1872,8 +1845,6 @@ static void dapm_seq_check_event(struct snd_soc_card *card,
 	if (w->event && (w->event_flags & event)) {
 		int ret;
 
-		dapm_pop_dbg(dev, card->pop_time, "pop test : %s %s\n",
-			w->name, ev_name);
 		dapm_async_complete(w->dapm);
 		trace_snd_soc_dapm_widget_event_start(w, event);
 		ret = w->event(w, NULL, event);
@@ -1888,7 +1859,6 @@ static void dapm_seq_check_event(struct snd_soc_card *card,
 static void dapm_seq_run_coalesced(struct snd_soc_card *card,
 				   struct list_head *pending)
 {
-	struct device *dev = card->dev;
 	struct snd_soc_dapm_context *dapm;
 	struct snd_soc_dapm_widget *w;
 	int reg;
@@ -1909,10 +1879,6 @@ static void dapm_seq_run_coalesced(struct snd_soc_card *card,
 		else
 			value |= w->off_val << w->shift;
 
-		dapm_pop_dbg(dev, card->pop_time,
-			"pop test : Queue %s: reg=0x%x, 0x%x/0x%x\n",
-			w->name, reg, value, mask);
-
 		/* Check for events */
 		dapm_seq_check_event(card, w, SND_SOC_DAPM_PRE_PMU);
 		dapm_seq_check_event(card, w, SND_SOC_DAPM_PRE_PMD);
@@ -1922,11 +1888,6 @@ static void dapm_seq_run_coalesced(struct snd_soc_card *card,
 		/* Any widget will do, they should all be updating the
 		 * same register.
 		 */
-
-		dapm_pop_dbg(dev, card->pop_time,
-			"pop test : Applying 0x%x/0x%x to %x in %dms\n",
-			value, mask, reg, card->pop_time);
-		dapm_pop_wait(card->pop_time);
 		dapm_update_bits(dapm, reg, mask, value);
 	}
 
@@ -2392,10 +2353,6 @@ static int dapm_power_widgets(struct snd_soc_card *card, int event,
 			return ret;
 	}
 
-	dapm_pop_dbg(card->dev, card->pop_time,
-		"DAPM sequencing finished, waiting %dms\n", card->pop_time);
-	dapm_pop_wait(card->pop_time);
-
 	trace_snd_soc_dapm_done(card, event);
 
 	return 0;
-- 
2.43.0

Re: [RFC][PATCH 0/3] ASoC: remove card->pop_time

[Takashi Iwai]

On Thu, 09 Apr 2026 06:36:48 +0200,
Kuninori Morimoto wrote:
> 
> Hi Mark, OMAP member
> 
> I'm now trying to cleanup snd_soc_card, and noticed that card->pop_time is used
> only from OMAP1 board-ams-delta.
> 
> card->pop_time user drivers are...
> 
> 	> git grep -w -l pop_time sound/soc include/sound
> 	include/sound/soc.h
> (A)	sound/soc/codecs/cx20442.c
> 	sound/soc/soc-card.c
> 	sound/soc/soc-dapm.c
> (B)	sound/soc/ti/ams-delta.c
> 
> Except ASoC framework, pop_time user is only cx20442 (A) / ams-delta (B).
> And board-ams-delta (Z) is using it.
> 
> (Z)	linux/arch/arm/mach-omap1/board-ams-delta.c
> 
> 	static struct platform_device ams_delta_audio_device = {
> (A)		.name   = "ams-delta-audio",
> 	};
> 
> 	static struct platform_device cx20442_codec_device = {
> (B)		.name   = "cx20442-codec",
> 	};
> 
> 	static struct platform_device *ams_delta_devices[] __initdata = {
> 		...
> (A)		&ams_delta_audio_device,
> 		...
> (B)		&cx20442_codec_device,
> 		...
> 	};
> 
> With off-list talk with Mark, this card->pop_time is is much less of a
> problem.

Hmm, are you sure?  IIUC, it's used for a flag in the modem desc for
cx81801, and a bunch of setup seems depending on it, and your second
patch removes the whole of those without alternative.

> So let's remove it. But it needs Acked-by from OMAP1 member.
> I added [RFC] on Subject.

So the actual question is whether the modem is utterly broken for this
hardware or not.  If yes, it'd be a good reason to drop and apply this
cleanup.

But if it's not, we'd need an alternative implementation, e.g. move
the flag and its handling to the modem's own object instead of the
ASoC core, instead.


thanks,

Takashi

Re: [RFC][PATCH 0/3] ASoC: remove card->pop_time

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

On Thu, Apr 09, 2026 at 07:22:22AM +0200, Takashi Iwai wrote:
> Kuninori Morimoto wrote:

> > With off-list talk with Mark, this card->pop_time is is much less of a
> > problem.

> Hmm, are you sure?  IIUC, it's used for a flag in the modem desc for
> cx81801, and a bunch of setup seems depending on it, and your second
> patch removes the whole of those without alternative.

> > So let's remove it. But it needs Acked-by from OMAP1 member.
> > I added [RFC] on Subject.

> So the actual question is whether the modem is utterly broken for this
> hardware or not.  If yes, it'd be a good reason to drop and apply this
> cleanup.

> But if it's not, we'd need an alternative implementation, e.g. move
> the flag and its handling to the modem's own object instead of the
> ASoC core, instead.

Yeah, we need to figure out if there's any actual users of this still
and if so how are they using it.  It looks like you need some custom
userspace to drive them model which is fun...  There's also some
more references to the timer which need to be cleaned up/updated if the
setup is removed.

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

Re: [RFC][PATCH 0/3] ASoC: remove card->pop_time

[Péter Ujfalusi]

On 09/04/2026 07:36, Kuninori Morimoto wrote:
> Hi Mark, OMAP member
> 
> I'm now trying to cleanup snd_soc_card, and noticed that card->pop_time is used
> only from OMAP1 board-ams-delta.

The pop_time is an amazingly good tool to debug pop noise and is mostly
used via the debugfs.
You set the pop_time to non 0 and DAPM will 'slow down' and you can
identify the step which produces the pop.

Please, don't remove it!

> 
> card->pop_time user drivers are...
> 
> 	> git grep -w -l pop_time sound/soc include/sound
> 	include/sound/soc.h
> (A)	sound/soc/codecs/cx20442.c
> 	sound/soc/soc-card.c
> 	sound/soc/soc-dapm.c
> (B)	sound/soc/ti/ams-delta.c
> 
> Except ASoC framework, pop_time user is only cx20442 (A) / ams-delta (B).
> And board-ams-delta (Z) is using it.
> 
> (Z)	linux/arch/arm/mach-omap1/board-ams-delta.c
> 
> 	static struct platform_device ams_delta_audio_device = {
> (A)		.name   = "ams-delta-audio",
> 	};
> 
> 	static struct platform_device cx20442_codec_device = {
> (B)		.name   = "cx20442-codec",
> 	};
> 
> 	static struct platform_device *ams_delta_devices[] __initdata = {
> 		...
> (A)		&ams_delta_audio_device,
> 		...
> (B)		&cx20442_codec_device,
> 		...
> 	};
> 
> With off-list talk with Mark, this card->pop_time is is much less of a
> problem. So let's remove it. But it needs Acked-by from OMAP1 member.
> I added [RFC] on Subject.
> 
> Kuninori Morimoto (3):
>   ASoC: ti: amx-delta: don't use card->pop_time
>   ASoC: cx20442: don't use card->pop_time
>   ASoC: soc-card: remvoe card->pop_time
> 
>  include/sound/soc.h        |  2 --
>  sound/soc/codecs/cx20442.c |  3 --
>  sound/soc/soc-core.c       |  3 --
>  sound/soc/soc-dapm.c       | 43 ------------------------
>  sound/soc/ti/ams-delta.c   | 67 +-------------------------------------
>  5 files changed, 1 insertion(+), 117 deletions(-)
> 

-- 
Péter

Re: [RFC][PATCH 0/3] ASoC: remove card->pop_time

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

On Thu, Apr 09, 2026 at 11:14:08AM +0300, Péter Ujfalusi wrote:
> On 09/04/2026 07:36, Kuninori Morimoto wrote:

> > I'm now trying to cleanup snd_soc_card, and noticed that card->pop_time is used
> > only from OMAP1 board-ams-delta.

> The pop_time is an amazingly good tool to debug pop noise and is mostly
> used via the debugfs.
> You set the pop_time to non 0 and DAPM will 'slow down' and you can
> identify the step which produces the pop.

> Please, don't remove it!

For Morimoto-san's purposes I think it's sufficient to just make it not
card specific so we could just have a global control in debugfs that
applies to all cards instead.  That's likely sufficient for debugging
use?

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

Re: [RFC][PATCH 0/3] ASoC: remove card->pop_time

[Kuninori Morimoto]

Hi Péter, Mark

Thank you for your review

> > The pop_time is an amazingly good tool to debug pop noise and is mostly
> > used via the debugfs.
> > You set the pop_time to non 0 and DAPM will 'slow down' and you can
> > identify the step which produces the pop.

Ah... OK, now I could understand the usage.

> Please, don't remove it!

OK. Thank you for clarify it !

> For Morimoto-san's purposes I think it's sufficient to just make it not
> card specific so we could just have a global control in debugfs that
> applies to all cards instead.  That's likely sufficient for debugging
> use?

Biggest my motivation is I want to capsuling snd_soc_card.
If we can move it to some where, for example dapm static / local, etc
(It seems it was before), it is very nice.

If OMAP1 was OK, maybe we can move pop_time from card to soc-dapm.c ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [RFC][PATCH 0/3] ASoC: remove card->pop_time

[Péter Ujfalusi]

On 09/04/2026 22:11, Mark Brown wrote:
> On Thu, Apr 09, 2026 at 11:14:08AM +0300, Péter Ujfalusi wrote:
>> On 09/04/2026 07:36, Kuninori Morimoto wrote:
> 
>>> I'm now trying to cleanup snd_soc_card, and noticed that card->pop_time is used
>>> only from OMAP1 board-ams-delta.
> 
>> The pop_time is an amazingly good tool to debug pop noise and is mostly
>> used via the debugfs.
>> You set the pop_time to non 0 and DAPM will 'slow down' and you can
>> identify the step which produces the pop.
> 
>> Please, don't remove it!
> 
> For Morimoto-san's purposes I think it's sufficient to just make it not
> card specific so we could just have a global control in debugfs that
> applies to all cards instead.  That's likely sufficient for debugging
> use?

and I think at least the amx-delta is using the pop_time incorrectly, it
is using it as a handshake flag between the codec driver and the machine
driver and if I look back it started to to it with

fac3f5e20dce ("ASoC: don't use codec hw_write on cx20442/omap-ams-delta")
which changed it from codec->hw_write to the pop_time

f0fba2ad1b6b ("ASoC: multi-component - ASoC Multi-Component Support")
changed the original codec->control_data to codec->hw_write.

Yes, the global pop_time (card level?) is fine for debugging and the
cx20442/amx-delta needs proper handshake implementation.

-- 
Péter