Re: [v2 03/17] ASoC: mediatek: mt8186: support adda in platform driver
From: AngeloGioacchino Del Regno
Date: Mon Mar 07 2022 - 04:30:24 EST
Il 05/03/22 11:49, Jiaxin Yu ha scritto:
On Fri, 2022-02-18 at 15:54 +0100, AngeloGioacchino Del Regno wrote:
Il 17/02/22 14:41, Jiaxin Yu ha scritto:
This patch adds mt8186 adda dai driver
Signed-off-by: Jiaxin Yu <jiaxin.yu@xxxxxxxxxxxx>
---
sound/soc/mediatek/mt8186/mt8186-dai-adda.c | 891
++++++++++++++++++++
1 file changed, 891 insertions(+)
create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-adda.c
diff --git a/sound/soc/mediatek/mt8186/mt8186-dai-adda.c
b/sound/soc/mediatek/mt8186/mt8186-dai-adda.c
new file mode 100644
index 000000000000..6d7dd1533da0
--- /dev/null
+++ b/sound/soc/mediatek/mt8186/mt8186-dai-adda.c
@@ -0,0 +1,891 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// MediaTek ALSA SoC Audio DAI ADDA Control
+//
+// Copyright (c) 2022 MediaTek Inc.
+// Author: Jiaxin Yu <jiaxin.yu@xxxxxxxxxxxx>
+
+#include <linux/regmap.h>
+#include <linux/delay.h>
+#include "mt8186-afe-clk.h"
+#include "mt8186-afe-common.h"
+#include "mt8186-afe-gpio.h"
+#include "mt8186-interconnection.h"
+
...snip...
+/* dai ops */
+static int mtk_dai_adda_hw_params(struct snd_pcm_substream
*substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+ struct mt8186_afe_private *afe_priv = afe->platform_priv;
+ unsigned int rate = params_rate(params);
+ int id = dai->id;
+ struct mtk_afe_adda_priv *adda_priv = afe_priv->dai_priv[id];
+
+ dev_info(afe->dev, "%s(), id %d, stream %d, rate %d\n",
+ __func__,
+ id,
+ substream->stream,
+ rate);
+
+ if (!adda_priv) {
+ dev_info(afe->dev, "%s(), adda_priv == NULL",
__func__);
+ return -EINVAL;
+ }
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ unsigned int dl_src2_con0 = 0;
+ unsigned int dl_src2_con1 = 0;
This initialization is redundant: you're never using these variables
before initializing them later, so initializing them to zero is not
needed here.
Yes, got it. Thank you.
+
+ adda_priv->dl_rate = rate;
+
+ /* set sampling rate */
+ dl_src2_con0 = adda_dl_rate_transform(afe, rate) <<
+ DL_2_INPUT_MODE_CTL_SFT;
+
+ /* set output mode, UP_SAMPLING_RATE_X8 */
+ dl_src2_con0 |= (0x3 << DL_2_OUTPUT_SEL_CTL_SFT);
+
+ /* turn off mute function */
+ dl_src2_con0 |= (0x01 <<
DL_2_MUTE_CH2_OFF_CTL_PRE_SFT);
BIT() macro, please
+ dl_src2_con0 |= (0x01 <<
DL_2_MUTE_CH1_OFF_CTL_PRE_SFT);
+
+ /* set voice input data if input sample rate is 8k or
16k */
+ if (rate == 8000 || rate == 16000)
+ dl_src2_con0 |= 0x01 <<
DL_2_VOICE_MODE_CTL_PRE_SFT;
+
+ /* SA suggest apply -0.3db to audio/speech path */
+ dl_src2_con1 = MTK_AFE_ADDA_DL_GAIN_NORMAL <<
+ DL_2_GAIN_CTL_PRE_SFT;
+
+ /* turn on down-link gain */
+ dl_src2_con0 |= (0x01 << DL_2_GAIN_ON_CTL_PRE_SFT);
+
+ if (id == MT8186_DAI_ADDA) {
+ /* clean predistortion */
+ regmap_write(afe->regmap, AFE_ADDA_PREDIS_CON0,
0);
+ regmap_write(afe->regmap, AFE_ADDA_PREDIS_CON1,
0);
+
+ regmap_write(afe->regmap,
+ AFE_ADDA_DL_SRC2_CON0,
dl_src2_con0);
+ regmap_write(afe->regmap,
+ AFE_ADDA_DL_SRC2_CON1,
dl_src2_con1);
+
+ /* set sdm gain */
+ regmap_update_bits(afe->regmap,
+ AFE_ADDA_DL_SDM_DCCOMP_CON,
+ ATTGAIN_CTL_MASK_SFT,
+ AUDIO_SDM_LEVEL_NORMAL <<
+ ATTGAIN_CTL_SFT);
+
+ /* Use new 2nd sdm */
+ regmap_update_bits(afe->regmap,
+ AFE_ADDA_DL_SDM_DITHER_CON,
+ AFE_DL_SDM_DITHER_64TAP_EN_M
ASK_SFT,
+ 0x1 <<
AFE_DL_SDM_DITHER_64TAP_EN_SFT);
BIT(AFE_DL_SDM_DITHER_64TAP_EN_SFT)
+ regmap_update_bits(afe->regmap,
+ AFE_ADDA_DL_SDM_AUTO_RESET_C
ON,
+ AFE_DL_USE_NEW_2ND_SDM_MASK_
SFT,
+ 0x1 <<
AFE_DL_USE_NEW_2ND_SDM_SFT);
BIT(AFE_DL_USE_NEW_2ND_SDM_SFT)
+ regmap_update_bits(afe->regmap,
+ AFE_ADDA_DL_SDM_DCCOMP_CON,
+ USE_3RD_SDM_MASK_SFT,
+ AUDIO_SDM_2ND <<
USE_3RD_SDM_SFT);
+
+ /* sdm auto reset */
+ regmap_write(afe->regmap,
+ AFE_ADDA_DL_SDM_AUTO_RESET_CON,
+ SDM_AUTO_RESET_THRESHOLD);
+ regmap_update_bits(afe->regmap,
+ AFE_ADDA_DL_SDM_AUTO_RESET_C
ON,
+ SDM_AUTO_RESET_TEST_ON_MASK_
SFT,
+ 0x1 <<
SDM_AUTO_RESET_TEST_ON_SFT);
BIT(SDM_AUTO_RESET_TEST_ON_SFT)
+ }
+ } else {
+ unsigned int voice_mode = 0;
what about...
unsigned int ul_src_con0 = 0; /* default value */
unsigned int voice_mode = adda_ul_rate_transform(afe,
rate);
Agree with you.
+ unsigned int ul_src_con0 = 0; /* default value */
+
+ adda_priv->ul_rate = rate;
+
+ voice_mode = adda_ul_rate_transform(afe, rate);
+
+ ul_src_con0 |= (voice_mode << 17) & (0x7 << 17);
+
+ /* enable iir */
+ ul_src_con0 |= (1 << UL_IIR_ON_TMP_CTL_SFT) &
+ UL_IIR_ON_TMP_CTL_MASK_SFT;
+ ul_src_con0 |= (UL_IIR_SW << UL_IIRMODE_CTL_SFT) &
+ UL_IIRMODE_CTL_MASK_SFT;
+ switch (id) {
+ case MT8186_DAI_ADDA:
+ case MT8186_DAI_AP_DMIC:
+ /* 35Hz @ 48k */
+ regmap_write(afe->regmap,
+ AFE_ADDA_IIR_COEF_02_01,
0x00000000);
Please drop leading zeroes:
regmap_write(afe->regmap, AFE_ADDA_IIR_COEF_02_01, 0);
+ regmap_write(afe->regmap,
+ AFE_ADDA_IIR_COEF_04_03,
0x00003FB8);
... and also please write hex in lower-case:
Got it.
regmap_write(afe->regmap,
AFE_ADDA_IIR_COEF_04_03, 0x03fb8);
+ regmap_write(afe->regmap,
+ AFE_ADDA_IIR_COEF_06_05,
0x3FB80000);
+ regmap_write(afe->regmap,
+ AFE_ADDA_IIR_COEF_08_07,
0x3FB80000);
+ regmap_write(afe->regmap,
+ AFE_ADDA_IIR_COEF_10_09,
0x0000C048);
+
+ regmap_write(afe->regmap,
+ AFE_ADDA_UL_SRC_CON0,
ul_src_con0);
+
+ /* Using Internal ADC */
+ regmap_update_bits(afe->regmap,
+ AFE_ADDA_TOP_CON0,
+ 0x1 << 0,
+ 0x0 << 0);
Please use the BIT() macro:
regmap_update_bits(afe->regmap, AFE_ADDA_TOP_CON0, BIT(0), 0);
P.S.: 87 columns is ok
How can I judge whether it can exceed 80 lines?
https://www.kernel.org/doc/html/latest/process/coding-style.html
Besides, the 80 columns rule was made for old terminals (like VT100 and such)
that supported displaying a maximum of 80 cols... in the modern era, that's
not the case anymore.
It's somewhat preferred if something can fit in 80 columns, but if it does
impact on readability, then you can go over that, up to 100 columns.
scripts/checkpatch.pl was also changed to complain about 100 columns and not
80 anymore...
my $max_line_length = 100;
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl?h=v5.17-rc7#n59
Cheers,
Angelo