Re: [PATCH v12 19/41] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support

[Wesley Cheng <quic_wcheng@quicinc.com>]

Hi Hillf,

On 1/7/2024 1:32 AM, Hillf Danton wrote:
> On Tue, 2 Jan 2024 13:45:27 -0800 Wesley Cheng <quic_wcheng@quicinc.com>
>> +/**
>> + * qc_usb_audio_offload_probe() - platform op connect handler
>> + * @chip: USB SND device
>> + *
>> + * Platform connect handler when a USB SND device is detected. Will
>> + * notify SOC USB about the connection to enable the USB ASoC backend
>> + * and populate internal USB chip array.
>> + *
>> + */
>> +static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
>> +{
>> +	struct usb_device *udev = chip->dev;
>> +	struct snd_soc_usb_device *sdev;
>> +	struct xhci_sideband *sb;
>> +
>> +	/*
>> +	 * If there is no priv_data, the connected device is on a USB bus
>> +	 * that doesn't support offloading.  Avoid populating entries for
>> +	 * this device.
>> +	 */
>> +	if (!snd_soc_usb_find_priv_data(usb_get_usb_backend(udev)))
>> +		return;
>> +
>> +	mutex_lock(&chip->mutex);
>> +	if (!uadev[chip->card->number].chip) {
>> +		sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
>> +		if (!sdev)
>> +			goto exit;
>> +
>> +		sb = xhci_sideband_register(udev);
>> +		if (!sb)
>> +			goto free_sdev;
>> +	} else {
>> +		sb = uadev[chip->card->number].sb;
>> +		sdev = uadev[chip->card->number].sdev;
>> +	}
>> +
>> +	mutex_lock(&qdev_mutex);
>> +	if (!uaudio_qdev)
>> +		qc_usb_audio_offload_init_qmi_dev(udev);
>> +
>> +	atomic_inc(&uaudio_qdev->qdev_in_use);
>> +	mutex_unlock(&qdev_mutex);
>> +
>> +	uadev[chip->card->number].sb = sb;
>> +	uadev[chip->card->number].chip = chip;
> 
> Protecting uadev[] with a non global lock makes no sense.
>> +
>> +	sdev->card_idx = chip->card->number;
>> +	sdev->chip_idx = chip->index;
>> +	uadev[chip->card->number].sdev = sdev;
>> +
>> +	uaudio_qdev->last_card_num = chip->card->number;
>> +	snd_soc_usb_connect(usb_get_usb_backend(udev), sdev);
>> +
>> +	mutex_unlock(&chip->mutex);
>> +
>> +	return;
>> +
>> +free_sdev:
>> +	kfree(sdev);
>> +exit:
>> +	mutex_unlock(&chip->mutex);
>> +}
>> +
>> +/**
>> + * qc_usb_audio_offload_disconnect() - platform op disconnect handler
>> + * @chip: USB SND device
>> + *
>> + * Platform disconnect handler.  Will ensure that any pending stream is
>> + * halted by issuing a QMI disconnect indication packet to the adsp.
>> + *
>> + */
>> +static void qc_usb_audio_offload_disconnect(struct snd_usb_audio *chip)
>> +{
>> +	struct qmi_uaudio_stream_ind_msg_v01 disconnect_ind = {0};
>> +	struct uaudio_qmi_svc *svc = uaudio_svc;
>> +	struct uaudio_dev *dev;
>> +	int card_num;
>> +	int ret;
>> +
>> +	if (!chip)
>> +		return;
>> +
>> +	card_num = chip->card->number;
>> +	if (card_num >= SNDRV_CARDS)
>> +		return;
>> +
>> +	mutex_lock(&qdev_mutex);
>> +	mutex_lock(&chip->mutex);
> 
> Lock order looks correct here.
> 

Thanks for the review, will fix the order in probe.

Thanks
Wesley Cheng