Received-SPF: pass (google.com: domain of linux-kernel+bounces-55797-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <dbe544de-dc04-59a8-6642-883fc00214f3@quicinc.com>
Date: Tue, 6 Feb 2024 17:24:46 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v13 48/53] ALSA: usb-audio: mixer: Add USB offloading
 mixer control
Content-Language: en-US
To: Takashi Iwai <tiwai@suse.de>
CC: <srinivas.kandagatla@linaro.org>, <mathias.nyman@intel.com>,
        <perex@perex.cz>, <conor+dt@kernel.org>, <corbet@lwn.net>,
        <lgirdwood@gmail.com>, <andersson@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <gregkh@linuxfoundation.org>,
        <Thinh.Nguyen@synopsys.com>, <broonie@kernel.org>,
        <bgoswami@quicinc.com>, <tiwai@suse.com>, <robh+dt@kernel.org>,
        <konrad.dybcio@linaro.org>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-sound@vger.kernel.org>,
        <linux-usb@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-doc@vger.kernel.org>, <alsa-devel@alsa-project.org>
References: <20240203023645.31105-1-quic_wcheng@quicinc.com>
 <20240203023645.31105-49-quic_wcheng@quicinc.com>
 <871q9pwy0l.wl-tiwai@suse.de>
From: Wesley Cheng <quic_wcheng@quicinc.com>
In-Reply-To: <871q9pwy0l.wl-tiwai@suse.de>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

Hi Takashi,

On 2/6/2024 4:57 AM, Takashi Iwai wrote:
> On Sat, 03 Feb 2024 03:36:40 +0100,
> 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
>> index is associated with the ASoC platform card supporting offloading.  In
>> the USB audio offloading framework, the ASoC BE DAI link is the entity
>> responsible for registering to the SOC USB layer.  SOC USB will expose more
>> details about the current offloading status, which includes the USB sound
>> card and USB PCM device indexes currently being used.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> 
> The concept is understandable, but the control element name ("SNDUSB
> OFFLD playback available") looks non-intrusive and non-conformant.
> Use a bit more understandable name instead.
> 
> This provides a card number where the offload driver is bound, and the
> name should indicate something about that.
> 

Hmmm, does USB sound have a naming convention that it usually follows 
for mixer/control interfaces?

For something that is more closely related, how about:
"USB offload capable card"

> Also, about the implementation:
> 
>> +static int
>> +snd_usb_offload_create_mixer(struct usb_mixer_interface *mixer,
>> +		       const struct snd_kcontrol_new *new_kctl)
>> +{
>> +	struct snd_kcontrol *kctl;
>> +	struct usb_mixer_elem_info *elem;
>> +
>> +	elem = kzalloc(sizeof(struct usb_mixer_elem_info), GFP_KERNEL);
>> +	if (!elem)
>> +		return -ENOMEM;
>> +
>> +	elem->head.mixer = mixer;
>> +	elem->val_type = USB_MIXER_S32;
>> +	elem->control = 0;
>> +	elem->head.id = 0;
>> +	elem->channels = 1;
>> +
>> +	kctl = snd_ctl_new1(new_kctl, elem);
>> +	if (!kctl) {
>> +		kfree(elem);
>> +		return -ENOMEM;
>> +	}
>> +	kctl->private_free = snd_usb_mixer_elem_free;
>> +
>> +	return snd_usb_mixer_add_control(&elem->head, kctl);
> 
> This control has almost little to do with the standard USB interface,
> and it'll be much simpler if you create a raw control element.
> Pass the bus or the sysdev to private_data, and that's all you need in
> the get callback.
> 

Sure, I'll remove the need to register over the SND USB mixer API, and 
just use the core SND control APIs.

> Also, don't forget to set the proper access bits (it's read-only).
> 

Thanks for pointing this out, will fix.

Thanks
Wesley Cheng

> 
> thanks,
> 
> Takashi