linux-kernel - Re: [PATCH v24 29/34] ALSA: usb-audio: qcom: Add USB offload route kcontrol

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4d5fe3f8-d7ba-4647-8dd7-22656ec2fde5@linux.intel.com>
Date: Thu, 1 Aug 2024 11:02:08 +0200
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To: Wesley Cheng <quic_wcheng@...cinc.com>, srinivas.kandagatla@...aro.org,
 mathias.nyman@...el.com, perex@...ex.cz, conor+dt@...nel.org,
 corbet@....net, broonie@...nel.org, lgirdwood@...il.com, krzk+dt@...nel.org,
 Thinh.Nguyen@...opsys.com, bgoswami@...cinc.com, tiwai@...e.com,
 gregkh@...uxfoundation.org, robh@...nel.org
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-sound@...r.kernel.org, linux-usb@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, linux-doc@...r.kernel.org,
 alsa-devel@...a-project.org
Subject: Re: [PATCH v24 29/34] ALSA: usb-audio: qcom: Add USB offload route
 kcontrol



> +ifneq ($(CONFIG_SND_USB_QC_OFFLOAD_MIXER),)
> +snd-usb-audio-qmi-objs += mixer_usb_offload.o
> +endif
> \ No newline at end of file

add one?

> diff --git a/sound/usb/qcom/mixer_usb_offload.c b/sound/usb/qcom/mixer_usb_offload.c
> new file mode 100644
> index 000000000000..c00770400c67
> --- /dev/null
> +++ b/sound/usb/qcom/mixer_usb_offload.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/usb.h>
> +
> +#include <sound/core.h>
> +#include <sound/control.h>
> +#include <sound/soc-usb.h>
> +
> +#include "../card.h"
> +#include "../mixer.h"
> +#include "../usbaudio.h"
> +
> +#include "mixer_usb_offload.h"
> +
> +#define PCM_IDX(n)  (n & 0xffff)
> +#define CARD_IDX(n) (n >> 16)
> +
> +static int
> +snd_usb_offload_route_get(struct snd_kcontrol *kcontrol,
> +		      struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct device *sysdev = snd_kcontrol_chip(kcontrol);
> +	int card;
> +	int pcm;
> +
> +	card = soc_usb_get_offload_device(sysdev, CARD_IDX(kcontrol->private_value),
> +					  PCM_IDX(kcontrol->private_value),
> +					  SND_SOC_USB_KCTL_CARD_ROUTE);
> +
> +	pcm = soc_usb_get_offload_device(sysdev, CARD_IDX(kcontrol->private_value),
> +					 PCM_IDX(kcontrol->private_value),
> +					 SND_SOC_USB_KCTL_PCM_ROUTE);
> +	if (card < 0 || pcm < 0) {
> +		card = -1;
> +		pcm = -1;
> +	}
> +
> +	ucontrol->value.integer.value[0] = card;
> +	ucontrol->value.integer.value[1] = pcm;
> +
> +	return 0;
> +}

see my earlier comment, should those two calls be collapsed to return
all the information in one shot?

> +
> +static int snd_usb_offload_route_info(struct snd_kcontrol *kcontrol,
> +			      struct snd_ctl_elem_info *uinfo)
> +{
> +	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> +	uinfo->count = 2;
> +	uinfo->value.integer.min = -1;
> +	/* Arbitrary max value, as there is no 'limit' on number of PCM devices */
> +	uinfo->value.integer.max = 0xff;
> +
> +	return 0;
> +}
> +
> +static struct snd_kcontrol_new snd_usb_offload_mapped_ctl = {
> +	.iface = SNDRV_CTL_ELEM_IFACE_CARD,
> +	.access = SNDRV_CTL_ELEM_ACCESS_READ,
> +	.info = snd_usb_offload_route_info,
> +	.get = snd_usb_offload_route_get,
> +};
> +
> +/**
> + * snd_usb_offload_create_ctl() - Add USB offload bounded mixer
> + * @chip - USB SND chip device
> + *
> + * Creates a sound control for a USB audio device, so that applications can
> + * query for if there is an available USB audio offload path, and which
> + * card is managing it.
> + */
> +int snd_usb_offload_create_ctl(struct snd_usb_audio *chip)
> +{
> +	struct usb_device *udev = chip->dev;
> +	struct snd_kcontrol_new *chip_kctl;
> +	struct snd_usb_stream *as;
> +	char ctl_name[37];
> +	int ret;
> +
> +	list_for_each_entry(as, &chip->pcm_list, list) {
> +		chip_kctl = &snd_usb_offload_mapped_ctl;
> +		chip_kctl->count = 1;
> +		/*
> +		 * Store the associated USB SND card number and PCM index for
> +		 * the kctl.
> +		 */
> +		chip_kctl->private_value = as->pcm_index |
> +					  chip->card->number << 16;
> +		sprintf(ctl_name, "USB Offload Playback Route PCM#%d",
> +			as->pcm_index);
> +		chip_kctl->name = ctl_name;
> +		ret = snd_ctl_add(chip->card, snd_ctl_new1(chip_kctl,
> +				  udev->bus->sysdev));
> +		if (ret < 0)
> +			break;
> +	}
> +
> +	return ret;

None of this looks Qualcomm-specific, shouldn't this be part of the
soc_usb framework instead of being added in the qcom/ stuff?