[PATCH] ASoC: soc-pcm: add the judgment of unsupported commands for the function soc_pcm_trigger

[PATCH] ASoC: soc-pcm: add the judgment of unsupported commands for the function soc_pcm_trigger

[xuhanwu]

From: xuhanwu <xu.hanwu@zxelec.com>

Function soc_pcm_trigger, if the parameter cmd does not support the need to return an error (-EINVAL).

Signed-off-by: xuhanwu <xu.hanwu@zxelec.com>
---
 sound/soc/soc-pcm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 77ee103b7..eba737729 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1173,6 +1173,9 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 			if (r < 0)
 				break;
 		}
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	/*
-- 
2.17.1

Re: [PATCH] ASoC: soc-pcm: add the judgment of unsupported commands for the function soc_pcm_trigger

[Mark Brown]

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

On Fri, Mar 08, 2024 at 06:07:06PM +0800, xuhanwu wrote:

> Function soc_pcm_trigger, if the parameter cmd does not support the need to return an error (-EINVAL).

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> @@ -1173,6 +1173,9 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
>  			if (r < 0)
>  				break;
>  		}
> +		break;
> +	default:
> +		return -EINVAL;
>  	}

As can be seen from inspection of the immediately preceeding handling
of start, we're deliberately ignoring half the values in each switch.

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

[PATCH] ASoC: soc-pcm: add a check for unsupported commands to the soc_pcm_trigger

[xuhanwu]

From: xuhanwu <xu.hanwu@zxelec.com>

The function soc_pcm_trigger needs to return an error(-EINVAL)
if the cmd parameter is not supported.

Signed-off-by: xuhanwu <xu.hanwu@zxelec.com>
---
Dear Brown

Thank you for your guidance.
The issue with characters exceeding 80 has been resolved.

>	As can be seen from inspection of the immediately
>	preceeding handling of start, we're deliberately
>        ignoring half the values in each switch.

In kernel version 6.2, when the soc_pcm_trigger function receives a
command parameter cmd set to SNDRV_PCM_TRIGGER_DRAIN,
it returns a value of -EINVAL.The function snd_pcm_drain checks
the returned error value and exits accordingly.

kernel-version6.2 Function call relationship
		  snd_pcm_drain->
			snd_pcm_action->
				snd_pcm_do_drain_init->
					    soc_pcm_trigger->

    snd_pcm_drain
	result = snd_pcm_action(&snd_pcm_action_drain_init, substream,
				ACTION_ARG_IGNORE);
	if (result < 0)
		goto unlock;

     snd_pcm_do_drain_init
	if (runtime->state == SNDRV_PCM_STATE_DRAINING &&
	    runtime->trigger_master == substream &&
	    (runtime->hw.info & SNDRV_PCM_INFO_DRAIN_TRIGGER))
		return substream->ops->trigger(substream,
					       SNDRV_PCM_TRIGGER_DRAIN);

In the latest code, when the cmd parameter is set to SNDRV_PCM_TRIGGER_DRAIN,
the return value is 0. If snd_pcm_drain finds that the return value is 0,
it will not execute the goto unlock;

The above is my understanding of the code, please help me correct it.

Thanks
Xuhanwu
---
 sound/soc/soc-pcm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 77ee103b7..eba737729 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1173,6 +1173,9 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 			if (r < 0)
 				break;
 		}
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	/*
-- 
2.17.1

Re: [PATCH] ASoC: soc-pcm: add a check for unsupported commands to the soc_pcm_trigger

[Mark Brown]

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

On Mon, Mar 11, 2024 at 03:05:48PM +0800, xuhanwu wrote:

> >	As can be seen from inspection of the immediately
> >	preceeding handling of start, we're deliberately
> >        ignoring half the values in each switch.

> In kernel version 6.2, when the soc_pcm_trigger function receives a
> command parameter cmd set to SNDRV_PCM_TRIGGER_DRAIN,
> it returns a value of -EINVAL.The function snd_pcm_drain checks
> the returned error value and exits accordingly.

The current kernel version is v6.8 (we're in the merge window for v6.9
now) - any changes that you're submitting should be submitted against
current code, not v6.2.

> In the latest code, when the cmd parameter is set to SNDRV_PCM_TRIGGER_DRAIN,
> the return value is 0. If snd_pcm_drain finds that the return value is 0,
> it will not execute the goto unlock;

Well, it will get to the unlock eventually after going through the for
loop so the lock doesn't get leaked AFAICT?  I'm not sure what the issue
you're seeing here is.

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

[PATCH] ASoC: soc-pcm: add a check for unsupported commands to the soc_pcm_trigger

[xuhanwu]

Dear broonie

Issue: Before and after Linux version 6.2, when the cmd is 
SNDRV_PCM_TRIGGER_DRAIN, calling the function soc_pcm_trigger results 
in different return values.
Through your guidance, I've realized that the code logic has been adjusted. 
I will further deepen my study of the code. Thank you for your guidance.

Thank you for your help.
xuhanwu