Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 29 Jun 2022 11:58:05 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc:     alsa-devel@alsa-project.org, lgirdwood@gmail.com, perex@perex.cz,
        tiwai@suse.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] ASoC: codecs: wsa883x: add control, dapm widgets and
 map
Message-ID: <YrwwPbm0sL1RGwjO@sirena.org.uk>
References: <20220629090644.67982-1-srinivas.kandagatla@linaro.org>
 <20220629090644.67982-4-srinivas.kandagatla@linaro.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="gcLfZaC9ZTnbhhTf"
Content-Disposition: inline
In-Reply-To: <20220629090644.67982-4-srinivas.kandagatla@linaro.org>
Precedence: bulk


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

On Wed, Jun 29, 2022 at 10:06:43AM +0100, Srinivas Kandagatla wrote:

> +static int wsa_dev_mode_put(struct snd_kcontrol *kcontrol,
> +			    struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
> +	struct wsa883x_priv *wsa883x = snd_soc_component_get_drvdata(component);
> +
> +	if (wsa883x->dev_mode == ucontrol->value.enumerated.item[0])
> +		return 0;
> +
> +	wsa883x->dev_mode = ucontrol->value.enumerated.item[0];
> +
> +	return 0;
> +}

This isn't returning 1 when the value changes so will miss generating
events, please test the driver with mixer-test.

> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		if (wsa883x->dev_mode == RECEIVER) {
> +			snd_soc_component_write_field(component, WSA883X_CDC_PATH_MODE,
> +						      WSA883X_RXD_MODE_MASK,
> +						      WSA883X_RXD_MODE_HIFI);
> +			snd_soc_component_write_field(component, WSA883X_SPKR_PWM_CLK_CTL,
> +						      WSA883X_SPKR_PWM_FREQ_SEL_MASK,
> +						      WSA883X_SPKR_PWM_FREQ_F600KHZ);
> +			snd_soc_component_write_field(component, WSA883X_DRE_CTL_0,
> +						       WSA883X_DRE_PROG_DELAY_MASK, 0x0);
> +		} else if (wsa883x->dev_mode == SPEAKER) {

This looks like it'd be better written as a switch statement.

> +static const struct snd_kcontrol_new wsa883x_snd_controls[] = {
> +	SOC_SINGLE_RANGE_TLV("PA Gain", WSA883X_DRE_CTL_1, 1,
> +			     0x0, 0x1f, 1, pa_gain),

Volume controls should end in Volume, mixer-test should also have caught
this.

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

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmK8MD0ACgkQJNaLcl1U
h9BXjwf+MBbQuEiO10aKYLzGCSekPsbbLZ5nbdrQRJrPPHQBz7GZeCm94ocxTqW6
urpKw5bfp4TZu28szf0jv2G/i6/pBDk+1gCbquKS//eps+rQyse3GmQ5Y/lV5lUT
8EumngMnlYMHMrBNK2iHDoYNAtrT7dYnoy3EWuhK8nR8End9AaJtTeBBztRWYU8T
SLvvPyzwBJd0gSSUzxnQG1IIHqfwLuDO/10fu2Y2Cv9DXhBAlNMivycxdy7UvmP0
aiofhOWakZx8fBM7LwQlxZVw0YB5UQHUrMMOWn4injxV6Ko0Xgoafios15wPpAr9
6JJlYCnoKRcGsjs4OAUYgwDMqRXuRg==
=91P7
-----END PGP SIGNATURE-----

--gcLfZaC9ZTnbhhTf--