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@xxxxxxxxxxx/
>> +/**
>> + * 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
>>
>
