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: <02cbf2a1-f8e6-4d5a-9d46-5c21f593bcdd@quicinc.com>
Date: Tue, 3 Sep 2024 16:52:03 -0700
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        <srinivas.kandagatla@...aro.org>, <mathias.nyman@...el.com>,
        <perex@...ex.cz>, <conor+dt@...nel.org>, <dmitry.torokhov@...il.com>,
        <corbet@....net>, <broonie@...nel.org>, <lgirdwood@...il.com>,
        <tiwai@...e.com>, <krzk+dt@...nel.org>, <Thinh.Nguyen@...opsys.com>,
        <bgoswami@...cinc.com>, <robh@...nel.org>,
        <gregkh@...uxfoundation.org>
CC: <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-sound@...r.kernel.org>, <linux-input@...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 v26 31/33] ALSA: usb-audio: Add USB offload route kcontrol

Hi Pierre,

On 8/30/2024 3:05 AM, Pierre-Louis Bossart wrote:
>
> On 8/29/24 21:41, Wesley Cheng wrote:
>> In order to allow userspace/applications know about USB offloading status,
>> expose a sound kcontrol that fetches information about which sound card
>> and PCM index the USB device is mapped to for supporting offloading.  In
>> the USB audio offloading framework, the ASoC BE DAI link is the entity
>> responsible for registering to the SOC USB layer.
>>
>> It is expected for the USB SND offloading driver to add the kcontrol to the
>> sound card associated with the USB audio device.  An example output would
>> look like:
> this is very odd, the offloading driver adds a control to another card?
>
> That seems like a rather important layering violation.
>
> I thought it was done the other way, the USB audio card created a
> control that would point to the other card/device.
The USB SND offloading vendor driver (qc_audio_offload), which technically co-exists with USB SND core, is the entity that will add the offload kcontrol to the USB audio device's sound card.  Initially, I had the kcontrol creation as part of the USB SND mixer, but I believe Takashi preferred if the vendor driver did it instead.
>
>> tinymix -D 1 get 'USB Offload Playback Route PCM#0'
>> -1, -1 (range -1->255)
>>
>> This example signifies that there is no mapped ASoC path available for the
>> USB SND device.
> but that control would not even exist if the ASoC-based driver isn't probed.
>
> It's become really hard to follow, I've been on all this for 1.5hrs and
> losing track of the design.

Hence why it probably is a good idea to leave it within the USB offload vendor driver.  There are checks to ensure that the ASoC based driver is probed/available before the kcontrols are created for a device.  If there are devices that were identified before the ASoC components were probed, then the snd_usb_rediscover_devices() is triggered as part of soc-usb, and that would call the connect_cb() callback to the USB SND vendor offload driver to create the kcontrol.

Thanks

Wesley Cheng

