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.
>
> > 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?
>
>
>
> >
> > 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.
>
> 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?
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.
Why not just add the needed functionality to the sound driver itself
instead of trying to rely on these odd "callbacks"?
thanks,
greg k-h
