linux-kernel - Re: [PATCH 3/4] ASoC: codecs: wsa883x: add control, dapm widgets and map

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: <2fdd0847-5102-ea9f-cfbb-f3dd378d3a8a@linaro.org>
Date:   Wed, 29 Jun 2022 12:00:23 +0100
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Mark Brown <broonie@...nel.org>
Cc:     alsa-devel@...a-project.org, lgirdwood@...il.com, perex@...ex.cz,
        tiwai@...e.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] ASoC: codecs: wsa883x: add control, dapm widgets and
 map

Thanks Mark,

On 29/06/2022 11:58, Mark Brown wrote:
> 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.
Yes my bad..

> 
>> +	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.
Sounds good,

> 
>> +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.
Okay, will fix all these in next spin.