Re: [PATCH v3 09/28] sound: usb: card: Introduce USB SND platform op callbacks

From: Takashi Iwai <tiwai@suse.de>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Wesley Cheng <quic_wcheng@quicinc.com>,
	srinivas.kandagatla@linaro.org, mathias.nyman@intel.com,
	broonie@kernel.org, lgirdwood@gmail.com,
	krzysztof.kozlowski+dt@linaro.org, agross@kernel.org,
	Thinh.Nguyen@synopsys.com, bgoswami@quicinc.com,
	andersson@kernel.org, robh+dt@kernel.org, tiwai@suse.com,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
	linux-usb@vger.kernel.org, quic_jackp@quicinc.com,
	quic_plai@quicinc.com
Subject: Re: [PATCH v3 09/28] sound: usb: card: Introduce USB SND platform op callbacks
Date: Thu, 09 Mar 2023 15:10:51 +0100	[thread overview]
Message-ID: <87r0tyf21g.wl-tiwai@suse.de> (raw)
In-Reply-To: <ZAmAPX6Q1m0HU/Qo@kroah.com>

On Thu, 09 Mar 2023 07:44:13 +0100,
Greg KH wrote:
> 
> On Wed, Mar 08, 2023 at 03:57:32PM -0800, Wesley Cheng wrote:
> > Allow for different platforms to be notified on USB SND connect/disconnect
> > seqeunces.  This allows for platform USB SND modules to properly initialize
> > and populate internal structures with references to the USB SND chip
> > device.
> > 
> > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> > ---
> >  sound/usb/card.c | 36 ++++++++++++++++++++++++++++++++++++
> >  sound/usb/card.h | 20 ++++++++++++++++++++
> >  2 files changed, 56 insertions(+)
> > 
> > diff --git a/sound/usb/card.c b/sound/usb/card.c
> > index 26268ffb8274..9bcbaa0c0a55 100644
> > --- a/sound/usb/card.c
> > +++ b/sound/usb/card.c
> > @@ -117,6 +117,30 @@ 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_platform_ops *platform_ops;
> 
> As I've said before, you can not just have one of these.  They need to
> be per-bus structure.  Or per-device, something dynamic, not static like
> this.

Basically the change for USB-audio driver is a kind of add-on/plugin;
that is, it allows the platform_ops to override some operations of the
standard USB-audio driver for supporting the offload operations.

So, in that sense, this add-on itself is neither per-bus nor
per-device, and it's fine as a single pointer for now, IMO, since this
is the only known use case and it'd simplify the code.  If we'll have
more similar requirements in future, we may extend the implementation
and the API to allow the registration of multiple addons, too.

Though, there are a few obvious coding issues in the current patches,
and they have to be addressed, sure :)

thanks,

Takashi