| Message ID | 20240801011730.4797-11-quic_wcheng@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce QC USB SND audio offloading support | expand |
Hi Pierre, On 8/1/2024 1:07 AM, Pierre-Louis Bossart wrote: > > >> +static inline int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component, >> + struct snd_soc_jack *jack) >> +{ >> + return -ENODEV; >> +} >> + >> +static inline int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component) >> +{ >> + return -ENODEV; >> +} > usually fallback functions return 0, is the error code intentional? ACK. > >> +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", > do we need the reference to Playback? No, will remove. >> + SND_JACK_HEADPHONE, jack); > wondering if there would be any merit in defining a new type of jack, > e.g. SND_JACK_USB since here the purpose is to notify that there's a USB > device connected. The type of device does not really matter, does it? > Not as of now, but I think we discussed in the past that maybe depending on if playback and capture is supported, we can notify SND_JACK_HEADSET? That is something I will need to change depending on how we want to handle the comments on patch#9 Thanks Wesley Cheng >> + 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_err(component->card->dev, "Failed to set jack: %d\n", ret); >> + return ret; >> + } >> + >> + return 0;
On 8/2/2024 12:43 AM, Wesley Cheng wrote: > Hi Pierre, > > On 8/1/2024 1:07 AM, Pierre-Louis Bossart wrote: >> >> >>> +static inline int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component, >>> + struct snd_soc_jack *jack) >>> +{ >>> + return -ENODEV; >>> +} >>> + >>> +static inline int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component) >>> +{ >>> + return -ENODEV; >>> +} >> usually fallback functions return 0, is the error code intentional? > ACK. >> >>> +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", >> do we need the reference to Playback? > No, will remove. >>> + SND_JACK_HEADPHONE, jack); >> wondering if there would be any merit in defining a new type of jack, >> e.g. SND_JACK_USB since here the purpose is to notify that there's a USB >> device connected. The type of device does not really matter, does it? >> > Not as of now, but I think we discussed in the past that maybe depending on if playback and capture is supported, we can notify SND_JACK_HEADSET? That is something I will need to change depending on how we want to handle the comments on patch#9 > I agree with Pierre that SND_JACK_HEADPHONE is too specific in this case, adding SND_JACK_USB sounds like good solution, as there are more device types than headset and headphones. Alternatively you could also consider defining some type of USB Audio Class mapping to existing SND_JACK types. (Look for UAC_INPUT_TERMINAL_*, UAC_OUTPUT_TERMINAL_* & UAC_BIDIR_TERMINAL_*.)
Hi Amadeusz, On 8/6/2024 7:51 AM, Amadeusz Sławiński wrote: > On 8/2/2024 12:43 AM, Wesley Cheng wrote: >> Hi Pierre, >> >> On 8/1/2024 1:07 AM, Pierre-Louis Bossart wrote: >>> >>> >>>> +static inline int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component, >>>> + struct snd_soc_jack *jack) >>>> +{ >>>> + return -ENODEV; >>>> +} >>>> + >>>> +static inline int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component) >>>> +{ >>>> + return -ENODEV; >>>> +} >>> usually fallback functions return 0, is the error code intentional? >> ACK. >>> >>>> +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", >>> do we need the reference to Playback? >> No, will remove. >>>> + SND_JACK_HEADPHONE, jack); >>> wondering if there would be any merit in defining a new type of jack, >>> e.g. SND_JACK_USB since here the purpose is to notify that there's a USB >>> device connected. The type of device does not really matter, does it? >>> >> Not as of now, but I think we discussed in the past that maybe depending on if playback and capture is supported, we can notify SND_JACK_HEADSET? That is something I will need to change depending on how we want to handle the comments on patch#9 >> > > I agree with Pierre that SND_JACK_HEADPHONE is too specific in this case, adding SND_JACK_USB sounds like good solution, as there are more device types than headset and headphones. Alternatively you could also consider defining some type of USB Audio Class mapping to existing SND_JACK types. (Look for UAC_INPUT_TERMINAL_*, UAC_OUTPUT_TERMINAL_* & UAC_BIDIR_TERMINAL_*.) Let me take a look at this and get back to you if I have any questions. Thanks Wesley Cheng
diff --git a/include/sound/soc-usb.h b/include/sound/soc-usb.h index 20394b3552cd..d6b576f971ae 100644 --- a/include/sound/soc-usb.h +++ b/include/sound/soc-usb.h @@ -6,6 +6,8 @@ #ifndef __LINUX_SND_SOC_USB_H #define __LINUX_SND_SOC_USB_H +#include <sound/soc.h> + /** * struct snd_soc_usb_device * @card_idx - sound card index associated with USB device @@ -46,6 +48,10 @@ 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 *dev); +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); + struct snd_soc_usb *snd_soc_usb_allocate_port(struct snd_soc_component *component, int num_streams, void *data); void snd_soc_usb_free_port(struct snd_soc_usb *usb); @@ -69,6 +75,17 @@ static inline void *snd_soc_usb_find_priv_data(struct device *dev) return NULL; } +static inline int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component, + struct snd_soc_jack *jack) +{ + return -ENODEV; +} + +static inline int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component) +{ + return -ENODEV; +} + static inline struct snd_soc_usb *snd_soc_usb_allocate_port( struct snd_soc_component *component, int num_streams, void *data) diff --git a/sound/soc/soc-usb.c b/sound/soc/soc-usb.c index b8a87e3fc6b2..fe2a75a28af4 100644 --- a/sound/soc/soc-usb.c +++ b/sound/soc/soc-usb.c @@ -4,8 +4,10 @@ */ #include <linux/of.h> #include <linux/usb.h> -#include <sound/soc.h> + +#include <sound/jack.h> #include <sound/soc-usb.h> + #include "../usb/card.h" static DEFINE_MUTEX(ctx_mutex); @@ -57,6 +59,64 @@ 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 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_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_find_priv_data() - Retrieve private data stored * @dev: device reference
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> --- include/sound/soc-usb.h | 17 +++++++++++ sound/soc/soc-usb.c | 62 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-)