linux-kernel - Re: [PATCH 4/4] ALSA: usb-audio: UAC3 Add support for connector insertion.

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: <s5hr2n6ru79.wl-tiwai@suse.de>
Date:   Mon, 23 Apr 2018 14:19:38 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     "Jorge Sanjuan" <jorge.sanjuan@...ethink.co.uk>
Cc:     <alsa-devel@...a-project.org>, <gregkh@...uxfoundation.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] ALSA: usb-audio: UAC3 Add support for connector insertion.

On Fri, 20 Apr 2018 19:03:27 +0200,
Jorge Sanjuan wrote:
> 
> diff --git a/include/linux/usb/audio-v3.h b/include/linux/usb/audio-v3.h
> index a8959aaba0ae..6cedb6d499ba 100644
> --- a/include/linux/usb/audio-v3.h
> +++ b/include/linux/usb/audio-v3.h
> @@ -221,6 +221,12 @@ struct uac3_iso_endpoint_descriptor {
>  	__le16 wLockDelay;
>  } __attribute__((packed));
>  
> +/* 5.2.1.6.1 INSERTION CONTROL PARAMETER BLOCK */
> +struct uac3_insertion_ctl_blk {
> +	__u8 bSize;
> +	__u8 bmConInserted[1];

Do we need to declare this as an array?

>  static struct snd_kcontrol_new usb_feature_unit_ctl = {
>  	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
>  	.name = "", /* will be filled later manually */
> @@ -1322,6 +1367,15 @@ static struct snd_kcontrol_new usb_bool_master_control_ctl_ro = {
>  	.put = NULL,
>  };
>  
> +static struct snd_kcontrol_new usb_connector_ctl_ro = {

Put const.


> @@ -1904,16 +1966,29 @@ static int parse_audio_input_terminal(struct mixer_build *state, int unitid,
>  				      void *raw_desc)
>  {
>  	struct usb_audio_term iterm;
> -	struct uac2_input_terminal_descriptor *d = raw_desc;
> +	unsigned int control, bmctls, term_id;
>  
> -	check_input_term(state, d->bTerminalID, &iterm);
>  	if (state->mixer->protocol == UAC_VERSION_2) {
> -		/* Check for jack detection. */
> -		if (uac_v2v3_control_is_readable(d->bmControls,
> -						 UAC2_TE_CONNECTOR)) {
> -			build_connector_control(state, &iterm, true);
> -		}
> -	}
> +		struct uac2_input_terminal_descriptor *d_v2 = raw_desc;
> +		control = UAC2_TE_CONNECTOR;
> +		term_id = d_v2->bTerminalID;
> +		bmctls = d_v2->bmControls;
> +	}
> +	else if (state->mixer->protocol == UAC_VERSION_3) {

Put "else if" and the closing brace in the same line.


> +		struct uac3_input_terminal_descriptor *d_v3 = raw_desc;
> +		control = UAC3_TE_INSERTION;
> +		term_id = d_v3->bTerminalID;
> +		bmctls = d_v3->bmControls;

Doesn't it need le32_to_cpu()?

> +	}
> +	else /* UAC1. No Insertion control */

Ditto, put "else" and the closing brace in the same line.
Also, use braces for the rest block, otherwise it'll look
inconsistent.  Or rewrite with switch().

> @@ -2645,6 +2720,12 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer)
>  			err = parse_audio_unit(&state, desc->bCSourceID);
>  			if (err < 0 && err != -EINVAL)
>  				return err;
> +
> +			if (uac_v2v3_control_is_readable(desc->bmControls,

Missing le32_to_cpu()?


thanks,

Takashi