Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 8 Jun 2021 14:59:33 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc:     robh@kernel.org, devicetree@vger.kernel.org, perex@perex.cz,
        alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
        lgirdwood@gmail.com
Subject: Re: [PATCH v8 6/9] ASoC: codecs: wcd938x: add basic controls
Message-ID: <20210608135933.GE4200@sirena.org.uk>
References: <20210601113158.16085-1-srinivas.kandagatla@linaro.org>
 <20210601113158.16085-7-srinivas.kandagatla@linaro.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="HnQK338I3UIa/qiP"
Content-Disposition: inline
In-Reply-To: <20210601113158.16085-7-srinivas.kandagatla@linaro.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--HnQK338I3UIa/qiP
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

--HnQK338I3UIa/qiP
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmC/d8QACgkQJNaLcl1U
h9BaYgf/X23DLW+LGGdVIwyniq5WCdfl7Dc/btd+vAgkelUwclvB6Pl1Zw66kuVZ
0Vl+eDcoldYEFDznPWS+TOyWkSs5MGmttYtMmmf9EEznvLJvHiNnbKgmXXas2VQm
m3eu6nCCy5ZKoG0kNmsvjS8HUtMAtu3+oTS5KO4AZThEK/o0ujnztR9szPXkHP/0
pIRsfKiuDjNLis5fWeb1OAc0Pu+2mujfiII48rj0BTHzLLaPKHxiblt5t8T472TT
38wnhKbCQvQimxd5owse/dM31lcTVx0HCEGnlOIE/rSjiCgJrOfnszN4IMZLgzST
aQAeTfaRDEGkYFWDHuS/1W+9HW02aQ==
=hi4X
-----END PGP SIGNATURE-----

--HnQK338I3UIa/qiP--