On Wed, Jun 17, 2020 at 06:10:39PM +0900, ������ wrote: > > > > -----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. What do you mean? The license of the code is GPL version 2, so there is no "security" reason to not publish it. > I think this is true of other vendors as well. What other vendor needs these hooks? > > > 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. But that's not how these hooks are being created, you need to properly handle any USB bus type. As-is these will not work correctly. > > 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. But that's not what these hooks show. Again, please publish all of the code, we can not just add random kernel hooks (that aren't even correct), without having a user of the code. Would you want to have to maintain such a system? > > 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 I do not understand, sorry. Perhaps the code that uses the hooks would better explain this. thanks, greg k-h
