Hi Pierre,
On 8/30/2024 2:45 AM, Pierre-Louis Bossart wrote:
>
> On 8/29/24 21:40, Wesley Cheng wrote:
>> With USB audio offloading, an audio session is started from the ASoC
>> platform sound card and PCM devices. Likewise, the USB SND path is still
>> readily available for use, in case the non-offload path is desired. In
>> order to prevent the two entities from attempting to use the USB bus,
>> introduce a flag that determines when either paths are in use.
>>
>> If a PCM device is already in use, the check will return an error to
>> userspace notifying that the stream is currently busy. This ensures that
>> only one path is using the USB substream.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>
> I would also move this patch earlier in the series since it has no
> dependency on USB offload really, and if somehow it breaks USB audio
> regular paths we'd want to know early for bisection.
Sure I'll re-order this earlier since I'm going to send out another rev.
Thanks
Wesley Cheng
>
>> ---
>> sound/usb/card.h | 1 +
>> sound/usb/pcm.c | 29 ++++++++++++++++++++++++++---
>> 2 files changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/sound/usb/card.h b/sound/usb/card.h
>> index 15cda1730076..d8b8522e1613 100644
>> --- a/sound/usb/card.h
>> +++ b/sound/usb/card.h
>> @@ -165,6 +165,7 @@ struct snd_usb_substream {
>> unsigned int pkt_offset_adj; /* Bytes to drop from beginning of packets (for non-compliant devices) */
>> unsigned int stream_offset_adj; /* Bytes to drop from beginning of stream (for non-compliant devices) */
>>
>> + unsigned int opened:1; /* pcm device opened */
>> unsigned int running: 1; /* running status */
>> unsigned int period_elapsed_pending; /* delay period handling */
>>
>> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
>> index 18467da6fd9e..b24ee38fad72 100644
>> --- a/sound/usb/pcm.c
>> +++ b/sound/usb/pcm.c
>> @@ -1241,8 +1241,17 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>> struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
>> struct snd_pcm_runtime *runtime = substream->runtime;
>> struct snd_usb_substream *subs = &as->substream[direction];
>> + struct snd_usb_audio *chip = subs->stream->chip;
>> int ret;
>>
>> + mutex_lock(&chip->mutex);
>> + if (subs->opened) {
>> + mutex_unlock(&chip->mutex);
>> + return -EBUSY;
>> + }
>> + subs->opened = 1;
>> + mutex_unlock(&chip->mutex);
>> +
>> runtime->hw = snd_usb_hardware;
>> /* need an explicit sync to catch applptr update in low-latency mode */
>> if (direction == SNDRV_PCM_STREAM_PLAYBACK &&
>> @@ -1259,13 +1268,23 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>>
>> ret = setup_hw_info(runtime, subs);
>> if (ret < 0)
>> - return ret;
>> + goto err_open;
>> ret = snd_usb_autoresume(subs->stream->chip);
>> if (ret < 0)
>> - return ret;
>> + goto err_open;
>> ret = snd_media_stream_init(subs, as->pcm, direction);
>> if (ret < 0)
>> - snd_usb_autosuspend(subs->stream->chip);
>> + goto err_resume;
>> +
>> + return 0;
>> +
>> +err_resume:
>> + snd_usb_autosuspend(subs->stream->chip);
>> +err_open:
>> + mutex_lock(&chip->mutex);
>> + subs->opened = 0;
>> + mutex_unlock(&chip->mutex);
>> +
>> return ret;
>> }
>>
>> @@ -1274,6 +1293,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>> int direction = substream->stream;
>> struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
>> struct snd_usb_substream *subs = &as->substream[direction];
>> + struct snd_usb_audio *chip = subs->stream->chip;
>> int ret;
>>
>> snd_media_stop_pipeline(subs);
>> @@ -1287,6 +1307,9 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>>
>> subs->pcm_substream = NULL;
>> snd_usb_autosuspend(subs->stream->chip);
>> + mutex_lock(&chip->mutex);
>> + subs->opened = 0;
>> + mutex_unlock(&chip->mutex);
>>
>> return 0;
>> }