Messages in this thread | | | Date | Mon, 11 Sep 2023 10:57:38 -0700 | From | Wesley Cheng <> | Subject | Re: [PATCH v5 11/32] sound: usb: card: Introduce USB SND platform op callbacks |
| |
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@quicinc.com> >> --- >> 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(®ister_mutex); >> + if (platform_ops) { >> + ret = -EEXIST; >> + goto out; >> + } >> + >> + platform_ops = ops; >> +out: >> + mutex_unlock(®ister_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(®ister_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
| |