linux-kernel - Re: [PATCH v30 14/30] ASoC: usb: Create SOC USB SND jack 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: <11ced6c9-6917-41f7-975b-24641595002b@quicinc.com>
Date: Tue, 3 Dec 2024 15:52:11 -0800
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Cezary Rojewski <cezary.rojewski@...el.com>
CC: <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-sound@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <linux-input@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-doc@...r.kernel.org>, <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>, <krzk+dt@...nel.org>,
        <pierre-louis.bossart@...ux.intel.com>, <Thinh.Nguyen@...opsys.com>,
        <tiwai@...e.com>, <robh@...nel.org>, <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v30 14/30] ASoC: usb: Create SOC USB SND jack kcontrol


On 12/3/2024 8:14 AM, Cezary Rojewski wrote:
> On 2024-11-06 8:33 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.
>
> ...
>
>> +/* SOC USB sound kcontrols */
>
> I'd suggest to use 'SoC' over 'SOC'. The former is predominant in the ASoC code.
>

Ok, I can modify the abbreviations


>> +/**
>> + * 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 Jack",
>> +                    SND_JACK_USB, jack);
>> +    if (ret < 0) {
>> +        dev_err(component->card->dev, "Unable to add USB offload jack: %d\n",
>> +            ret);
>> +        return ret;
>> +    }
>> +
>> +    ret = snd_soc_component_set_jack(component, jack, NULL);
>> +    if (ret) {
>> +        dev_err(component->card->dev, "Failed to set jack: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(snd_soc_usb_setup_offload_jack);
>
> Do we really need this one? Error reporting/handling for both invocations above is redundant, the log message should be provided by lower-level API. No need to pollute each caller with them. And with that part removed, we end up with basic ASoC calls, hardly a new-API candidate.
>

In previous discussions, my understanding was that we wanted to have this API, so that the sound jack naming, etc.. was consistent throughout all designs.  So that was the incentive of having its own dedicated API.

https://lore.kernel.org/linux-usb/8e08fd5e-91b8-c73e-1d97-7cf4d98573d4@quicinc.com/


>> +/**
>> + * snd_soc_usb_disable_offload_jack() - Disables USB offloading jack
>> + * @component: USB DPCM backend DAI component
>> + *
>> + * Disables the offload jack device, so that further connection events
>> + * won't be notified.
>> + *
>> + * Returns 0 on success, negative on error.
>> + *
>> + */
>> +int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component)
>> +{
>> +    int ret;
>> +
>> +    ret = snd_soc_component_set_jack(component, NULL, NULL);
>> +    if (ret) {
>> +        dev_err(component->card->dev, "Failed to disable jack: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(snd_soc_usb_disable_offload_jack);
>
> Code duplication. ASoC already provides the API and the logging is redundant here.
>

OK, maybe the enable/disable apis are not too useful.


Thanks

Wesley Cheng


>> +/**
>> + * snd_soc_usb_enable_offload_jack() - Enables USB offloading jack
>> + * @component: USB DPCM backend DAI component
>> + * @jack: offload jack to enable
>> + *
>> + * Enables the offload jack device, so that further connection events
>> + * will be notified.  This is the complement to
>> + * snd_soc_usb_disable_offload_jack().
>> + *
>> + * Returns 0 on success, negative on error.
>> + *
>> + */
>> +int snd_soc_usb_enable_offload_jack(struct snd_soc_component *component,
>> +                    struct snd_soc_jack *jack)
>> +{
>> +    int ret;
>> +
>> +    ret = snd_soc_component_set_jack(component, jack, NULL);
>> +    if (ret) {
>> +        dev_err(component->card->dev, "Failed to enable jack: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(snd_soc_usb_enable_offload_jack);
>
> Ditto.
>
>>   /**
>>    * snd_soc_usb_find_priv_data() - Retrieve private data stored
>>    * @usbdev: device reference
>>
>