Re: [RFC PATCH 03/14] ASoC: qcom: Add USB backend ASoC driver for Q6

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Dec 27, 2022 at 01:04:55PM +0000, Mark Brown wrote:
> On Sat, Dec 24, 2022 at 10:02:59AM +0100, Greg KH wrote:
> > On Fri, Dec 23, 2022 at 03:31:49PM -0800, Wesley Cheng wrote:
> 
> > > + * struct q6usb_offload
> > > + * @dev - dev handle to usb be
> 
> > "be"?  What is that?
> 
> Back end.  This is a concept in DPCM which should be reasonably
> discoverable to people working on the audio portions of this code.

Ok, then how is the reference counting logic handled here?  USB devices
can be removed from the system at any point in time...

> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> 
> > All of the code in this patch series is older than 2022 as I know it has
> > been in shipping devices for many years.  Please use the proper
> > copyright year to make your lawyers happy...
> 
> Are you *positive* about this.  Based on some preparatory discussions
> the Qualcomm people had with Takashi and I it seemed like this was a new
> version of existing concepts.  I'm sure they had something already but
> it's not obvious to me that they're just posting the same code.

I thought that this same code has been shipping in devices for a few
years now in the last few Samsung phone models.  Is this not the same
code that is in those devices?

If not, why not, what happened to that codebase that makes it not worthy
of being submitted upstream?

> > > +static const struct snd_soc_dapm_route q6usb_dapm_routes[] = {
> > > +	{"USB Playback", NULL, "USB_RX_BE"},
> > > +};
> 
> > No terminating entry?  How does this not break?  Why do you need to
> > specify the size of the array, that feels like a design bug somewhere.
> 
> It's how the interface works, the number of entries is passed in when
> adding routes.

Ah, you all might want to change that to make it simpler :)

thanks,

greg k-h