Re: [PATCH v26 30/33] ALSA: usb-audio: qcom: Use card and PCM index from QMI request

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

On 8/29/24 21:41, Wesley Cheng wrote:
> Utilize the card and PCM index coming from the USB QMI stream request.
> This field follows what is set by the ASoC USB backend, and could
> potentially carry information about a specific device selected through the
> ASoC USB backend.  The backend also has information about the last USB
> sound device plugged in, so it can choose to select the last device plugged
> in, accordingly.
> 
> Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
> ---
>  sound/usb/qcom/qc_audio_offload.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> index 0bd533f539e4..a7ad15404fd1 100644
> --- a/sound/usb/qcom/qc_audio_offload.c
> +++ b/sound/usb/qcom/qc_audio_offload.c
> @@ -106,8 +106,6 @@ struct uaudio_qmi_dev {
>  	bool er_mapped;
>  	/* reference count to number of possible consumers */
>  	atomic_t qdev_in_use;
> -	/* idx to last udev card number plugged in */
> -	unsigned int last_card_num;
>  };
>  
>  struct uaudio_dev {
> @@ -1261,7 +1259,7 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,
>  
>  	pcm_dev_num = (req_msg->usb_token & QMI_STREAM_REQ_DEV_NUM_MASK) >> 8;
>  	xfer_buf_len = req_msg->xfer_buff_size;
> -	card_num = uaudio_qdev->last_card_num;
> +	card_num = (req_msg->usb_token & QMI_STREAM_REQ_CARD_NUM_MASK) >> 16;
>  
>  	if (!uadev[card_num].ctrl_intf) {
>  		dev_err(&subs->dev->dev, "audio ctrl intf info not cached\n");
> @@ -1455,8 +1453,7 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>  
>  	direction = (req_msg->usb_token & QMI_STREAM_REQ_DIRECTION);
>  	pcm_dev_num = (req_msg->usb_token & QMI_STREAM_REQ_DEV_NUM_MASK) >> 8;
> -	pcm_card_num = req_msg->enable ? uaudio_qdev->last_card_num :
> -				ffs(uaudio_qdev->card_slot) - 1;
> +	pcm_card_num = (req_msg->usb_token & QMI_STREAM_REQ_CARD_NUM_MASK) >> 16;
>  	if (pcm_card_num >= SNDRV_CARDS) {
>  		ret = -EINVAL;
>  		goto response;
> @@ -1706,7 +1703,6 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
>  		sdev->card_idx = chip->card->number;
>  		sdev->chip_idx = chip->index;
>  
> -		uaudio_qdev->last_card_num = chip->card->number;
>  		snd_soc_usb_connect(usb_get_usb_backend(udev), sdev);
>  	}

This entire path seems like a bad split/merge, it removes stuff that was
done earlier. Also it's not clear what this has to do with 'QMI', card
and PCM device management is usually done at a higher level.

not following, please be mindful of reviewer fatigue when adding such
changes in patch 30/33....