linux-kernel - Re: [PATCH v5 11/32] sound: usb: card: Introduce USB SND platform op callbacks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <875y4matbc.wl-tiwai@suse.de>
Date:   Thu, 07 Sep 2023 17:36:55 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Wesley Cheng <quic_wcheng@...cinc.com>
Cc:     <srinivas.kandagatla@...aro.org>, <mathias.nyman@...el.com>,
        <perex@...ex.cz>, <lgirdwood@...il.com>, <andersson@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <gregkh@...uxfoundation.org>,
        <Thinh.Nguyen@...opsys.com>, <broonie@...nel.org>,
        <bgoswami@...cinc.com>, <tiwai@...e.com>, <robh+dt@...nel.org>,
        <agross@...nel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <alsa-devel@...a-project.org>,
        <devicetree@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <quic_jackp@...cinc.com>, <quic_plai@...cinc.com>
Subject: Re: [PATCH v5 11/32] sound: usb: card: Introduce USB SND platform op callbacks

On Tue, 29 Aug 2023 23:06:36 +0200,
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@...cinc.com>
> ---
>  sound/usb/card.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/card.h |  9 +++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 1b2edc0fd2e9..067a1e82f4bf 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -118,6 +118,34 @@ 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;
> +
> +int snd_usb_register_platform_ops(struct snd_usb_platform_ops *ops)
> +{
> +	int ret;
> +
> +	mutex_lock(&register_mutex);
> +	if (platform_ops) {
> +		ret = -EEXIST;
> +		goto out;
> +	}
> +
> +	platform_ops = ops;
> +out:
> +	mutex_unlock(&register_mutex);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_usb_register_platform_ops);

For adding this kind of API, please give the proper comment.
Especially this API is special and need a caution, to mention that it
can be used only for a single instance.

Also, it should be mentioned that all callbacks are exclusive under
the global register_mutex.

> @@ -910,7 +938,11 @@ static int usb_audio_probe(struct usb_interface *intf,
>  	chip->num_interfaces++;
>  	usb_set_intfdata(intf, chip);
>  	atomic_dec(&chip->active);
> +
> +	if (platform_ops && platform_ops->connect_cb)
> +		platform_ops->connect_cb(chip);
>  	mutex_unlock(&register_mutex);

One uncertain thing is the argument for connect_cb and disconnect_cb.
Those take snd_usb_audio object, but the callback gets called per
interface at each probe and disconnect.  How does the callee handle
multiple calls?

Last but not least, the patch subject should be with "ALSA:" prefix,
and in this case, at best "ALSA: usb-audio: xxx".


thanks,

Takashi