> -----Original Message----- > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb- > owner@xxxxxxxxxxxxxxx] On Behalf Of Greg KH > Sent: Wednesday, June 17, 2020 4:53 PM > To: JaeHun Jung > Cc: linux-usb@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] ANDROID: sound: usb: Add vendor's hooking interface > > On Wed, Jun 17, 2020 at 07:17:38AM +0200, Greg KH wrote: > > On Wed, Jun 17, 2020 at 10:55:30AM +0900, JaeHun Jung wrote: > > > In mobile, a co-processor is used when using USB audio to improve > > > power consumption. > > > hooking is required for sync-up when operating the co-processor. So > > > register call-back function. > > > The main operation of the call-back function is as follows: > > > - Initialize the co-processor by transmitting data > > > when initializing. > > > - Change the co-processor setting value through > > > the interface function. > > > - Configure sampling rate > > > - pcm open/close > > > > > > Bug: 156315379 > > > > > > Change-Id: I32e1dd408e64aaef68ee06c480c4b4d4c95546dc > > > > No need for Bug or Change-Id on patches submitted to us, same for the > > odd "ANDROID:" in the subject. > > Ok, I will delete it. > > > Signed-off-by: JaeHun Jung <jh0801.jung@xxxxxxxxxxx> > > > --- > > > sound/usb/card.c | 16 ++++++++++++++++ > > > sound/usb/card.h | 1 + > > > sound/usb/clock.c | 5 +++++ > > > sound/usb/pcm.c | 33 +++++++++++++++++++++++++++++++++ > > > sound/usb/usbaudio.h | 30 ++++++++++++++++++++++++++++++ > > > 5 files changed, 85 insertions(+) > > > > Did you run scripts/get_maintainer.pl on this patch to determine that > > maybe the alsa-devel list should also be needed? > > Yes, it was sent looking for maintainer of sound/usb. This callbacks is for sync with Audio Core. So, I was implement on sound/usb. > > > > > > > > > > diff --git a/sound/usb/card.c b/sound/usb/card.c index > > > fd6fd17..2f3fa14 100644 > > > --- a/sound/usb/card.c > > > +++ b/sound/usb/card.c > > > @@ -111,6 +111,7 @@ 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; > > > +struct snd_usb_audio_vendor_ops *usb_audio_ops; > > > > > > /* > > > * disconnect streams > > > @@ -210,6 +211,12 @@ static int snd_usb_create_stream(struct > snd_usb_audio *chip, int ctrlif, int int > > > return 0; > > > } > > > > > > +void snd_set_vender_interface(struct snd_usb_audio_vendor_ops > > > +*vendor_ops) { > > > + usb_audio_ops = vendor_ops; > > > +} > > > +EXPORT_SYMBOL_GPL(snd_set_vender_interface); > > > > You are exporting a lot of new symbols, but you have no user of these > > symbols, which is not allowed, as you know. Please also post your > > user of them so we can see if you are doing things correctly or not. > > Yes, I know. This is called from Audio core module. Audio related drivers associated with this module cannot disclose because of security. I think this is true of other vendors as well. > > Also, only one set of "vendor ops" does not make any sense at all, > > this needs to be on a per-host-controller basis, right? If so, why is > > this all in the sound driver? > Currently, this interface is only for USB audio. USB information is that is has in the xhci host driver. When USB audio is connected, F/W of audio core performs the control of USB host for low power. > Also, your api is making a lot of assumptions about the running system, > there seems to not be any way to always "know" what bus/device the > callbacks are being used for in many places. This is only used in limited scenarios. And the information of USB host get through from exynos_usb_audio driver. > > Why not just add the needed functionality to the sound driver itself > instead of trying to rely on these odd "callbacks"? Audio core operates in F/W and is module. Because there are many connected modules, it is cannot on built-in and module to implement the non-callbacks > > thanks, > > greg k-h
