All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] ALSA: pcmtest: Add a volume control with event fault injection
@ 2026-11-03  1:10 HyeongJun An
  2026-11-03  1:16 ` sashiko-bot
  2026-11-03 16:49 ` Mark Brown
  0 siblings, 2 replies; 3+ messages in thread
From: HyeongJun An @ 2026-11-03  1:10 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, Ivan Orlov, linux-sound, linux-kernel,
	HyeongJun An, Mark Brown

The mixer-test selftest checks that a control sends an event when its
value changes, and only then. No virtual driver has a control that
breaks this rule, so without broken hardware nothing shows that these
checks work.

Add a stereo volume control to pcmtest, and two parameters that make
its put callback report the wrong result. With inject_ctl_no_event it
reports no change, and with inject_ctl_spurious_event it reports a
change every time.

With both off, mixer-test passes on the control. With either one on,
only the matching event check fails.

Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
This is an RFC. I would like to ask whether this is wanted before I go
further.

mixer-test runs eight checks on each control. To see one of them fail
today, you need hardware with the bug or a driver broken by hand. I
broke snd-dummy by hand to test the side effect check. This patch does
the same for the two event checks, with module parameters.

Two questions:

1. Is this useful enough to cover the other six checks too?

2. Should it live in pcmtest, or in a separate driver? pcmtest already
   has the inject_* parameters and a selftest, so it seemed the natural
   place.

If this goes ahead, the next version will also document the new
parameters in Documentation/sound/cards/pcmtest.rst.

mixer-test results for the new control:

  no fault                    all 8 checks pass
  inject_ctl_no_event=1       only event_missing fails
  inject_ctl_spurious_event=1 only event_spurious fails

The pcmtest selftest still passes all 10 tests.

Prepared with Claude Code (Claude Opus 5.5). Tested on x86_64 with the
sound/for-next driver, built for a 6.17 host.

 sound/drivers/pcmtest.c | 68 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/sound/drivers/pcmtest.c b/sound/drivers/pcmtest.c
index 66bee8a2ce34..c95bb4717487 100644
--- a/sound/drivers/pcmtest.c
+++ b/sound/drivers/pcmtest.c
@@ -34,6 +34,7 @@
 #include <linux/init.h>
 #include <sound/pcm.h>
 #include <sound/core.h>
+#include <sound/control.h>
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
@@ -57,6 +58,8 @@
 
 #define MAX_PATTERN_LEN 4096
 
+#define VOLUME_MAX	100
+
 static int index = -1;
 static char *id = "pcmtest";
 static bool enable = true;
@@ -65,6 +68,8 @@ static bool inject_hwpars_err;
 static bool inject_prepare_err;
 static bool inject_trigger_err;
 static bool inject_open_err;
+static bool inject_ctl_no_event;
+static bool inject_ctl_spurious_event;
 
 static short fill_mode = FILL_MODE_PAT;
 
@@ -109,11 +114,17 @@ module_param(inject_trigger_err, bool, 0600);
 MODULE_PARM_DESC(inject_trigger_err, "Inject EINVAL error in the 'trigger' callback");
 module_param(inject_open_err, bool, 0600);
 MODULE_PARM_DESC(inject_open_err, "Inject EBUSY error in the 'open' callback");
+module_param(inject_ctl_no_event, bool, 0600);
+MODULE_PARM_DESC(inject_ctl_no_event, "Report no change from the control 'put' callback");
+module_param(inject_ctl_spurious_event, bool, 0600);
+MODULE_PARM_DESC(inject_ctl_spurious_event, "Report a change from the control 'put' callback");
 
 struct pcmtst {
 	struct snd_pcm *pcm;
 	struct snd_card *card;
 	struct platform_device *pdev;
+	spinlock_t mixer_lock;
+	long volume[2];
 };
 
 struct pcmtst_buf_iter {
@@ -544,6 +555,58 @@ static const struct snd_pcm_ops snd_pcmtst_capture_ops = {
 	.pointer =	snd_pcmtst_pcm_pointer,
 };
 
+static int snd_pcmtst_volume_info(struct snd_kcontrol *kcontrol,
+				  struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 2;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = VOLUME_MAX;
+	return 0;
+}
+
+static int snd_pcmtst_volume_get(struct snd_kcontrol *kcontrol,
+				 struct snd_ctl_elem_value *ucontrol)
+{
+	struct pcmtst *pcmtst = snd_kcontrol_chip(kcontrol);
+
+	guard(spinlock_irq)(&pcmtst->mixer_lock);
+	ucontrol->value.integer.value[0] = pcmtst->volume[0];
+	ucontrol->value.integer.value[1] = pcmtst->volume[1];
+	return 0;
+}
+
+static int snd_pcmtst_volume_put(struct snd_kcontrol *kcontrol,
+				 struct snd_ctl_elem_value *ucontrol)
+{
+	struct pcmtst *pcmtst = snd_kcontrol_chip(kcontrol);
+	long left = ucontrol->value.integer.value[0];
+	long right = ucontrol->value.integer.value[1];
+	bool changed;
+
+	if (left < 0 || left > VOLUME_MAX || right < 0 || right > VOLUME_MAX)
+		return -EINVAL;
+
+	guard(spinlock_irq)(&pcmtst->mixer_lock);
+	changed = pcmtst->volume[0] != left || pcmtst->volume[1] != right;
+	pcmtst->volume[0] = left;
+	pcmtst->volume[1] = right;
+
+	if (inject_ctl_no_event)
+		return 0;
+	if (inject_ctl_spurious_event)
+		return 1;
+	return changed;
+}
+
+static const struct snd_kcontrol_new snd_pcmtst_volume = {
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+	.name = "PCM Playback Volume",
+	.info = snd_pcmtst_volume_info,
+	.get = snd_pcmtst_volume_get,
+	.put = snd_pcmtst_volume_put,
+};
+
 static int snd_pcmtst_new_pcm(struct pcmtst *pcmtst)
 {
 	struct snd_pcm *pcm;
@@ -582,11 +645,16 @@ static int pcmtst_probe(struct platform_device *pdev)
 	pcmtst = card->private_data;
 	pcmtst->card = card;
 	pcmtst->pdev = pdev;
+	spin_lock_init(&pcmtst->mixer_lock);
 
 	err = snd_pcmtst_new_pcm(pcmtst);
 	if (err < 0)
 		return err;
 
+	err = snd_ctl_add(card, snd_ctl_new1(&snd_pcmtst_volume, pcmtst));
+	if (err < 0)
+		return err;
+
 	strscpy(card->driver, "PCM-TEST Driver");
 	strscpy(card->shortname, "PCM-Test");
 	strscpy(card->longname, "PCM-Test virtual driver");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] ALSA: pcmtest: Add a volume control with event fault injection
  2026-11-03  1:10 [RFC PATCH] ALSA: pcmtest: Add a volume control with event fault injection HyeongJun An
@ 2026-11-03  1:16 ` sashiko-bot
  2026-11-03 16:49 ` Mark Brown
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-03  1:16 UTC (permalink / raw)
  To: HyeongJun An; +Cc: linux-sound

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] ALSA: pcmtest: missing documentation for new control fault module parameters
--

