lkml.org 
[lkml]   [2023]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 5/5] ASoC: codecs: Add WCD939x Codec driver
On 13/12/2023 20:20, Mark Brown wrote:
> On Thu, Dec 07, 2023 at 11:28:08AM +0100, Neil Armstrong wrote:
>
>> +static int wcd939x_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 wcd939x_priv *wcd939x = snd_soc_component_get_drvdata(component);
>> + u32 mode_val;
>> +
>> + mode_val = ucontrol->value.enumerated.item[0];
>> +
>> + if (wcd939x->variant == WCD9390) {
>> + if (mode_val == CLS_H_HIFI || mode_val == CLS_AB_HIFI) {
>> + dev_dbg(component->dev, "%s: Invalid HPH Mode\n", __func__);
>> + return -EINVAL;
>> + }
>> + }
>> + if (mode_val == CLS_H_NORMAL) {
>> + dev_dbg(component->dev, "%s: Unsupported HPH Mode\n", __func__);
>> + return -EINVAL;
>> + }
>> +
>> + wcd939x->hph_mode = mode_val;
>
> This seems strange - the code will accept any value other than a small
> number of specifically enumerated ones? I would have expected us to
> check a defined list of modes and reject anything that isn't in that
> list. This also means that the get() function can return out of bounds
> values which is buggy. Please use the mixer-test selftest on a card
> with this driver running, it should identify at least that issue.
>
>> +
>> + return 1;
>> +}
>
> This will also unconditionally report that the value of the mux changed,
> the function should return 0 if the value written is the control value
> hasn't changed.

Ack, I'll fix this, I wasn't happy anyway with the design

Neil

\
 
 \ /
  Last update: 2023-12-15 14:12    [W:0.066 / U:0.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site