[PATCH] ASoC: nau8821: new driver

[Seven Lee]

Amadeusz Sławiński 於 2021/8/9 下午 06:44 寫道:
> On 8/9/2021 12:10 PM, Seven Lee wrote:
>> Add driver for NAU88L21.
>>
>> Signed-off-by: Seven Lee <wtli at nuvoton.com>
>> ---
>
> ...
>
>> +
>> +static int dmic_clock_control(struct snd_soc_dapm_widget *w,
>> +        struct snd_kcontrol *k, int  event)
>> +{
>> +    struct snd_soc_component *component =
>> +        snd_soc_dapm_to_component(w->dapm);
>> +    struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
>> +    int i, speed_selection, clk_adc_src, clk_adc;
>> +    unsigned int clk_divider_r03;
>> +
>> +    /* The DMIC clock is gotten from adc clock divided by
>> +     * CLK_DMIC_SRC (1, 2, 4, 8). The clock has to be equal or
>> +     * less than nau8821->dmic_clk_threshold.
>> +     */
>> +    regmap_read(nau8821->regmap, NAU8821_R03_CLK_DIVIDER,
>> +        &clk_divider_r03);
>> +    clk_adc_src = (clk_divider_r03 & NAU8821_CLK_ADC_SRC_MASK)
>> +        >> NAU8821_CLK_ADC_SRC_SFT;
>> +
>> +    switch (clk_adc_src) {
>> +    case 0:
>> +        clk_adc = nau8821->fs * 256;
>> +        break;
>> +    case 1:
>> +        clk_adc = (nau8821->fs * 256) >> 1;
>> +        break;
>> +    case 2:
>> +        clk_adc = (nau8821->fs * 256) >> 2;
>> +        break;
>> +    case 3:
>> +        clk_adc = (nau8821->fs * 256) >> 3;
>> +        break;
>> +    }
>
> Just do:
> clk_adc = (nau8821->fs * 256) >> clk_adc_src;
> instead of whole switch?


Yes, you are right. I will fix it.


>
>> +
>> +    for (i = 0 ; i < 4 ; i++) {
>> +        if ((clk_adc >> dmic_speed_sel[i].param) <=
>> +            nau8821->dmic_clk_threshold) {
>> +            speed_selection = dmic_speed_sel[i].val;
>> +            break;
>> +        }
>> +    }
>> +
>> +    dev_dbg(nau8821->dev,
>> +        "clk_adc=%d, dmic_clk_threshold = %d, param=%d, val = %d\n",
>> +        clk_adc, nau8821->dmic_clk_threshold,
>> +        dmic_speed_sel[i].param, dmic_speed_sel[i].val);
>> +    regmap_update_bits(nau8821->regmap, NAU8821_R13_DMIC_CTRL,
>> +        NAU8821_DMIC_SRC_MASK,
>> +        (speed_selection << NAU8821_DMIC_SRC_SFT));
>> +
>> +    return 0;
>> +}
>> +
>
> ...
>
>> +
>> +static int nau8821_clock_check(struct nau8821 *nau8821,
>> +    int stream, int rate, int osr)
>> +{
>> +    int osrate = 0;
>> +
>> +    if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> +        if (osr >= ARRAY_SIZE(osr_dac_sel))
>> +            return -EINVAL;
>> +        osrate = osr_dac_sel[osr].osr;
>> +    } else {
>> +        if (osr >= ARRAY_SIZE(osr_adc_sel))
>> +            return -EINVAL;
>> +        osrate = osr_adc_sel[osr].osr;
>> +    }
> true and false cases seem to be the same here, you can remove the "if 
> else" and just leave one of them.


The OSR of DAC and ADC can be different. And the ratio corresponding
to the bit is also different. They are two different tables for
osr_dac_sel and osr_adc_sel. Then it should be noted that the OSR and
Fs must be selected carefully so that the max frequency of CLK_ADC or
CLK_DAC are less than or equal to 6.144MHz.


>
>> +
>> +    if (!osrate || rate * osrate > CLK_DA_AD_MAX) {
>> +        dev_err(nau8821->dev,
>> +        "exceed the maximum frequency of CLK_ADC or CLK_DAC\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>
> ...