linux-kernel - Re: [PATCH v3 2/7] ASoC: codecs: wcd-clsh: add new version support

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: <585401cc-4248-7769-4d9a-84553b2d0ea6@linaro.org>
Date:   Mon, 22 Mar 2021 10:26:21 +0000
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        broonie@...nel.org
Cc:     robh@...nel.org, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        lgirdwood@...il.com
Subject: Re: [PATCH v3 2/7] ASoC: codecs: wcd-clsh: add new version support



On 19/03/2021 15:23, Pierre-Louis Bossart wrote:
> 
>> +static void wcd_clsh_v3_set_hph_mode(struct snd_soc_component 
>> *component,
>> +                  int mode)
>> +{
>> +    u8 val = 0;
> 
> initialization not needed.
> 
I agree, will remove this in next spin.

>> +
>> +    switch (mode) {
>> +    case CLS_H_NORMAL:
>> +        val = 0x00;
>> +        break;
>> +    case CLS_AB:
>> +    case CLS_H_ULP:
>> +        val = 0x0C;
>> +        break;
>> +    case CLS_AB_HIFI:
>> +    case CLS_H_HIFI:
>> +        val = 0x08;
>> +        break;
>> +    case CLS_H_LP:
>> +    case CLS_H_LOHIFI:
>> +    case CLS_AB_LP:
>> +    case CLS_AB_LOHIFI:
>> +        val = 0x04;
>> +        break;
>> +    default:
>> +        dev_err(component->dev, "%s:Invalid mode %d\n", __func__, mode);
>> +        return;
>> +    };
>> +
>> +    snd_soc_component_update_bits(component, WCD9XXX_ANA_HPH, 0x0C, 
>> val);
>> +}
>> +
>> +
> 
> extra line
> 
will remove this in next spin.

>> +void wcd_clsh_set_hph_mode(struct wcd_clsh_ctrl *ctrl, int mode)
>> +{
>> +    struct snd_soc_component *comp = ctrl->comp;
>> +
>> +    if (ctrl->codec_version >= WCD937X)
>> +        wcd_clsh_v3_set_hph_mode(comp, mode);
>> +    else
>> +        wcd_clsh_v2_set_hph_mode(comp, mode);
>> +
>> +}
>> +
>>   static void wcd_clsh_set_flyback_current(struct snd_soc_component 
>> *comp,
>>                        int mode)
>>   {
>> @@ -289,6 +388,130 @@ static void 
>> wcd_clsh_set_buck_regulator_mode(struct snd_soc_component *comp,
>>                       WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_H);
>>   }
>> +static void wcd_clsh_v3_set_buck_regulator_mode(struct 
>> snd_soc_component *component,
>> +                        int mode)
>> +{
>> +    snd_soc_component_update_bits(component, WCD9XXX_ANA_RX_SUPPLIES,
>> +                0x02, 0x00);
>> +}
>> +
>> +static inline void wcd_clsh_v3_set_flyback_mode(struct 
>> snd_soc_component *component,
>> +                        int mode)
>> +{
>> +    if (mode == CLS_H_HIFI || mode == CLS_H_LOHIFI ||
>> +        mode == CLS_AB_HIFI || mode == CLS_AB_LOHIFI) {
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_ANA_RX_SUPPLIES,
>> +                0x04, 0x04);
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_FLYBACK_VNEG_CTRL_4,
>> +                0xF0, 0x80);
>> +    } else {
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_ANA_RX_SUPPLIES,
>> +                0x04, 0x00); /* set to Default */
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_FLYBACK_VNEG_CTRL_4,
>> +                0xF0, 0x70);
>> +    }
>> +}
>> +
>> +static inline void wcd_clsh_v3_force_iq_ctl(struct snd_soc_component 
>> *component,
>> +                     int mode, bool enable)
>> +{
>> +    if (enable) {
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_FLYBACK_VNEGDAC_CTRL_2,
>> +                0xE0, 0xA0);
>> +        /* 100usec delay is needed as per HW requirement */
>> +        usleep_range(100, 110);
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_CLASSH_MODE_3,
>> +                0x02, 0x02);
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_CLASSH_MODE_2,
>> +                0xFF, 0x1C);
>> +        if (mode == CLS_H_LOHIFI || mode == CLS_AB_LOHIFI) {
>> +            snd_soc_component_update_bits(component,
>> +                    WCD9XXX_HPH_NEW_INT_PA_MISC2,
>> +                    0x20, 0x20);
>> +            snd_soc_component_update_bits(component,
>> +                    WCD9XXX_RX_BIAS_HPH_LOWPOWER,
>> +                    0xF0, 0xC0);
>> +            snd_soc_component_update_bits(component,
>> +                    WCD9XXX_HPH_PA_CTL1,
>> +                    0x0E, 0x02);
>> +        }
>> +    } else {
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_HPH_NEW_INT_PA_MISC2,
>> +                0x20, 0x00);
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_RX_BIAS_HPH_LOWPOWER,
>> +                0xF0, 0x80);
>> +        snd_soc_component_update_bits(component,
>> +                WCD9XXX_HPH_PA_CTL1,
>> +                0x0E, 0x06);
>> +    }
>> +}
> 
> do you need the inline for the two functions above?
yes, these are totally unnecessary.

--srini
> 
>