Re: [PATCH v8 6/9] ASoC: codecs: wcd938x: add basic controls

[Mark Brown]

On Tue, Jun 01, 2021 at 12:31:55PM +0100, Srinivas Kandagatla wrote:

> +static int wcd938x_rx_hph_mode_put(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
> +	struct wcd938x_priv *wcd938x = snd_soc_component_get_drvdata(component);
> +
> +	wcd938x->hph_mode = ucontrol->value.enumerated.item[0];
> +
> +	return 0;
> +}

_put() should return true if it made a change, the same bug is present
in a lot of other drivers too.

> +static const struct snd_kcontrol_new wcd9380_snd_controls[] = {
> +	SOC_ENUM_EXT("RX HPH Mode", rx_hph_mode_mux_enum_wcd9380,
> +		     wcd938x_rx_hph_mode_get, wcd938x_rx_hph_mode_put),
> +	SOC_ENUM_EXT("TX0 MODE", tx_mode_mux_enum_wcd9380[0],
> +		     wcd938x_tx_mode_get, wcd938x_tx_mode_put),
> +	SOC_ENUM_EXT("TX1 MODE", tx_mode_mux_enum_wcd9380[1],
> +		     wcd938x_tx_mode_get, wcd938x_tx_mode_put),
> +	SOC_ENUM_EXT("TX2 MODE", tx_mode_mux_enum_wcd9380[2],
> +		     wcd938x_tx_mode_get, wcd938x_tx_mode_put),
> +	SOC_ENUM_EXT("TX3 MODE", tx_mode_mux_enum_wcd9380[3],
> +		     wcd938x_tx_mode_get, wcd938x_tx_mode_put),
> +};

Please don't use this pattern of indexing into arrays by absolute
number, it's error prone and hard to read.  Just declare static
variables for the enums and reference them individually.

Attachment: signature.asc
Description: PGP signature