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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Takashi,

On 9/7/2023 8:36 AM, Takashi Iwai wrote:
On Tue, 29 Aug 2023 23:06:36 +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@xxxxxxxxxxx>
---
  sound/usb/card.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
  sound/usb/card.h |  9 +++++++++
  2 files changed, 54 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 1b2edc0fd2e9..067a1e82f4bf 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -118,6 +118,34 @@ 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)
+{
+	int ret;
+
+	mutex_lock(&register_mutex);
+	if (platform_ops) {
+		ret = -EEXIST;
+		goto out;
+	}
+
+	platform_ops = ops;
+out:
+	mutex_unlock(&register_mutex);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_usb_register_platform_ops);

For adding this kind of API, please give the proper comment.
Especially this API is special and need a caution, to mention that it
can be used only for a single instance.

Also, it should be mentioned that all callbacks are exclusive under
the global register_mutex.


Thanks for taking the time to review. Sure, I'll add some comments in these new APIs to document what they are used for and how they are protected and limited.

@@ -910,7 +938,11 @@ static int usb_audio_probe(struct usb_interface *intf,
  	chip->num_interfaces++;
  	usb_set_intfdata(intf, chip);
  	atomic_dec(&chip->active);
+
+	if (platform_ops && platform_ops->connect_cb)
+		platform_ops->connect_cb(chip);
  	mutex_unlock(&register_mutex);

One uncertain thing is the argument for connect_cb and disconnect_cb.
Those take snd_usb_audio object, but the callback gets called per
interface at each probe and disconnect.  How does the callee handle
multiple calls?

I guess it should depend on how the platform driver wants to handle it? I haven't run into a device with multiple UAC interfaces before, so I'll need to mimic this configuration on a device, so I can see how it exposes itself.

Will investigate this a bit more on my end and come back with my findings.


Last but not least, the patch subject should be with "ALSA:" prefix,
and in this case, at best "ALSA: usb-audio: xxx".



Got it, thanks!

Thanks
Wesley Cheng



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux