[PATCH v26 29/33] ALSA: usb-audio: qcom: Don't allow USB offload path if PCM device is in use

Wesley Cheng posted 33 patches 2 weeks, 6 days ago
There is a newer version of this series
[PATCH v26 29/33] ALSA: usb-audio: qcom: Don't allow USB offload path if PCM device is in use
Posted by Wesley Cheng 2 weeks, 6 days ago
Add proper checks and updates to the USB substream once receiving a USB QMI
stream enable request.  If the substream is already in use from the non
offload path, reject the stream enable request.  In addition, update the
USB substream opened parameter when enabling the offload path, so the
non offload path can be blocked.

Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 sound/usb/qcom/qc_audio_offload.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index e9f2fd6bbb41..0bd533f539e4 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -1482,12 +1482,17 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 		goto response;
 	}
 
+	mutex_lock(&chip->mutex);
 	if (req_msg->enable) {
-		if (info_idx < 0 || chip->system_suspend) {
+		if (info_idx < 0 || chip->system_suspend || subs->opened) {
 			ret = -EBUSY;
+			mutex_unlock(&chip->mutex);
+
 			goto response;
 		}
+		subs->opened = 1;
 	}
+	mutex_unlock(&chip->mutex);
 
 	if (req_msg->service_interval_valid) {
 		ret = get_data_interval_from_si(subs,
@@ -1509,6 +1514,11 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 		if (!ret)
 			ret = prepare_qmi_response(subs, req_msg, &resp,
 						   info_idx);
+		if (ret < 0) {
+			mutex_lock(&chip->mutex);
+			subs->opened = 0;
+			mutex_unlock(&chip->mutex);
+		}
 	} else {
 		info = &uadev[pcm_card_num].info[info_idx];
 		if (info->data_ep_pipe) {
@@ -1532,6 +1542,9 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 		}
 
 		disable_audio_stream(subs);
+		mutex_lock(&chip->mutex);
+		subs->opened = 0;
+		mutex_unlock(&chip->mutex);
 	}
 
 response:
Re: [PATCH v26 29/33] ALSA: usb-audio: qcom: Don't allow USB offload path if PCM device is in use
Posted by Pierre-Louis Bossart 2 weeks, 5 days ago

On 8/29/24 21:41, Wesley Cheng wrote:
> Add proper checks and updates to the USB substream once receiving a USB QMI
> stream enable request.  If the substream is already in use from the non
> offload path, reject the stream enable request.  In addition, update the
> USB substream opened parameter when enabling the offload path, so the
> non offload path can be blocked.
> 
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---
>  sound/usb/qcom/qc_audio_offload.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> index e9f2fd6bbb41..0bd533f539e4 100644
> --- a/sound/usb/qcom/qc_audio_offload.c
> +++ b/sound/usb/qcom/qc_audio_offload.c
> @@ -1482,12 +1482,17 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>  		goto response;
>  	}
>  
> +	mutex_lock(&chip->mutex);
>  	if (req_msg->enable) {
> -		if (info_idx < 0 || chip->system_suspend) {
> +		if (info_idx < 0 || chip->system_suspend || subs->opened) {
>  			ret = -EBUSY;
> +			mutex_unlock(&chip->mutex);
> +
>  			goto response;
>  		}
> +		subs->opened = 1;
>  	}
> +	mutex_unlock(&chip->mutex);
>  
>  	if (req_msg->service_interval_valid) {
>  		ret = get_data_interval_from_si(subs,
> @@ -1509,6 +1514,11 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>  		if (!ret)
>  			ret = prepare_qmi_response(subs, req_msg, &resp,
>  						   info_idx);
> +		if (ret < 0) {
> +			mutex_lock(&chip->mutex);
> +			subs->opened = 0;
> +			mutex_unlock(&chip->mutex);
> +		}
>  	} else {
>  		info = &uadev[pcm_card_num].info[info_idx];
>  		if (info->data_ep_pipe) {
> @@ -1532,6 +1542,9 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>  		}
>  
>  		disable_audio_stream(subs);
> +		mutex_lock(&chip->mutex);
> +		subs->opened = 0;
> +		mutex_unlock(&chip->mutex);
>  	}
>  


This sounds ok but I wonder why all this needs to be done in
Qualcomm-specific layers instead of soc-usb?
Re: [PATCH v26 29/33] ALSA: usb-audio: qcom: Don't allow USB offload path if PCM device is in use
Posted by Wesley Cheng 2 weeks, 1 day ago
Hi Pierre,

On 8/30/2024 2:55 AM, Pierre-Louis Bossart wrote:
>
> On 8/29/24 21:41, Wesley Cheng wrote:
>> Add proper checks and updates to the USB substream once receiving a USB QMI
>> stream enable request.  If the substream is already in use from the non
>> offload path, reject the stream enable request.  In addition, update the
>> USB substream opened parameter when enabling the offload path, so the
>> non offload path can be blocked.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>>  sound/usb/qcom/qc_audio_offload.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
>> index e9f2fd6bbb41..0bd533f539e4 100644
>> --- a/sound/usb/qcom/qc_audio_offload.c
>> +++ b/sound/usb/qcom/qc_audio_offload.c
>> @@ -1482,12 +1482,17 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>>  		goto response;
>>  	}
>>  
>> +	mutex_lock(&chip->mutex);
>>  	if (req_msg->enable) {
>> -		if (info_idx < 0 || chip->system_suspend) {
>> +		if (info_idx < 0 || chip->system_suspend || subs->opened) {
>>  			ret = -EBUSY;
>> +			mutex_unlock(&chip->mutex);
>> +
>>  			goto response;
>>  		}
>> +		subs->opened = 1;
>>  	}
>> +	mutex_unlock(&chip->mutex);
>>  
>>  	if (req_msg->service_interval_valid) {
>>  		ret = get_data_interval_from_si(subs,
>> @@ -1509,6 +1514,11 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>>  		if (!ret)
>>  			ret = prepare_qmi_response(subs, req_msg, &resp,
>>  						   info_idx);
>> +		if (ret < 0) {
>> +			mutex_lock(&chip->mutex);
>> +			subs->opened = 0;
>> +			mutex_unlock(&chip->mutex);
>> +		}
>>  	} else {
>>  		info = &uadev[pcm_card_num].info[info_idx];
>>  		if (info->data_ep_pipe) {
>> @@ -1532,6 +1542,9 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>>  		}
>>  
>>  		disable_audio_stream(subs);
>> +		mutex_lock(&chip->mutex);
>> +		subs->opened = 0;
>> +		mutex_unlock(&chip->mutex);
>>  	}
>>  
>
> This sounds ok but I wonder why all this needs to be done in
> Qualcomm-specific layers instead of soc-usb?
>
This is to prevent conflicts within the non-offload/legacy USB SND path and the USB SND offload vendor driver.  Don't think we need to involve anything with ASoC in these checks.

Thanks

Wesley Cheng