commit 37a7a575ca0b14787e4948616e5cc636a185dad2
Author: HyeongJun An <sammiee5311@gmail.com>

ALSA: pcmtest: Add a volume control with event fault injection

This commit adds a stereo playback volume mixer control to the virtual pcmtest
driver along with two module parameters, inject_ctl_no_event and
inject_ctl_spurious_event, to inject event reporting faults in
snd_pcmtst_volume_put().

> diff --git a/sound/drivers/pcmtest.c b/sound/drivers/pcmtest.c
> index 66bee8a2ce343..c95bb47174871 100644
> --- a/sound/drivers/pcmtest.c
> +++ b/sound/drivers/pcmtest.c

[ ... ]

> @@ -109,11 +114,17 @@ module_param(inject_trigger_err, bool, 0600);
>  MODULE_PARM_DESC(inject_trigger_err, "Inject EINVAL error in the 'trigger' callback");
>  module_param(inject_open_err, bool, 0600);
>  MODULE_PARM_DESC(inject_open_err, "Inject EBUSY error in the 'open' callback");
> +module_param(inject_ctl_no_event, bool, 0600);
> +MODULE_PARM_DESC(inject_ctl_no_event, "Report no change from the control 'put' callback");
> +module_param(inject_ctl_spurious_event, bool, 0600);
> +MODULE_PARM_DESC(inject_ctl_spurious_event, "Report a change from the control 'put' callback");

[Severity: Low]
Since inject_ctl_no_event and inject_ctl_spurious_event are registered via
module_param() in sound/drivers/pcmtest.c and can be passed on the kernel
command line at boot (as snd_pcmtest.inject_ctl_no_event= and
snd_pcmtest.inject_ctl_spurious_event=) or set via sysfs, should these new
parameters be documented in Documentation/admin-guide/kernel-parameters.txt
and under the Configuration and Errors injection sections of
Documentation/sound/cards/pcmtest.rst in this patch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103011000.1012035-1-sammiee5311@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] ALSA: pcmtest: Add a volume control with event fault injection
  2026-11-03  1:10 [RFC PATCH] ALSA: pcmtest: Add a volume control with event fault injection HyeongJun An
  2026-11-03  1:16 ` sashiko-bot
@ 2026-11-03 16:49 ` Mark Brown
  1 sibling, 0 replies; 3+ messages in thread
From: Mark Brown @ 2026-11-03 16:49 UTC (permalink / raw)
  To: HyeongJun An
  Cc: Takashi Iwai, Jaroslav Kysela, Ivan Orlov, linux-sound,
	linux-kernel

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

On Tue, Nov 03, 2026 at 10:10:00AM +0900, HyeongJun An wrote:

> mixer-test runs eight checks on each control. To see one of them fail
> today, you need hardware with the bug or a driver broken by hand. I
> broke snd-dummy by hand to test the side effect check. This patch does
> the same for the two event checks, with module parameters.

> Two questions:

> 1. Is this useful enough to cover the other six checks too?

> 2. Should it live in pcmtest, or in a separate driver? pcmtest already
>    has the inject_* parameters and a selftest, so it seemed the natural
>    place.

It seems reasonable enough to me FWIW.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-03 16:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03  1:10 [RFC PATCH] ALSA: pcmtest: Add a volume control with event fault injection HyeongJun An
2026-11-03  1:16 ` sashiko-bot
2026-11-03 16:49 ` Mark Brown

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.