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: <88d5ed6f-1429-4381-8014-d5824ec7866e@quicinc.com>
Date: Tue, 20 Aug 2024 10:37:14 -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>, <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

Hi Pierre,

On 8/19/2024 11:39 PM, Pierre-Louis Bossart wrote:
>>>> +/**
>>>> + * 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;
>> Hi Pierre,
>>> None of this looks Qualcomm-specific, shouldn't this be part of the
>>> soc_usb framework instead of being added in the qcom/ stuff?
>> Started working on this particular comment, and there are some things that needs to be considered if we moved this into SOC USB:
>>
>> 1.  We do save the reference to the USB BE DAI link within the USB DT node, which can be fetched/referenced based on sysdev.  However, I'm not sure if everyone would potentially follow that way.
>>
>> 2.  I tried a few implementations of adding a new SOC USB API, and the argument list was a bit long, because I didn't want to directly reference the usb_chip.
>>
>> Sorry for the delay, but I wanted to give a good stab at implementing this before bringing up the implications.  It is possible, but definitely not as clean as how we have it now IMO.
> My comment was only referring to the location of the code, it's now in
> sound/usb/qcom/mixer_usb_offload.c but does not contain anything
> specific to Qualcomm. I was not asking for any encapsulation inside of
> soc-usb, I was only suggesting a move of the code to a shared helper
> library so that this code can be reused as is and not duplicated if the
> QCOM parts are not compiled in.

Ah, great, thanks for the clarification.  Let me take a look with that perspective.

Thanks

Wesley Cheng