Re: [PATCH v20 33/41] ASoC: usb: Create SOC USB SND jack kcontrol

From: Wesley Cheng <quic_wcheng@quicinc.com>
To: "Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>,
	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@kernel.org,
	konrad.dybcio@linaro.org
Cc: <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>
Subject: Re: [PATCH v20 33/41] ASoC: usb: Create SOC USB SND jack kcontrol
Date: Tue, 30 Apr 2024 17:32:24 -0700	[thread overview]
Message-ID: <a1115d39-b60e-a5da-5c7e-79266d6ced17@quicinc.com> (raw)
In-Reply-To: <c8bd16e8-b204-471b-a702-36e14c8695c5@linux.intel.com>

Hi Amadeusz,

On 4/26/2024 6:26 AM, Amadeusz Sławiński wrote:
> On 4/25/2024 11:51 PM, Wesley Cheng wrote:
>> Expose API for creation of a jack control for notifying of available
>> devices that are plugged in/discovered, and that support offloading.  
>> This
>> allows for control names to be standardized across implementations of USB
>> audio offloading.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
> 
> (...)
> 
>>   /* SOC USB sound kcontrols */
>> +/**
>> + * snd_soc_usb_setup_offload_jack() - Create USB offloading jack
>> + * @component: USB DPCM backend DAI component
>> + * @jack: jack structure to create
>> + *
>> + * Creates a jack device for notifying userspace of the availability
>> + * of an offload capable device.
>> + *
>> + * Returns 0 on success, negative on error.
>> + *
>> + */
>> +int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component,
>> +                    struct snd_soc_jack *jack)
>> +{
>> +    int ret;
>> +
>> +    ret = snd_soc_card_jack_new(component->card, "USB Offload 
>> Playback Jack",
>> +                    SND_JACK_HEADPHONE, jack);
>> +    if (ret < 0) {
>> +        dev_err(component->card->dev, "Unable to add USB offload 
>> jack\n");
>> +        return ret;
>> +    }
>> +
>> +    ret = snd_soc_component_set_jack(component, jack, NULL);
>> +    if (ret) {
>> +        dev_warn(component->card->dev, "Failed to set jack: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(snd_soc_usb_setup_offload_jack);
>> +
>>   static int snd_soc_usb_get_offload_card_status(struct snd_kcontrol 
>> *kcontrol,
>>                      struct snd_ctl_elem_value *ucontrol)
>>   {
>>
> 
> I'm not sure if this should be handled in generic USB API, this feels 
> like something that should be handled in specific device driver side, 
> like all users currently do.
> 

In some of the previous comments, it was mentioned that maybe it was 
better to have more consistent/defined naming across devices that do 
have support for audio offload.  Initially, I did have these within our 
vendor specific ASoC platform driver also.

> Anyway I think there should also be some function that tears jack down, 
> by calling:
> snd_soc_component_set_jack(component, NULL, NULL);
> so it can get cleaned up properly?

I can add that.  I didn't realize there were some situations where maybe 
components would want to disable the jack.  I will leave the cleanup 
part to ASoC when the platform card is removed.

Thanks
Wesley Cheng