[v2,3/8] ALSA: aloop: loopback_timer_stop: Support return of error code
diff mbox series

Message ID 20191105143218.5948-4-andrew_gabbasov@mentor.com
State New
Headers show
Series
  • ALSA: aloop: Support sound timer as clock source instead of jiffies
Related show

Commit Message

Andrew Gabbasov Nov. 5, 2019, 2:32 p.m. UTC
From: Timo Wischer <twischer@de.adit-jv.com>

This is required for additional timer implementations which could detect
errors and want to throw them.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 sound/drivers/aloop.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Takashi Iwai Nov. 6, 2019, 3:51 p.m. UTC | #1
On Tue, 05 Nov 2019 15:32:13 +0100,
Andrew Gabbasov wrote:
> 
> From: Timo Wischer <twischer@de.adit-jv.com>
> 
> This is required for additional timer implementations which could detect
> errors and want to throw them.
> 
> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>

I'd fold this into the patch 2.  Both patches do fairly same things
but just for start and stop.

And the same question as patch#2 is applied to this one, too, BTW.


thanks,

Takashi
Andrew Gabbasov Nov. 6, 2019, 5:45 p.m. UTC | #2
Thank you very much for your response.

> From: Takashi Iwai <tiwai@suse.de>
> Sent: Wednesday, November 6, 2019 18:51
> 
> On Tue, 05 Nov 2019 15:32:13 +0100,
> Andrew Gabbasov wrote:
> >
> > From: Timo Wischer <twischer@de.adit-jv.com>
> >
> > This is required for additional timer implementations which could detect
> > errors and want to throw them.
> >
> > Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
> > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> 
> I'd fold this into the patch 2.  Both patches do fairly same things
> but just for start and stop.

OK, I agree. I'll squash these 2 commits into a single one in the next
update (there will be an update for sure to fix the snd_cards reference
in patch #7).

> 
> And the same question as patch#2 is applied to this one, too, BTW.

As for the notifications in case of timer operation failures:

For stop/suspend operations, the return code of the timer operation,
and of the PCM trigger function as a whole, actually makes no difference,
the streams state is changed anyway, so the notification should be done
in any case.

For start/resume operations, it seems OK to send notifications
even if the timer operation fails, if the cable->running and cable->pause
fields are set before that (as is now), so that the notified control
reflects the changed state. In case of failure the whole operation
will be un-done by upper PCM layer, changing the state back,
and sending one more notifcation.

Alternatively, we could re-order the code and do not set the running
fields if timer operation fails (and do not notify in this case).
But the undoing stop operation will be executed in this case
that will cause the (unpaired) notification, which is probably
not quite correct.

Thanks.

Best regards,
Andrew

Patch
diff mbox series

diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index b9ee5b72a996..dc9154662d5b 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -176,15 +176,19 @@  static int loopback_timer_start(struct loopback_pcm *dpcm)
 }
 
 /* call in cable->lock */
-static inline void loopback_timer_stop(struct loopback_pcm *dpcm)
+static inline int loopback_timer_stop(struct loopback_pcm *dpcm)
 {
 	del_timer(&dpcm->timer);
 	dpcm->timer.expires = 0;
+
+	return 0;
 }
 
-static inline void loopback_timer_stop_sync(struct loopback_pcm *dpcm)
+static inline int loopback_timer_stop_sync(struct loopback_pcm *dpcm)
 {
 	del_timer_sync(&dpcm->timer);
+
+	return 0;
 }
 
 #define CABLE_VALID_PLAYBACK	(1 << SNDRV_PCM_STREAM_PLAYBACK)
@@ -275,7 +279,7 @@  static int loopback_trigger(struct snd_pcm_substream *substream, int cmd)
 		spin_lock(&cable->lock);	
 		cable->running &= ~stream;
 		cable->pause &= ~stream;
-		loopback_timer_stop(dpcm);
+		err = loopback_timer_stop(dpcm);
 		spin_unlock(&cable->lock);
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 			loopback_active_notify(dpcm);
@@ -284,7 +288,7 @@  static int loopback_trigger(struct snd_pcm_substream *substream, int cmd)
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 		spin_lock(&cable->lock);	
 		cable->pause |= stream;
-		loopback_timer_stop(dpcm);
+		err = loopback_timer_stop(dpcm);
 		spin_unlock(&cable->lock);
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 			loopback_active_notify(dpcm);
@@ -323,9 +327,11 @@  static int loopback_prepare(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct loopback_pcm *dpcm = runtime->private_data;
 	struct loopback_cable *cable = dpcm->cable;
-	int bps, salign;
+	int err, bps, salign;
 
-	loopback_timer_stop_sync(dpcm);
+	err = loopback_timer_stop_sync(dpcm);
+	if (err < 0)
+		return err;
 
 	salign = (snd_pcm_format_physical_width(runtime->format) *
 						runtime->channels) / 8;