diff mbox series

[v30,14/30] ASoC: usb: Create SOC USB SND jack kcontrol

Message ID 20241106193413.1730413-15-quic_wcheng@quicinc.com
State New
Headers show
Series Introduce QC USB SND audio offloading support | expand

Commit Message

Wesley Cheng Nov. 6, 2024, 7:33 p.m. UTC 
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.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 include/sound/soc-usb.h | 23 +++++++++++
 sound/soc/soc-usb.c     | 89 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

Comments

Cezary Rojewski Dec. 3, 2024, 4:14 p.m. UTC | #1 
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.

> +/**
> + * 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.

> +/**
> + * 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.

> +/**
> + * 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
>
Wesley Cheng Dec. 3, 2024, 11:52 p.m. UTC | #2 
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
>>
>
diff mbox series

Patch

diff --git a/include/sound/soc-usb.h b/include/sound/soc-usb.h
index db9ff0b4191d..587ea07a8cf5 100644
--- a/include/sound/soc-usb.h
+++ b/include/sound/soc-usb.h
@@ -56,6 +56,12 @@  int snd_soc_usb_connect(struct device *usbdev, struct snd_soc_usb_device *sdev);
 int snd_soc_usb_disconnect(struct device *usbdev, struct snd_soc_usb_device *sdev);
 void *snd_soc_usb_find_priv_data(struct device *usbdev);
 
+int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component,
+				   struct snd_soc_jack *jack);
+int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component);
+int snd_soc_usb_enable_offload_jack(struct snd_soc_component *component,
+				    struct snd_soc_jack *jack);
+
 struct snd_soc_usb *snd_soc_usb_allocate_port(struct snd_soc_component *component,
 					      void *data);
 void snd_soc_usb_free_port(struct snd_soc_usb *usb);
@@ -86,6 +92,23 @@  static inline void *snd_soc_usb_find_priv_data(struct device *usbdev)
 	return NULL;
 }
 
+static inline int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component,
+						 struct snd_soc_jack *jack)
+{
+	return 0;
+}
+
+static inline int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component)
+{
+	return 0;
+}
+
+static inline int snd_soc_usb_enable_offload_jack(struct snd_soc_component *component,
+						  struct snd_soc_jack *jack)
+{
+	return 0;
+}
+
 static inline struct snd_soc_usb *
 snd_soc_usb_allocate_port(struct snd_soc_component *component, void *data)
 {
diff --git a/sound/soc/soc-usb.c b/sound/soc/soc-usb.c
index c63033468e4a..ab914878e101 100644
--- a/sound/soc/soc-usb.c
+++ b/sound/soc/soc-usb.c
@@ -4,7 +4,10 @@ 
  */
 #include <linux/of.h>
 #include <linux/usb.h>
+
+#include <sound/jack.h>
 #include <sound/soc-usb.h>
+
 #include "../usb/card.h"
 
 static DEFINE_MUTEX(ctx_mutex);
@@ -56,6 +59,92 @@  static struct snd_soc_usb *snd_soc_find_usb_ctx(struct device *dev)
 	return ctx ? ctx : NULL;
 }
 
+/* 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 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);
+
+/**
+ * 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);
+
+/**
+ * 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);
+
 /**
  * snd_soc_usb_find_priv_data() - Retrieve private data stored
  * @usbdev: device reference