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

[Hillf Danton]

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.

> +	dev = &uadev[card_num];
> +
> +	/* Device has already been cleaned up, or never populated */
> +	if (!dev->chip) {
> +		mutex_unlock(&qdev_mutex);
> +		mutex_unlock(&chip->mutex);
> +		return;
> +	}
> +
> +	/* cleaned up already */
> +	if (!dev->udev)
> +		goto done;
> +
> +	if (atomic_read(&dev->in_use)) {
> +		mutex_unlock(&chip->mutex);
> +		mutex_unlock(&qdev_mutex);
> +		dev_dbg(uaudio_qdev->data->dev,
> +			"sending qmi indication disconnect\n");
> +		disconnect_ind.dev_event = USB_QMI_DEV_DISCONNECT_V01;
> +		disconnect_ind.slot_id = dev->udev->slot_id;
> +		disconnect_ind.controller_num = dev->usb_core_id;
> +		disconnect_ind.controller_num_valid = 1;
> +		ret = qmi_send_indication(svc->uaudio_svc_hdl, &svc->client_sq,
> +				QMI_UAUDIO_STREAM_IND_V01,
> +				QMI_UAUDIO_STREAM_IND_MSG_V01_MAX_MSG_LEN,
> +				qmi_uaudio_stream_ind_msg_v01_ei,
> +				&disconnect_ind);
> +		if (ret < 0)
> +			dev_err(uaudio_qdev->data->dev,
> +				"qmi send failed with err: %d\n", ret);
> +
> +		ret = wait_event_interruptible_timeout(dev->disconnect_wq,
> +				!atomic_read(&dev->in_use),
> +				msecs_to_jiffies(DEV_RELEASE_WAIT_TIMEOUT));
> +		if (!ret) {
> +			dev_err(uaudio_qdev->data->dev,
> +				"timeout while waiting for dev_release\n");
> +			atomic_set(&dev->in_use, 0);
> +		} else if (ret < 0) {
> +			dev_err(uaudio_qdev->data->dev,
> +				"failed with ret %d\n", ret);
> +			atomic_set(&dev->in_use, 0);
> +		}
> +		mutex_lock(&qdev_mutex);
> +		mutex_lock(&chip->mutex);
> +	}
> +
> +	uaudio_dev_cleanup(dev);
> +done:
> +	/*
> +	 * If num_interfaces == 1, the last USB SND interface is being removed.
> +	 * This is to accommodate for devices w/ multiple UAC functions.
> +	 */
> +	if (chip->num_interfaces == 1) {
> +		snd_soc_usb_disconnect(usb_get_usb_backend(chip->dev), dev->sdev);
> +		xhci_sideband_unregister(dev->sb);
> +		dev->chip = NULL;
> +		kfree(dev->sdev);
> +		dev->sdev = NULL;
> +	}
> +	mutex_unlock(&chip->mutex);
> +
> +	atomic_dec(&uaudio_qdev->qdev_in_use);
> +	if (!atomic_read(&uaudio_qdev->qdev_in_use))
> +		qc_usb_audio_cleanup_qmi_dev();
> +
> +	mutex_unlock(&qdev_mutex);
> +}