>> tinymix -D 1 get 'USB Offload Playback Route PCM#0'
>> 0, 0 (range -1->255)
>>
>> This example signifies that the offload path is available over ASoC sound
>> card index#0 and PCM device#0.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
>> ---
>>  sound/usb/Kconfig                 |  10 +++
>>  sound/usb/Makefile                |   2 +
>>  sound/usb/mixer_usb_offload.c     | 102 ++++++++++++++++++++++++++++++
>>  sound/usb/mixer_usb_offload.h     |  17 +++++
>>  sound/usb/qcom/Makefile           |   2 +-
>>  sound/usb/qcom/qc_audio_offload.c |   2 +
>>  6 files changed, 134 insertions(+), 1 deletion(-)
>>  create mode 100644 sound/usb/mixer_usb_offload.c
>>  create mode 100644 sound/usb/mixer_usb_offload.h
>>
>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
>> index 5cc3eaf53e6b..e3fbf9541d0b 100644
>> --- a/sound/usb/Kconfig
>> +++ b/sound/usb/Kconfig
>> @@ -176,10 +176,20 @@ config SND_BCD2000
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called snd-bcd2000.
>>  
>> +config SND_USB_OFFLOAD_MIXER
>> +	tristate "USB Audio Offload mixer control"
>> +	help
>> +	 Say Y to enable the USB audio offloading mixer controls.  This
>> +	 exposes an USB offload capable kcontrol to signal to applications
>> +	 about which platform sound card can support USB audio offload.
>> +	 The returning values specify the mapped ASoC card and PCM device
>> +	 the USB audio device is associated to.
>> +
>>  config SND_USB_AUDIO_QMI
>>  	tristate "Qualcomm Audio Offload driver"
>>  	depends on QCOM_QMI_HELPERS && SND_USB_AUDIO && USB_XHCI_SIDEBAND && SND_SOC_USB
>>  	select SND_PCM
>> +	select SND_USB_OFFLOAD_MIXER
>>  	help
>>  	  Say Y here to enable the Qualcomm USB audio offloading feature.
>>  
>> diff --git a/sound/usb/Makefile b/sound/usb/Makefile
>> index 54a06a2f73ca..2f19f854944c 100644
>> --- a/sound/usb/Makefile
>> +++ b/sound/usb/Makefile
>> @@ -36,3 +36,5 @@ obj-$(CONFIG_SND_USB_US122L) += snd-usbmidi-lib.o
>>  
>>  obj-$(CONFIG_SND) += misc/ usx2y/ caiaq/ 6fire/ hiface/ bcd2000/ qcom/
>>  obj-$(CONFIG_SND_USB_LINE6)	+= line6/
>> +
>> +obj-$(CONFIG_SND_USB_OFFLOAD_MIXER) += mixer_usb_offload.o
>> diff --git a/sound/usb/mixer_usb_offload.c b/sound/usb/mixer_usb_offload.c
>> new file mode 100644
>> index 000000000000..16e2fd634684
>> --- /dev/null
>> +++ b/sound/usb/mixer_usb_offload.c
>> @@ -0,0 +1,102 @@
>> +// 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 "usbaudio.h"
>> +#include "card.h"
>> +#include "helper.h"
>> +#include "mixer.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 ret;
>> +
>> +	ret = snd_soc_usb_update_offload_route(sysdev,
>> +					       CARD_IDX(kcontrol->private_value),
>> +					       PCM_IDX(kcontrol->private_value),
>> +					       SNDRV_PCM_STREAM_PLAYBACK,
>> +					       ucontrol->value.integer.value);
>> +	if (ret < 0) {
>> +		ucontrol->value.integer.value[0] = -1;
>> +		ucontrol->value.integer.value[1] = -1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +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_substream *subs;
>> +	struct snd_usb_stream *as;
>> +	char ctl_name[37];
>> +	int ret;
>> +
>> +	list_for_each_entry(as, &chip->pcm_list, list) {
>> +		subs = &as->substream[SNDRV_PCM_STREAM_PLAYBACK];
>> +		if (!subs->ep_num)
>> +			continue;
>> +
>> +		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;
>> +}
>> +EXPORT_SYMBOL_GPL(snd_usb_offload_create_ctl);
>> diff --git a/sound/usb/mixer_usb_offload.h b/sound/usb/mixer_usb_offload.h
>> new file mode 100644
>> index 000000000000..3f6e2a8b19c8
>> --- /dev/null
>> +++ b/sound/usb/mixer_usb_offload.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef __USB_OFFLOAD_MIXER_H
>> +#define __USB_OFFLOAD_MIXER_H
>> +
>> +#if IS_ENABLED(CONFIG_SND_USB_OFFLOAD_MIXER)
>> +int snd_usb_offload_create_ctl(struct snd_usb_audio *chip);
>> +#else
>> +static inline int snd_usb_offload_create_ctl(struct snd_usb_audio *chip)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +#endif /* __USB_OFFLOAD_MIXER_H */
>> diff --git a/sound/usb/qcom/Makefile b/sound/usb/qcom/Makefile
>> index a81c9b28d484..4005e8391ab9 100644
>> --- a/sound/usb/qcom/Makefile
>> +++ b/sound/usb/qcom/Makefile
>> @@ -1,2 +1,2 @@
>>  snd-usb-audio-qmi-objs := usb_audio_qmi_v01.o qc_audio_offload.o
>> -obj-$(CONFIG_SND_USB_AUDIO_QMI) += snd-usb-audio-qmi.o
>> \ No newline at end of file
>> +obj-$(CONFIG_SND_USB_AUDIO_QMI) += snd-usb-audio-qmi.o
>> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
>> index a7ad15404fd1..5b9262a116be 100644
>> --- a/sound/usb/qcom/qc_audio_offload.c
>> +++ b/sound/usb/qcom/qc_audio_offload.c
>> @@ -36,6 +36,7 @@
>>  #include "../helper.h"
>>  #include "../pcm.h"
>>  #include "../power.h"
>> +#include "../mixer_usb_offload.h"
>>  
>>  #include "usb_audio_qmi_v01.h"
>>  
>> @@ -1703,6 +1704,7 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
>>  		sdev->card_idx = chip->card->number;
>>  		sdev->chip_idx = chip->index;
>>  
>> +		snd_usb_offload_create_ctl(chip);
>>  		snd_soc_usb_connect(usb_get_usb_backend(udev), sdev);
>>  	}
>>  

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