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

[Wesley Cheng <quic_wcheng@xxxxxxxxxxx>]

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@xxxxxxxxxxx>
> > +/**
> > + * 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