Re: [PATCH 2/2] usb: gadget: u_audio: remove unnecessary array declaration of snd_kcontrol_new

[Pavel Hofman <pavel.hofman@xxxxxxxxxxx>]

Dne 12. 10. 21 v 18:53 Yunhao Tian napsal(a):
> From: Yunhao Tian <t123yh.xyz@xxxxxxxxx>
>
> Currently, an array is used to contain all snd_kcontrol_new objects
> of the audio gadget. This is unnecessary and possibly prone to an
> (unlikely happen) race condition between the assignment of name
> and call of snd_ctl_new1 if two audio gadget is being set up simutaneously.
> This patch removes the global snd_kcontrol_new array and initialize
> individual snd_kcontrol_new object when it's being used.
>
> Signed-off-by: Yunhao Tian <t123yh.xyz@xxxxxxxxx>
> ---
>   drivers/usb/gadget/function/u_audio.c | 65 +++++++++++----------------
>   1 file changed, 25 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index c5f39998c653..1f4226d75dd8 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -27,12 +27,6 @@
>   #define PRD_SIZE_MAX	PAGE_SIZE
>   #define MIN_PERIODS	4
>   
> -enum {
> -	UAC_FBACK_CTRL,
> -	UAC_MUTE_CTRL,
> -	UAC_VOLUME_CTRL,
> -};
> -
>   /* Runtime data params for one stream */
>   struct uac_rtd_params {
>   	struct snd_uac_chip *uac;	/* parent chip */
> @@ -914,31 +908,6 @@ static int u_audio_volume_put(struct snd_kcontrol *kcontrol,
>   	return change;
>   }
>   
> -
> -static struct snd_kcontrol_new u_audio_controls[]  = {
> -	[UAC_FBACK_CTRL] {
> -		.iface =        SNDRV_CTL_ELEM_IFACE_PCM,
> -		.name =         "Capture Pitch 1000000",
> -		.info =         u_audio_pitch_info,
> -		.get =          u_audio_pitch_get,
> -		.put =          u_audio_pitch_put,
> -	},
> -	[UAC_MUTE_CTRL] {
> -		.iface =	SNDRV_CTL_ELEM_IFACE_MIXER,
> -		.name =		"", /* will be filled later */
> -		.info =		u_audio_mute_info,
> -		.get =		u_audio_mute_get,
> -		.put =		u_audio_mute_put,
> -	},
> -	[UAC_VOLUME_CTRL] {
> -		.iface =	SNDRV_CTL_ELEM_IFACE_MIXER,
> -		.name =		"", /* will be filled later */
> -		.info =		u_audio_volume_info,
> -		.get =		u_audio_volume_get,
> -		.put =		u_audio_volume_put,
> -	},
> -};

Hi,

Please is this patch necessary? My patch (a fixed version of which I 
will submit today) defines another control and several other important 
controls are on their way in a few patches. My current devel version has:

enum {
	UAC_FBACK_CTRL,
	UAC_P_PITCH_CTRL,
	UAC_MUTE_CTRL,
	UAC_VOLUME_CTRL,
	UAC_CAPTURE_RATE_CTRL,
	UAC_PLAYBACK_RATE_CTRL,
	UAC_CAPTURE_REQ_CTRL,
	UAC_PLAYBACK_REQ_CTRL,
};

I actually like the current method, IMO it keeps it quite organized.

Anyway if you want to remove it, please can you wait for all the 
important patches to land first?

Thanks a lot for considering.

Pavel.