Messages in this thread | | | Date | Tue, 27 Dec 2022 23:33:10 +0200 | Subject | Re: [RFC PATCH 04/14] sound: usb: card: Introduce USB SND vendor op callbacks | From | Dmitry Baryshkov <> |
| |
On 27/12/2022 23:07, Wesley Cheng wrote: > Hi Dmitry, > > On 12/24/2022 3:03 AM, Dmitry Baryshkov wrote: >> Hi, >> >> On Sat, 24 Dec 2022 at 01:33, Wesley Cheng <quic_wcheng@quicinc.com> >> wrote: >>> >>> Allow for different vendors to be notified on USB SND connect/disconnect >>> seqeunces. This allows for vendor USB SND modules to properly >>> initialize >>> and populate internal structures with references to the USB SND chip >>> device. >> >> The commit message definitely needs some improvement. We do not notify >> vendors on SND connect/disconnect events. >> >> >>> >>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >>> --- >>> sound/usb/card.c | 22 ++++++++++++++++++++++ >>> sound/usb/card.h | 7 +++++++ >>> 2 files changed, 29 insertions(+) >>> >>> diff --git a/sound/usb/card.c b/sound/usb/card.c >>> index 26268ffb8274..212f55a7683c 100644 >>> --- a/sound/usb/card.c >>> +++ b/sound/usb/card.c >>> @@ -117,6 +117,21 @@ 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_vendor_ops *vendor_ops; >>> + >>> +int snd_usb_register_vendor_ops(struct snd_usb_vendor_ops *ops) >> >> platform ops? >> > > Will change it. > >>> +{ >>> + vendor_ops = ops; >>> + return 0; >>> +} >>> +EXPORT_SYMBOL_GPL(snd_usb_register_vendor_ops); >> >> What happens if several platforms try to register different ops? I saw >> from the patch 09/14 that you register these ops unconditionally. If >> other devices follow your approach there is an obvious conflict. >> > > Thank you for the review. > > That is true. I don't think there is a proper need to have multiple > vendor ops being registered, so maybe just returning an error for if ops > are already registered is sufficient.
This would be a required step. And also you have to check the running platform before registering your ops unconditionally. Ideally this should be done as a part of the device's driver, so that we can control registration of the platform ops using the usual interface.
> > Thanks > Wesley Cheng
-- With best wishes Dmitry
| |