Re: [Sound-open-firmware] [PATCH v5 05/14] ASoC: SOF: Add PCM operations support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/9/19 10:48 AM, Takashi Iwai wrote:
On Tue, 09 Apr 2019 16:23:17 +0200,
Pierre-Louis Bossart wrote:


ok, thanks for confirming. we'll remove the INFO_RESUME flag in SOF
and follow-up with the removal on all other Intel drivers. Thanks
for enlightening us on this.

Actually one more question related to the documentation, which reads

"Note that the trigger with SUSPEND can always be called when
snd_pcm_suspend_all() is called, regardless of the
SNDRV_PCM_INFO_RESUME flag. The RESUME flag affects only the behavior
of snd_pcm_resume(). (Thus, in theory, SNDRV_PCM_TRIGGER_RESUME isn’t
needed to be handled in the trigger callback when no
SNDRV_PCM_INFO_RESUME flag is set. But, it’s better to keep it for
compatibility reasons.)"

I could not figure out what the last sentence means. It's my
understanding that the resume_trigger will never be called with the
code flow below when INFO_RESUME isn't declared. Would you mind
clarifying what this compatibility might be? Thanks!

Well, in the above "better to keep it" text -- here "it" was meant as
SNDRV_PCM_TRIGGER_RESUME case handling in the trigger callback, not as
SNDRV_PCM_INFO_RESUME flag.  That is, the above recommends a trigger
callback like below would keep SNDRV_PCM_TRIGGER_RESUME although it
won't be called practically:

That's the part that I find odd. we keep the TRIGGER_RESUME but it will never be called, that's an unreachable/untestable switch case, no? Or we should trap it as an error case.

static int foo_trigger(....)
{
	switch (cmd) {
	case SNDRV_PCM_TRIGGER_START:
	case SNDRV_PCM_TRIGGER_RESUME:
		do_start();
		break;
	case SNDRV_PCM_TRIGGER_STOP:
	case SNDRV_PCM_TRIGGER_SUSPEND:
		do_stop();
		break;
	....
	}
	....
}


This text needs clearly a better rephrasing...


Takashi

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux