Re: [PATCH v4 11/32] sound: usb: card: Introduce USB SND platform op callbacks

On Tue, 25 Jul 2023 04:33:55 +0200, Wesley Cheng wrote:

Allow for different platforms to be notified on USB SND connect/disconnect seqeunces. This allows for platform USB SND modules to properly initialize and populate internal structures with references to the USB SND chip device.

Signed-off-by: Wesley Cheng quic_wcheng@quicinc.com

sound/usb/card.c | 36 ++++++++++++++++++++++++++++++++++++ sound/usb/card.h | 20 ++++++++++++++++++++ 2 files changed, 56 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c index f6e99ced8068..9365d1e17836 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -117,6 +117,30 @@ MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no) static DEFINE_MUTEX(register_mutex); static struct snd_usb_audio *usb_chip[SNDRV_CARDS]; static struct usb_driver usb_audio_driver; +static struct snd_usb_platform_ops *platform_ops;

+int snd_usb_register_platform_ops(struct snd_usb_platform_ops *ops) +{

• if (platform_ops)

• return -EEXIST;
  

• mutex_lock(&register_mutex);
• platform_ops = ops;
• mutex_unlock(&register_mutex);

The check should be inside register_mutex lock, too. Otherwise it's racy.

--- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -207,4 +207,24 @@ struct snd_usb_stream { struct list_head list; };

+struct snd_usb_platform_ops {

• void (*connect_cb)(struct snd_usb_audio *chip);
• void (*disconnect_cb)(struct snd_usb_audio *chip);
• void (*suspend_cb)(struct usb_interface *intf, pm_message_t message);

+};

Don't we need resume_cb? Even if it's unused for your platform, it'd make sense to be a pair.

+#if IS_ENABLED(CONFIG_SND_USB_AUDIO) +int snd_usb_register_platform_ops(struct snd_usb_platform_ops *ops); +int snd_usb_unregister_platform_ops(void); +#else +static int snd_usb_register_platform_ops(struct snd_usb_platform_ops *ops) +{

• return -EOPNOTSUPP;

+}

+static int snd_usb_unregister_platform_ops(void) +{

• return -EOPNOTSUPP;

+} +#endif /* IS_ENABLED(CONFIG_SND_USB_AUDIO) */

Any need for dummy function definitions? I thought those exported functions are always with CONFIG_SND_USB_AUDIO enabled?

IOW, are they use of those functions without USB audio driver?

thanks,

Takashi