| Message ID | 20191105143218.5948-4-andrew_gabbasov@mentor.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: aloop: Support sound timer as clock source instead of jiffies | expand |
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
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
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;