linux-kernel - Re: [PATCH 08/12] ASoC: wcd9335: add basic controls

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]

Date:   Wed, 25 Jul 2018 17:34:07 +0530
From:   Vinod <vkoul@...nel.org>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc:     lee.jones@...aro.org, robh+dt@...nel.org, broonie@...nel.org,
        mark.rutland@....com, lgirdwood@...il.com, tiwai@...e.com,
        bgoswami@...eaurora.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, alsa-devel@...a-project.org
Subject: Re: [PATCH 08/12] ASoC: wcd9335: add basic controls

On 23-07-18, 16:54, Srinivas Kandagatla wrote:

> +static int wcd9335_set_compander(struct snd_kcontrol *kc,
> +				 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_soc_kcontrol_component(kc);
> +	struct wcd9335_codec *wcd = dev_get_drvdata(component->dev);
> +	int comp = ((struct soc_mixer_control *) kc->private_value)->shift;
> +	int value = ucontrol->value.integer.value[0];
> +	int sel;
> +
> +	wcd->comp_enabled[comp] = value;
> +	sel = value ? WCD9335_HPH_GAIN_SRC_SEL_COMPANDER :
> +		WCD9335_HPH_GAIN_SRC_SEL_REGISTER;
> +
> +	/* Any specific register configuration for compander */
> +	switch (comp) {
> +	case COMPANDER_1:
> +		/* Set Gain Source Select based on compander enable/disable */
> +		snd_soc_component_update_bits(component, WCD9335_HPH_L_EN,
> +				      WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
> +		break;
> +	case COMPANDER_2:
> +		snd_soc_component_update_bits(component, WCD9335_HPH_R_EN,
> +				      WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
> +		break;
> +	case COMPANDER_3:
> +		break;
> +	case COMPANDER_4:
> +		break;
> +	case COMPANDER_5:
> +		snd_soc_component_update_bits(component, WCD9335_SE_LO_LO3_GAIN,
> +				      WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
> +		break;
> +	case COMPANDER_6:
> +		snd_soc_component_update_bits(component, WCD9335_SE_LO_LO4_GAIN,
> +				      WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
> +		break;
> +	case COMPANDER_7:
> +		break;
> +	case COMPANDER_8:
> +		break;

why not do:

        case COMPANDER_3:
        case COMPANDER_4:
        case COMPANDER_7:
        case COMPANDER_8:
                break;

thereby clubbing all the ones not handled together..

> +static int wcd9335_rx_hph_mode_get(struct snd_kcontrol *kc,
> +				 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_soc_kcontrol_component(kc);
> +	struct wcd9335_codec *wcd = dev_get_drvdata(component->dev);
> +
> +	ucontrol->value.integer.value[0] = wcd->hph_mode;

i think this is an enum so shouldn't value.enumerated.item[0] be used?

> +
> +	return 0;
> +}
> +
> +static int wcd9335_rx_hph_mode_put(struct snd_kcontrol *kc,
> +				 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_soc_kcontrol_component(kc);
> +	struct wcd9335_codec *wcd = dev_get_drvdata(component->dev);
> +	u32 mode_val;
> +
> +	mode_val = ucontrol->value.enumerated.item[0];

yaaay
-- 
~Vinod