* [PATCH v3 1/6] ASoC: mediatek: mt7986: add common header
2023-07-28 9:08 [PATCH v3 0/6] ASoC: mediatek: Add support for MT7986 SoC Maso Huang
@ 2023-07-28 9:08 ` Maso Huang
2023-08-04 9:15 ` Alexandre Mergnat
2023-07-28 9:08 ` [PATCH v3 2/6] ASoC: mediatek: mt7986: support etdm in platform driver Maso Huang
` (4 subsequent siblings)
5 siblings, 1 reply; 33+ messages in thread
From: Maso Huang @ 2023-07-28 9:08 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Maso Huang
Add header files for register definition and structure.
Signed-off-by: Maso Huang <maso.huang@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
sound/soc/mediatek/mt7986/mt7986-afe-common.h | 49 +++++
sound/soc/mediatek/mt7986/mt7986-reg.h | 206 ++++++++++++++++++
2 files changed, 255 insertions(+)
create mode 100644 sound/soc/mediatek/mt7986/mt7986-afe-common.h
create mode 100644 sound/soc/mediatek/mt7986/mt7986-reg.h
diff --git a/sound/soc/mediatek/mt7986/mt7986-afe-common.h b/sound/soc/mediatek/mt7986/mt7986-afe-common.h
new file mode 100644
index 000000000000..1c59549d91b4
--- /dev/null
+++ b/sound/soc/mediatek/mt7986/mt7986-afe-common.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * mt7986-afe-common.h -- MediaTek 7986 audio driver definitions
+ *
+ * Copyright (c) 2021 MediaTek Inc.
+ * Author: Vic Wu <vic.wu@mediatek.com>
+ * Maso Huang <maso.huang@mediatek.com>
+ */
+
+#ifndef _MT_7986_AFE_COMMON_H_
+#define _MT_7986_AFE_COMMON_H_
+
+#include <sound/soc.h>
+#include <linux/clk.h>
+#include <linux/list.h>
+#include <linux/regmap.h>
+#include "../common/mtk-base-afe.h"
+
+enum {
+ MT7986_MEMIF_DL1,
+ MT7986_MEMIF_VUL12,
+ MT7986_MEMIF_NUM,
+ MT7986_DAI_ETDM = MT7986_MEMIF_NUM,
+ MT7986_DAI_NUM,
+};
+
+enum {
+ MT7986_IRQ_0,
+ MT7986_IRQ_1,
+ MT7986_IRQ_2,
+ MT7986_IRQ_NUM,
+};
+
+struct mt7986_afe_private {
+ struct clk_bulk_data *clks;
+ int num_clks;
+
+ int pm_runtime_bypass_reg_ctl;
+
+ /* dai */
+ void *dai_priv[MT7986_DAI_NUM];
+};
+
+unsigned int mt7986_afe_rate_transform(struct device *dev,
+ unsigned int rate);
+
+/* dai register */
+int mt7986_dai_etdm_register(struct mtk_base_afe *afe);
+#endif
diff --git a/sound/soc/mediatek/mt7986/mt7986-reg.h b/sound/soc/mediatek/mt7986/mt7986-reg.h
new file mode 100644
index 000000000000..88861f81890f
--- /dev/null
+++ b/sound/soc/mediatek/mt7986/mt7986-reg.h
@@ -0,0 +1,206 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * mt7986-reg.h -- MediaTek 7986 audio driver reg definition
+ *
+ * Copyright (c) 2021 MediaTek Inc.
+ * Author: Vic Wu <vic.wu@mediatek.com>
+ * Maso Huang <maso.huang@mediatek.com>
+ */
+
+#ifndef _MT7986_REG_H_
+#define _MT7986_REG_H_
+
+#define AUDIO_TOP_CON2 0x0008
+#define AUDIO_TOP_CON4 0x0010
+#define AUDIO_ENGEN_CON0 0x0014
+#define AFE_IRQ_MCU_EN 0x0100
+#define AFE_IRQ_MCU_STATUS 0x0120
+#define AFE_IRQ_MCU_CLR 0x0128
+#define AFE_IRQ0_MCU_CFG0 0x0140
+#define AFE_IRQ0_MCU_CFG1 0x0144
+#define AFE_IRQ1_MCU_CFG0 0x0148
+#define AFE_IRQ1_MCU_CFG1 0x014c
+#define AFE_IRQ2_MCU_CFG0 0x0150
+#define AFE_IRQ2_MCU_CFG1 0x0154
+#define ETDM_IN5_CON0 0x13f0
+#define ETDM_IN5_CON1 0x13f4
+#define ETDM_IN5_CON2 0x13f8
+#define ETDM_IN5_CON3 0x13fc
+#define ETDM_IN5_CON4 0x1400
+#define ETDM_OUT5_CON0 0x1570
+#define ETDM_OUT5_CON4 0x1580
+#define ETDM_OUT5_CON5 0x1584
+#define ETDM_4_7_COWORK_CON0 0x15e0
+#define ETDM_4_7_COWORK_CON1 0x15e4
+#define AFE_CONN018_1 0x1b44
+#define AFE_CONN018_4 0x1b50
+#define AFE_CONN019_1 0x1b64
+#define AFE_CONN019_4 0x1b70
+#define AFE_CONN124_1 0x2884
+#define AFE_CONN124_4 0x2890
+#define AFE_CONN125_1 0x28a4
+#define AFE_CONN125_4 0x28b0
+#define AFE_CONN_RS_0 0x3920
+#define AFE_CONN_RS_3 0x392c
+#define AFE_CONN_16BIT_0 0x3960
+#define AFE_CONN_16BIT_3 0x396c
+#define AFE_CONN_24BIT_0 0x3980
+#define AFE_CONN_24BIT_3 0x398c
+#define AFE_MEMIF_CON0 0x3d98
+#define AFE_MEMIF_RD_MON 0x3da0
+#define AFE_MEMIF_WR_MON 0x3da4
+#define AFE_DL0_BASE_MSB 0x3e40
+#define AFE_DL0_BASE 0x3e44
+#define AFE_DL0_CUR_MSB 0x3e48
+#define AFE_DL0_CUR 0x3e4c
+#define AFE_DL0_END_MSB 0x3e50
+#define AFE_DL0_END 0x3e54
+#define AFE_DL0_RCH_MON 0x3e58
+#define AFE_DL0_LCH_MON 0x3e5c
+#define AFE_DL0_CON0 0x3e60
+#define AFE_VUL0_BASE_MSB 0x4220
+#define AFE_VUL0_BASE 0x4224
+#define AFE_VUL0_CUR_MSB 0x4228
+#define AFE_VUL0_CUR 0x422c
+#define AFE_VUL0_END_MSB 0x4230
+#define AFE_VUL0_END 0x4234
+#define AFE_VUL0_CON0 0x4238
+
+#define AFE_MAX_REGISTER AFE_VUL0_CON0
+#define AFE_IRQ_STATUS_BITS 0x7
+#define AFE_IRQ_CNT_SHIFT 0
+#define AFE_IRQ_CNT_MASK 0xffffff
+
+/* AUDIO_TOP_CON2 */
+#define CLK_OUT5_PDN BIT(14)
+#define CLK_OUT5_PDN_MASK BIT(14)
+#define CLK_IN5_PDN BIT(7)
+#define CLK_IN5_PDN_MASK BIT(7)
+
+/* AUDIO_TOP_CON4 */
+#define PDN_APLL_TUNER2 BIT(12)
+#define PDN_APLL_TUNER2_MASK BIT(12)
+
+/* AUDIO_ENGEN_CON0 */
+#define AUD_APLL2_EN BIT(3)
+#define AUD_APLL2_EN_MASK BIT(3)
+#define AUD_26M_EN BIT(0)
+#define AUD_26M_EN_MASK BIT(0)
+
+/* AFE_DL0_CON0 */
+#define DL0_ON_SFT 28
+#define DL0_ON_MASK 0x1
+#define DL0_ON_MASK_SFT BIT(28)
+#define DL0_MINLEN_SFT 20
+#define DL0_MINLEN_MASK 0xf
+#define DL0_MINLEN_MASK_SFT (0xf << 20)
+#define DL0_MODE_SFT 8
+#define DL0_MODE_MASK 0x1f
+#define DL0_MODE_MASK_SFT (0x1f << 8)
+#define DL0_PBUF_SIZE_SFT 5
+#define DL0_PBUF_SIZE_MASK 0x3
+#define DL0_PBUF_SIZE_MASK_SFT (0x3 << 5)
+#define DL0_MONO_SFT 4
+#define DL0_MONO_MASK 0x1
+#define DL0_MONO_MASK_SFT BIT(4)
+#define DL0_HALIGN_SFT 2
+#define DL0_HALIGN_MASK 0x1
+#define DL0_HALIGN_MASK_SFT BIT(2)
+#define DL0_HD_MODE_SFT 0
+#define DL0_HD_MODE_MASK 0x3
+#define DL0_HD_MODE_MASK_SFT (0x3 << 0)
+
+/* AFE_VUL0_CON0 */
+#define VUL0_ON_SFT 28
+#define VUL0_ON_MASK 0x1
+#define VUL0_ON_MASK_SFT BIT(28)
+#define VUL0_MODE_SFT 8
+#define VUL0_MODE_MASK 0x1f
+#define VUL0_MODE_MASK_SFT (0x1f << 8)
+#define VUL0_MONO_SFT 4
+#define VUL0_MONO_MASK 0x1
+#define VUL0_MONO_MASK_SFT BIT(4)
+#define VUL0_HALIGN_SFT 2
+#define VUL0_HALIGN_MASK 0x1
+#define VUL0_HALIGN_MASK_SFT BIT(2)
+#define VUL0_HD_MODE_SFT 0
+#define VUL0_HD_MODE_MASK 0x3
+#define VUL0_HD_MODE_MASK_SFT (0x3 << 0)
+
+/* AFE_IRQ_MCU_CON */
+#define IRQ_MCU_MODE_SFT 4
+#define IRQ_MCU_MODE_MASK 0x1f
+#define IRQ_MCU_MODE_MASK_SFT (0x1f << 4)
+#define IRQ_MCU_ON_SFT 0
+#define IRQ_MCU_ON_MASK 0x1
+#define IRQ_MCU_ON_MASK_SFT BIT(0)
+#define IRQ0_MCU_CLR_SFT 0
+#define IRQ0_MCU_CLR_MASK 0x1
+#define IRQ0_MCU_CLR_MASK_SFT BIT(0)
+#define IRQ1_MCU_CLR_SFT 1
+#define IRQ1_MCU_CLR_MASK 0x1
+#define IRQ1_MCU_CLR_MASK_SFT BIT(1)
+#define IRQ2_MCU_CLR_SFT 2
+#define IRQ2_MCU_CLR_MASK 0x1
+#define IRQ2_MCU_CLR_MASK_SFT BIT(2)
+
+/* ETDM_IN5_CON2 */
+#define IN_CLK_SRC(x) ((x) << 10)
+#define IN_CLK_SRC_SFT 10
+#define IN_CLK_SRC_MASK GENMASK(12, 10)
+
+/* ETDM_IN5_CON3 */
+#define IN_SEL_FS(x) ((x) << 26)
+#define IN_SEL_FS_SFT 26
+#define IN_SEL_FS_MASK GENMASK(30, 26)
+
+/* ETDM_IN5_CON4 */
+#define IN_RELATCH(x) ((x) << 20)
+#define IN_RELATCH_SFT 20
+#define IN_RELATCH_MASK GENMASK(24, 20)
+#define IN_CLK_INV BIT(18)
+#define IN_CLK_INV_MASK BIT(18)
+
+/* ETDM_IN5_CON0 & ETDM_OUT5_CON0 */
+#define RELATCH_SRC(x) ((x) << 28)
+#define RELATCH_SRC_SFT 28
+#define RELATCH_SRC_MASK GENMASK(30, 28)
+#define ETDM_CH_NUM(x) (((x) - 1) << 23)
+#define ETDM_CH_NUM_SFT 23
+#define ETDM_CH_NUM_MASK GENMASK(27, 23)
+#define ETDM_WRD_LEN(x) (((x) - 1) << 16)
+#define ETDM_WRD_LEN_SFT 16
+#define ETDM_WRD_LEN_MASK GENMASK(20, 16)
+#define ETDM_BIT_LEN(x) (((x) - 1) << 11)
+#define ETDM_BIT_LEN_SFT 11
+#define ETDM_BIT_LEN_MASK GENMASK(15, 11)
+#define ETDM_FMT(x) ((x) << 6)
+#define ETDM_FMT_SFT 6
+#define ETDM_FMT_MASK GENMASK(8, 6)
+#define ETDM_SYNC BIT(1)
+#define ETDM_SYNC_MASK BIT(1)
+#define ETDM_EN BIT(0)
+#define ETDM_EN_MASK BIT(0)
+
+/* ETDM_OUT5_CON4 */
+#define OUT_RELATCH(x) ((x) << 24)
+#define OUT_RELATCH_SFT 24
+#define OUT_RELATCH_MASK GENMASK(28, 24)
+#define OUT_CLK_SRC(x) ((x) << 6)
+#define OUT_CLK_SRC_SFT 6
+#define OUT_CLK_SRC_MASK GENMASK(8, 6)
+#define OUT_SEL_FS(x) (x)
+#define OUT_SEL_FS_SFT 0
+#define OUT_SEL_FS_MASK GENMASK(4, 0)
+
+/* ETDM_OUT5_CON5 */
+#define ETDM_CLK_DIV BIT(12)
+#define ETDM_CLK_DIV_MASK BIT(12)
+#define OUT_CLK_INV BIT(9)
+#define OUT_CLK_INV_MASK BIT(9)
+
+/* ETDM_4_7_COWORK_CON0 */
+#define OUT_SEL(x) ((x) << 12)
+#define OUT_SEL_SFT 12
+#define OUT_SEL_MASK GENMASK(15, 12)
+#endif
--
2.18.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 1/6] ASoC: mediatek: mt7986: add common header
2023-07-28 9:08 ` [PATCH v3 1/6] ASoC: mediatek: mt7986: add common header Maso Huang
@ 2023-08-04 9:15 ` Alexandre Mergnat
2023-08-04 10:24 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: Alexandre Mergnat @ 2023-08-04 9:15 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Jaroslav Kysela, Takashi Iwai,
Trevor Wu, Arnd Bergmann, Mars Chen, Allen-KH Cheng, alsa-devel,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
On 28/07/2023 11:08, Maso Huang wrote:
> Add header files for register definition and structure.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> sound/soc/mediatek/mt7986/mt7986-afe-common.h | 49 +++++
> sound/soc/mediatek/mt7986/mt7986-reg.h | 206 ++++++++++++++++++
> 2 files changed, 255 insertions(+)
> create mode 100644 sound/soc/mediatek/mt7986/mt7986-afe-common.h
> create mode 100644 sound/soc/mediatek/mt7986/mt7986-reg.h
>
> diff --git a/sound/soc/mediatek/mt7986/mt7986-afe-common.h b/sound/soc/mediatek/mt7986/mt7986-afe-common.h
> new file mode 100644
> index 000000000000..1c59549d91b4
> --- /dev/null
> +++ b/sound/soc/mediatek/mt7986/mt7986-afe-common.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * mt7986-afe-common.h -- MediaTek 7986 audio driver definitions
> + *
> + * Copyright (c) 2021 MediaTek Inc.
2023
> + * Author: Vic Wu <vic.wu@mediatek.com>
Authors
> + * Maso Huang <maso.huang@mediatek.com>
> + */
> +
> +#ifndef _MT_7986_AFE_COMMON_H_
> +#define _MT_7986_AFE_COMMON_H_
> +
> +#include <sound/soc.h>
> +#include <linux/clk.h>
> +#include <linux/list.h>
> +#include <linux/regmap.h>
> +#include "../common/mtk-base-afe.h"
> +
> +enum {
> + MT7986_MEMIF_DL1,
> + MT7986_MEMIF_VUL12,
> + MT7986_MEMIF_NUM,
> + MT7986_DAI_ETDM = MT7986_MEMIF_NUM,
> + MT7986_DAI_NUM,
> +};
> +
> +enum {
> + MT7986_IRQ_0,
> + MT7986_IRQ_1,
> + MT7986_IRQ_2,
> + MT7986_IRQ_NUM,
> +};
> +
> +struct mt7986_afe_private {
> + struct clk_bulk_data *clks;
> + int num_clks;
> +
> + int pm_runtime_bypass_reg_ctl;
> +
> + /* dai */
> + void *dai_priv[MT7986_DAI_NUM];
> +};
> +
> +unsigned int mt7986_afe_rate_transform(struct device *dev,
> + unsigned int rate);
> +
> +/* dai register */
> +int mt7986_dai_etdm_register(struct mtk_base_afe *afe);
> +#endif
> diff --git a/sound/soc/mediatek/mt7986/mt7986-reg.h b/sound/soc/mediatek/mt7986/mt7986-reg.h
> new file mode 100644
> index 000000000000..88861f81890f
> --- /dev/null
> +++ b/sound/soc/mediatek/mt7986/mt7986-reg.h
> @@ -0,0 +1,206 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * mt7986-reg.h -- MediaTek 7986 audio driver reg definition
> + *
> + * Copyright (c) 2021 MediaTek Inc.
Ditto
> + * Author: Vic Wu <vic.wu@mediatek.com>
Ditto
> + * Maso Huang <maso.huang@mediatek.com>
> + */
> +
> +#ifndef _MT7986_REG_H_
> +#define _MT7986_REG_H_
> +
> +#define AUDIO_TOP_CON2 0x0008
> +#define AUDIO_TOP_CON4 0x0010
> +#define AUDIO_ENGEN_CON0 0x0014
> +#define AFE_IRQ_MCU_EN 0x0100
> +#define AFE_IRQ_MCU_STATUS 0x0120
> +#define AFE_IRQ_MCU_CLR 0x0128
> +#define AFE_IRQ0_MCU_CFG0 0x0140
> +#define AFE_IRQ0_MCU_CFG1 0x0144
> +#define AFE_IRQ1_MCU_CFG0 0x0148
> +#define AFE_IRQ1_MCU_CFG1 0x014c
> +#define AFE_IRQ2_MCU_CFG0 0x0150
> +#define AFE_IRQ2_MCU_CFG1 0x0154
> +#define ETDM_IN5_CON0 0x13f0
> +#define ETDM_IN5_CON1 0x13f4
> +#define ETDM_IN5_CON2 0x13f8
> +#define ETDM_IN5_CON3 0x13fc
> +#define ETDM_IN5_CON4 0x1400
> +#define ETDM_OUT5_CON0 0x1570
> +#define ETDM_OUT5_CON4 0x1580
> +#define ETDM_OUT5_CON5 0x1584
> +#define ETDM_4_7_COWORK_CON0 0x15e0
> +#define ETDM_4_7_COWORK_CON1 0x15e4
> +#define AFE_CONN018_1 0x1b44
> +#define AFE_CONN018_4 0x1b50
> +#define AFE_CONN019_1 0x1b64
> +#define AFE_CONN019_4 0x1b70
> +#define AFE_CONN124_1 0x2884
> +#define AFE_CONN124_4 0x2890
> +#define AFE_CONN125_1 0x28a4
> +#define AFE_CONN125_4 0x28b0
> +#define AFE_CONN_RS_0 0x3920
> +#define AFE_CONN_RS_3 0x392c
> +#define AFE_CONN_16BIT_0 0x3960
> +#define AFE_CONN_16BIT_3 0x396c
> +#define AFE_CONN_24BIT_0 0x3980
> +#define AFE_CONN_24BIT_3 0x398c
> +#define AFE_MEMIF_CON0 0x3d98
> +#define AFE_MEMIF_RD_MON 0x3da0
> +#define AFE_MEMIF_WR_MON 0x3da4
> +#define AFE_DL0_BASE_MSB 0x3e40
> +#define AFE_DL0_BASE 0x3e44
> +#define AFE_DL0_CUR_MSB 0x3e48
> +#define AFE_DL0_CUR 0x3e4c
> +#define AFE_DL0_END_MSB 0x3e50
> +#define AFE_DL0_END 0x3e54
> +#define AFE_DL0_RCH_MON 0x3e58
> +#define AFE_DL0_LCH_MON 0x3e5c
> +#define AFE_DL0_CON0 0x3e60
> +#define AFE_VUL0_BASE_MSB 0x4220
> +#define AFE_VUL0_BASE 0x4224
> +#define AFE_VUL0_CUR_MSB 0x4228
> +#define AFE_VUL0_CUR 0x422c
> +#define AFE_VUL0_END_MSB 0x4230
> +#define AFE_VUL0_END 0x4234
> +#define AFE_VUL0_CON0 0x4238
> +
> +#define AFE_MAX_REGISTER AFE_VUL0_CON0
> +#define AFE_IRQ_STATUS_BITS 0x7
> +#define AFE_IRQ_CNT_SHIFT 0
> +#define AFE_IRQ_CNT_MASK 0xffffff
> +
> +/* AUDIO_TOP_CON2 */
> +#define CLK_OUT5_PDN BIT(14)
> +#define CLK_OUT5_PDN_MASK BIT(14)
> +#define CLK_IN5_PDN BIT(7)
> +#define CLK_IN5_PDN_MASK BIT(7)
> +
> +/* AUDIO_TOP_CON4 */
> +#define PDN_APLL_TUNER2 BIT(12)
> +#define PDN_APLL_TUNER2_MASK BIT(12)
> +
> +/* AUDIO_ENGEN_CON0 */
> +#define AUD_APLL2_EN BIT(3)
> +#define AUD_APLL2_EN_MASK BIT(3)
> +#define AUD_26M_EN BIT(0)
> +#define AUD_26M_EN_MASK BIT(0)
> +
> +/* AFE_DL0_CON0 */
> +#define DL0_ON_SFT 28
> +#define DL0_ON_MASK 0x1
> +#define DL0_ON_MASK_SFT BIT(28)
> +#define DL0_MINLEN_SFT 20
> +#define DL0_MINLEN_MASK 0xf
> +#define DL0_MINLEN_MASK_SFT (0xf << 20)
> +#define DL0_MODE_SFT 8
> +#define DL0_MODE_MASK 0x1f
> +#define DL0_MODE_MASK_SFT (0x1f << 8)
> +#define DL0_PBUF_SIZE_SFT 5
> +#define DL0_PBUF_SIZE_MASK 0x3
> +#define DL0_PBUF_SIZE_MASK_SFT (0x3 << 5)
> +#define DL0_MONO_SFT 4
> +#define DL0_MONO_MASK 0x1
> +#define DL0_MONO_MASK_SFT BIT(4)
> +#define DL0_HALIGN_SFT 2
> +#define DL0_HALIGN_MASK 0x1
> +#define DL0_HALIGN_MASK_SFT BIT(2)
> +#define DL0_HD_MODE_SFT 0
> +#define DL0_HD_MODE_MASK 0x3
> +#define DL0_HD_MODE_MASK_SFT (0x3 << 0)
> +
> +/* AFE_VUL0_CON0 */
> +#define VUL0_ON_SFT 28
> +#define VUL0_ON_MASK 0x1
> +#define VUL0_ON_MASK_SFT BIT(28)
> +#define VUL0_MODE_SFT 8
> +#define VUL0_MODE_MASK 0x1f
> +#define VUL0_MODE_MASK_SFT (0x1f << 8)
> +#define VUL0_MONO_SFT 4
> +#define VUL0_MONO_MASK 0x1
> +#define VUL0_MONO_MASK_SFT BIT(4)
> +#define VUL0_HALIGN_SFT 2
> +#define VUL0_HALIGN_MASK 0x1
> +#define VUL0_HALIGN_MASK_SFT BIT(2)
> +#define VUL0_HD_MODE_SFT 0
> +#define VUL0_HD_MODE_MASK 0x3
> +#define VUL0_HD_MODE_MASK_SFT (0x3 << 0)
> +
> +/* AFE_IRQ_MCU_CON */
> +#define IRQ_MCU_MODE_SFT 4
> +#define IRQ_MCU_MODE_MASK 0x1f
> +#define IRQ_MCU_MODE_MASK_SFT (0x1f << 4)
> +#define IRQ_MCU_ON_SFT 0
> +#define IRQ_MCU_ON_MASK 0x1
> +#define IRQ_MCU_ON_MASK_SFT BIT(0)
> +#define IRQ0_MCU_CLR_SFT 0
> +#define IRQ0_MCU_CLR_MASK 0x1
> +#define IRQ0_MCU_CLR_MASK_SFT BIT(0)
> +#define IRQ1_MCU_CLR_SFT 1
> +#define IRQ1_MCU_CLR_MASK 0x1
> +#define IRQ1_MCU_CLR_MASK_SFT BIT(1)
> +#define IRQ2_MCU_CLR_SFT 2
> +#define IRQ2_MCU_CLR_MASK 0x1
> +#define IRQ2_MCU_CLR_MASK_SFT BIT(2)
> +
> +/* ETDM_IN5_CON2 */
> +#define IN_CLK_SRC(x) ((x) << 10)
> +#define IN_CLK_SRC_SFT 10
> +#define IN_CLK_SRC_MASK GENMASK(12, 10)
> +
> +/* ETDM_IN5_CON3 */
> +#define IN_SEL_FS(x) ((x) << 26)
> +#define IN_SEL_FS_SFT 26
> +#define IN_SEL_FS_MASK GENMASK(30, 26)
> +
> +/* ETDM_IN5_CON4 */
> +#define IN_RELATCH(x) ((x) << 20)
> +#define IN_RELATCH_SFT 20
> +#define IN_RELATCH_MASK GENMASK(24, 20)
> +#define IN_CLK_INV BIT(18)
> +#define IN_CLK_INV_MASK BIT(18)
> +
> +/* ETDM_IN5_CON0 & ETDM_OUT5_CON0 */
> +#define RELATCH_SRC(x) ((x) << 28)
> +#define RELATCH_SRC_SFT 28
> +#define RELATCH_SRC_MASK GENMASK(30, 28)
> +#define ETDM_CH_NUM(x) (((x) - 1) << 23)
> +#define ETDM_CH_NUM_SFT 23
> +#define ETDM_CH_NUM_MASK GENMASK(27, 23)
> +#define ETDM_WRD_LEN(x) (((x) - 1) << 16)
> +#define ETDM_WRD_LEN_SFT 16
> +#define ETDM_WRD_LEN_MASK GENMASK(20, 16)
> +#define ETDM_BIT_LEN(x) (((x) - 1) << 11)
> +#define ETDM_BIT_LEN_SFT 11
> +#define ETDM_BIT_LEN_MASK GENMASK(15, 11)
> +#define ETDM_FMT(x) ((x) << 6)
> +#define ETDM_FMT_SFT 6
> +#define ETDM_FMT_MASK GENMASK(8, 6)
> +#define ETDM_SYNC BIT(1)
> +#define ETDM_SYNC_MASK BIT(1)
> +#define ETDM_EN BIT(0)
> +#define ETDM_EN_MASK BIT(0)
> +
> +/* ETDM_OUT5_CON4 */
> +#define OUT_RELATCH(x) ((x) << 24)
> +#define OUT_RELATCH_SFT 24
> +#define OUT_RELATCH_MASK GENMASK(28, 24)
> +#define OUT_CLK_SRC(x) ((x) << 6)
> +#define OUT_CLK_SRC_SFT 6
> +#define OUT_CLK_SRC_MASK GENMASK(8, 6)
> +#define OUT_SEL_FS(x) (x)
> +#define OUT_SEL_FS_SFT 0
> +#define OUT_SEL_FS_MASK GENMASK(4, 0)
> +
> +/* ETDM_OUT5_CON5 */
> +#define ETDM_CLK_DIV BIT(12)
> +#define ETDM_CLK_DIV_MASK BIT(12)
> +#define OUT_CLK_INV BIT(9)
> +#define OUT_CLK_INV_MASK BIT(9)
> +
> +/* ETDM_4_7_COWORK_CON0 */
> +#define OUT_SEL(x) ((x) << 12)
> +#define OUT_SEL_SFT 12
> +#define OUT_SEL_MASK GENMASK(15, 12)
> +#endif
--
Regards,
Alexandre
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 1/6] ASoC: mediatek: mt7986: add common header
2023-08-04 9:15 ` Alexandre Mergnat
@ 2023-08-04 10:24 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-08-04 10:24 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, amergnat@baylibre.com,
lgirdwood@gmail.com, linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-08-04 at 11:15 +0200, Alexandre Mergnat wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On 28/07/2023 11:08, Maso Huang wrote:
> > Add header files for register definition and structure.
> >
> > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> > ---
> > sound/soc/mediatek/mt7986/mt7986-afe-common.h | 49 +++++
> > sound/soc/mediatek/mt7986/mt7986-reg.h | 206
> ++++++++++++++++++
> > 2 files changed, 255 insertions(+)
> > create mode 100644 sound/soc/mediatek/mt7986/mt7986-afe-common.h
> > create mode 100644 sound/soc/mediatek/mt7986/mt7986-reg.h
> >
> > diff --git a/sound/soc/mediatek/mt7986/mt7986-afe-common.h
> b/sound/soc/mediatek/mt7986/mt7986-afe-common.h
> > new file mode 100644
> > index 000000000000..1c59549d91b4
> > --- /dev/null
> > +++ b/sound/soc/mediatek/mt7986/mt7986-afe-common.h
> > @@ -0,0 +1,49 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * mt7986-afe-common.h -- MediaTek 7986 audio driver definitions
> > + *
> > + * Copyright (c) 2021 MediaTek Inc.
>
> 2023
>
> > + * Author: Vic Wu <vic.wu@mediatek.com>
>
> Authors
>
> > + * Maso Huang <maso.huang@mediatek.com>
> > + */
> > +
> > +#ifndef _MT_7986_AFE_COMMON_H_
> > +#define _MT_7986_AFE_COMMON_H_
> > +
> > +#include <sound/soc.h>
> > +#include <linux/clk.h>
> > +#include <linux/list.h>
> > +#include <linux/regmap.h>
> > +#include "../common/mtk-base-afe.h"
> > +
> > +enum {
> > +MT7986_MEMIF_DL1,
> > +MT7986_MEMIF_VUL12,
> > +MT7986_MEMIF_NUM,
> > +MT7986_DAI_ETDM = MT7986_MEMIF_NUM,
> > +MT7986_DAI_NUM,
> > +};
> > +
> > +enum {
> > +MT7986_IRQ_0,
> > +MT7986_IRQ_1,
> > +MT7986_IRQ_2,
> > +MT7986_IRQ_NUM,
> > +};
> > +
> > +struct mt7986_afe_private {
> > +struct clk_bulk_data *clks;
> > +int num_clks;
> > +
> > +int pm_runtime_bypass_reg_ctl;
> > +
> > +/* dai */
> > +void *dai_priv[MT7986_DAI_NUM];
> > +};
> > +
> > +unsigned int mt7986_afe_rate_transform(struct device *dev,
> > + unsigned int rate);
> > +
> > +/* dai register */
> > +int mt7986_dai_etdm_register(struct mtk_base_afe *afe);
> > +#endif
> > diff --git a/sound/soc/mediatek/mt7986/mt7986-reg.h
> b/sound/soc/mediatek/mt7986/mt7986-reg.h
> > new file mode 100644
> > index 000000000000..88861f81890f
> > --- /dev/null
> > +++ b/sound/soc/mediatek/mt7986/mt7986-reg.h
> > @@ -0,0 +1,206 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * mt7986-reg.h -- MediaTek 7986 audio driver reg definition
> > + *
> > + * Copyright (c) 2021 MediaTek Inc.
>
> Ditto
>
> > + * Author: Vic Wu <vic.wu@mediatek.com>
>
> Ditto
>
> > + * Maso Huang <maso.huang@mediatek.com>
> > + */
> > +
> > +#ifndef _MT7986_REG_H_
> > +#define _MT7986_REG_H_
> > +
> > +#define AUDIO_TOP_CON2 0x0008
> > +#define AUDIO_TOP_CON4 0x0010
> > +#define AUDIO_ENGEN_CON0 0x0014
> > +#define AFE_IRQ_MCU_EN 0x0100
> > +#define AFE_IRQ_MCU_STATUS 0x0120
> > +#define AFE_IRQ_MCU_CLR 0x0128
> > +#define AFE_IRQ0_MCU_CFG0 0x0140
> > +#define AFE_IRQ0_MCU_CFG1 0x0144
> > +#define AFE_IRQ1_MCU_CFG0 0x0148
> > +#define AFE_IRQ1_MCU_CFG1 0x014c
> > +#define AFE_IRQ2_MCU_CFG0 0x0150
> > +#define AFE_IRQ2_MCU_CFG1 0x0154
> > +#define ETDM_IN5_CON0 0x13f0
> > +#define ETDM_IN5_CON1 0x13f4
> > +#define ETDM_IN5_CON2 0x13f8
> > +#define ETDM_IN5_CON3 0x13fc
> > +#define ETDM_IN5_CON4 0x1400
> > +#define ETDM_OUT5_CON0 0x1570
> > +#define ETDM_OUT5_CON4 0x1580
> > +#define ETDM_OUT5_CON5 0x1584
> > +#define ETDM_4_7_COWORK_CON0 0x15e0
> > +#define ETDM_4_7_COWORK_CON1 0x15e4
> > +#define AFE_CONN018_1 0x1b44
> > +#define AFE_CONN018_4 0x1b50
> > +#define AFE_CONN019_1 0x1b64
> > +#define AFE_CONN019_4 0x1b70
> > +#define AFE_CONN124_1 0x2884
> > +#define AFE_CONN124_4 0x2890
> > +#define AFE_CONN125_1 0x28a4
> > +#define AFE_CONN125_4 0x28b0
> > +#define AFE_CONN_RS_0 0x3920
> > +#define AFE_CONN_RS_3 0x392c
> > +#define AFE_CONN_16BIT_0 0x3960
> > +#define AFE_CONN_16BIT_3 0x396c
> > +#define AFE_CONN_24BIT_0 0x3980
> > +#define AFE_CONN_24BIT_3 0x398c
> > +#define AFE_MEMIF_CON0 0x3d98
> > +#define AFE_MEMIF_RD_MON 0x3da0
> > +#define AFE_MEMIF_WR_MON 0x3da4
> > +#define AFE_DL0_BASE_MSB 0x3e40
> > +#define AFE_DL0_BASE 0x3e44
> > +#define AFE_DL0_CUR_MSB 0x3e48
> > +#define AFE_DL0_CUR 0x3e4c
> > +#define AFE_DL0_END_MSB 0x3e50
> > +#define AFE_DL0_END 0x3e54
> > +#define AFE_DL0_RCH_MON 0x3e58
> > +#define AFE_DL0_LCH_MON 0x3e5c
> > +#define AFE_DL0_CON0 0x3e60
> > +#define AFE_VUL0_BASE_MSB 0x4220
> > +#define AFE_VUL0_BASE 0x4224
> > +#define AFE_VUL0_CUR_MSB 0x4228
> > +#define AFE_VUL0_CUR 0x422c
> > +#define AFE_VUL0_END_MSB 0x4230
> > +#define AFE_VUL0_END 0x4234
> > +#define AFE_VUL0_CON0 0x4238
> > +
> > +#define AFE_MAX_REGISTER AFE_VUL0_CON0
> > +#define AFE_IRQ_STATUS_BITS 0x7
> > +#define AFE_IRQ_CNT_SHIFT 0
> > +#define AFE_IRQ_CNT_MASK 0xffffff
> > +
> > +/* AUDIO_TOP_CON2 */
> > +#define CLK_OUT5_PDN BIT(14)
> > +#define CLK_OUT5_PDN_MASK BIT(14)
> > +#define CLK_IN5_PDN BIT(7)
> > +#define CLK_IN5_PDN_MASK BIT(7)
> > +
> > +/* AUDIO_TOP_CON4 */
> > +#define PDN_APLL_TUNER2 BIT(12)
> > +#define PDN_APLL_TUNER2_MASK BIT(12)
> > +
> > +/* AUDIO_ENGEN_CON0 */
> > +#define AUD_APLL2_EN BIT(3)
> > +#define AUD_APLL2_EN_MASK BIT(3)
> > +#define AUD_26M_EN BIT(0)
> > +#define AUD_26M_EN_MASK BIT(0)
> > +
> > +/* AFE_DL0_CON0 */
> > +#define DL0_ON_SFT 28
> > +#define DL0_ON_MASK 0x1
> > +#define DL0_ON_MASK_SFT BIT(28)
> > +#define DL0_MINLEN_SFT 20
> > +#define DL0_MINLEN_MASK 0xf
> > +#define DL0_MINLEN_MASK_SFT (0xf << 20)
> > +#define DL0_MODE_SFT 8
> > +#define DL0_MODE_MASK 0x1f
> > +#define DL0_MODE_MASK_SFT (0x1f << 8)
> > +#define DL0_PBUF_SIZE_SFT 5
> > +#define DL0_PBUF_SIZE_MASK 0x3
> > +#define DL0_PBUF_SIZE_MASK_SFT (0x3 << 5)
> > +#define DL0_MONO_SFT 4
> > +#define DL0_MONO_MASK 0x1
> > +#define DL0_MONO_MASK_SFT BIT(4)
> > +#define DL0_HALIGN_SFT 2
> > +#define DL0_HALIGN_MASK 0x1
> > +#define DL0_HALIGN_MASK_SFT BIT(2)
> > +#define DL0_HD_MODE_SFT 0
> > +#define DL0_HD_MODE_MASK 0x3
> > +#define DL0_HD_MODE_MASK_SFT (0x3 << 0)
> > +
> > +/* AFE_VUL0_CON0 */
> > +#define VUL0_ON_SFT 28
> > +#define VUL0_ON_MASK 0x1
> > +#define VUL0_ON_MASK_SFT BIT(28)
> > +#define VUL0_MODE_SFT 8
> > +#define VUL0_MODE_MASK 0x1f
> > +#define VUL0_MODE_MASK_SFT (0x1f << 8)
> > +#define VUL0_MONO_SFT 4
> > +#define VUL0_MONO_MASK 0x1
> > +#define VUL0_MONO_MASK_SFT BIT(4)
> > +#define VUL0_HALIGN_SFT 2
> > +#define VUL0_HALIGN_MASK 0x1
> > +#define VUL0_HALIGN_MASK_SFT BIT(2)
> > +#define VUL0_HD_MODE_SFT 0
> > +#define VUL0_HD_MODE_MASK 0x3
> > +#define VUL0_HD_MODE_MASK_SFT (0x3 << 0)
> > +
> > +/* AFE_IRQ_MCU_CON */
> > +#define IRQ_MCU_MODE_SFT 4
> > +#define IRQ_MCU_MODE_MASK 0x1f
> > +#define IRQ_MCU_MODE_MASK_SFT (0x1f << 4)
> > +#define IRQ_MCU_ON_SFT 0
> > +#define IRQ_MCU_ON_MASK 0x1
> > +#define IRQ_MCU_ON_MASK_SFT BIT(0)
> > +#define IRQ0_MCU_CLR_SFT 0
> > +#define IRQ0_MCU_CLR_MASK 0x1
> > +#define IRQ0_MCU_CLR_MASK_SFT BIT(0)
> > +#define IRQ1_MCU_CLR_SFT 1
> > +#define IRQ1_MCU_CLR_MASK 0x1
> > +#define IRQ1_MCU_CLR_MASK_SFT BIT(1)
> > +#define IRQ2_MCU_CLR_SFT 2
> > +#define IRQ2_MCU_CLR_MASK 0x1
> > +#define IRQ2_MCU_CLR_MASK_SFT BIT(2)
> > +
> > +/* ETDM_IN5_CON2 */
> > +#define IN_CLK_SRC(x) ((x) << 10)
> > +#define IN_CLK_SRC_SFT 10
> > +#define IN_CLK_SRC_MASK GENMASK(12, 10)
> > +
> > +/* ETDM_IN5_CON3 */
> > +#define IN_SEL_FS(x) ((x) << 26)
> > +#define IN_SEL_FS_SFT 26
> > +#define IN_SEL_FS_MASK GENMASK(30, 26)
> > +
> > +/* ETDM_IN5_CON4 */
> > +#define IN_RELATCH(x) ((x) << 20)
> > +#define IN_RELATCH_SFT 20
> > +#define IN_RELATCH_MASK GENMASK(24, 20)
> > +#define IN_CLK_INV BIT(18)
> > +#define IN_CLK_INV_MASK BIT(18)
> > +
> > +/* ETDM_IN5_CON0 & ETDM_OUT5_CON0 */
> > +#define RELATCH_SRC(x) ((x) << 28)
> > +#define RELATCH_SRC_SFT 28
> > +#define RELATCH_SRC_MASK GENMASK(30, 28)
> > +#define ETDM_CH_NUM(x) (((x) - 1) << 23)
> > +#define ETDM_CH_NUM_SFT 23
> > +#define ETDM_CH_NUM_MASK GENMASK(27, 23)
> > +#define ETDM_WRD_LEN(x) (((x) - 1) << 16)
> > +#define ETDM_WRD_LEN_SFT 16
> > +#define ETDM_WRD_LEN_MASK GENMASK(20, 16)
> > +#define ETDM_BIT_LEN(x) (((x) - 1) << 11)
> > +#define ETDM_BIT_LEN_SFT 11
> > +#define ETDM_BIT_LEN_MASK GENMASK(15, 11)
> > +#define ETDM_FMT(x) ((x) << 6)
> > +#define ETDM_FMT_SFT 6
> > +#define ETDM_FMT_MASK GENMASK(8, 6)
> > +#define ETDM_SYNC BIT(1)
> > +#define ETDM_SYNC_MASK BIT(1)
> > +#define ETDM_EN BIT(0)
> > +#define ETDM_EN_MASK BIT(0)
> > +
> > +/* ETDM_OUT5_CON4 */
> > +#define OUT_RELATCH(x) ((x) << 24)
> > +#define OUT_RELATCH_SFT 24
> > +#define OUT_RELATCH_MASK GENMASK(28, 24)
> > +#define OUT_CLK_SRC(x) ((x) << 6)
> > +#define OUT_CLK_SRC_SFT 6
> > +#define OUT_CLK_SRC_MASK GENMASK(8, 6)
> > +#define OUT_SEL_FS(x) (x)
> > +#define OUT_SEL_FS_SFT 0
> > +#define OUT_SEL_FS_MASK GENMASK(4, 0)
> > +
> > +/* ETDM_OUT5_CON5 */
> > +#define ETDM_CLK_DIV BIT(12)
> > +#define ETDM_CLK_DIV_MASK BIT(12)
> > +#define OUT_CLK_INV BIT(9)
> > +#define OUT_CLK_INV_MASK BIT(9)
> > +
> > +/* ETDM_4_7_COWORK_CON0 */
> > +#define OUT_SEL(x) ((x) << 12)
> > +#define OUT_SEL_SFT 12
> > +#define OUT_SEL_MASK GENMASK(15, 12)
> > +#endif
>
> --
> Regards,
> Alexandre
Hi Alexandre,
Thanks for your review.
I'll fix them in v4 patch.
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 2/6] ASoC: mediatek: mt7986: support etdm in platform driver
2023-07-28 9:08 [PATCH v3 0/6] ASoC: mediatek: Add support for MT7986 SoC Maso Huang
2023-07-28 9:08 ` [PATCH v3 1/6] ASoC: mediatek: mt7986: add common header Maso Huang
@ 2023-07-28 9:08 ` Maso Huang
2023-08-04 9:57 ` Alexandre Mergnat
2023-07-28 9:08 ` [PATCH v3 3/6] ASoC: mediatek: mt7986: add " Maso Huang
` (3 subsequent siblings)
5 siblings, 1 reply; 33+ messages in thread
From: Maso Huang @ 2023-07-28 9:08 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Maso Huang
Add mt7986 etdm dai driver support.
Signed-off-by: Maso Huang <maso.huang@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
sound/soc/mediatek/mt7986/mt7986-dai-etdm.c | 420 ++++++++++++++++++++
1 file changed, 420 insertions(+)
create mode 100644 sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
diff --git a/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c b/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
new file mode 100644
index 000000000000..dc094e25ddb4
--- /dev/null
+++ b/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek ALSA SoC Audio DAI eTDM Control
+ *
+ * Copyright (c) 2021 MediaTek Inc.
+ * Author: Vic Wu <vic.wu@mediatek.com>
+ * Maso Huang <maso.huang@mediatek.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/regmap.h>
+#include <sound/pcm_params.h>
+#include "mt7986-afe-common.h"
+#include "mt7986-reg.h"
+
+enum {
+ HOPPING_CLK = 0,
+ APLL_CLK = 1,
+};
+
+enum {
+ MTK_DAI_ETDM_FORMAT_I2S = 0,
+ MTK_DAI_ETDM_FORMAT_DSPA = 4,
+ MTK_DAI_ETDM_FORMAT_DSPB = 5,
+};
+
+enum {
+ ETDM_IN5 = 2,
+ ETDM_OUT5 = 10,
+};
+
+enum {
+ MTK_ETDM_RATE_8K = 0,
+ MTK_ETDM_RATE_12K = 1,
+ MTK_ETDM_RATE_16K = 2,
+ MTK_ETDM_RATE_24K = 3,
+ MTK_ETDM_RATE_32K = 4,
+ MTK_ETDM_RATE_48K = 5,
+ MTK_ETDM_RATE_96K = 7,
+ MTK_ETDM_RATE_192K = 9,
+ MTK_ETDM_RATE_11K = 16,
+ MTK_ETDM_RATE_22K = 17,
+ MTK_ETDM_RATE_44K = 18,
+ MTK_ETDM_RATE_88K = 19,
+ MTK_ETDM_RATE_176K = 20,
+};
+
+struct mtk_dai_etdm_priv {
+ bool bck_inv;
+ bool lrck_inv;
+ bool slave_mode;
+ unsigned int format;
+};
+
+static unsigned int mt7986_etdm_rate_transform(struct device *dev, unsigned int rate)
+{
+ switch (rate) {
+ case 8000:
+ return MTK_ETDM_RATE_8K;
+ case 11025:
+ return MTK_ETDM_RATE_11K;
+ case 12000:
+ return MTK_ETDM_RATE_12K;
+ case 16000:
+ return MTK_ETDM_RATE_16K;
+ case 22050:
+ return MTK_ETDM_RATE_22K;
+ case 24000:
+ return MTK_ETDM_RATE_24K;
+ case 32000:
+ return MTK_ETDM_RATE_32K;
+ case 44100:
+ return MTK_ETDM_RATE_44K;
+ case 48000:
+ return MTK_ETDM_RATE_48K;
+ case 88200:
+ return MTK_ETDM_RATE_88K;
+ case 96000:
+ return MTK_ETDM_RATE_96K;
+ case 176400:
+ return MTK_ETDM_RATE_176K;
+ case 192000:
+ return MTK_ETDM_RATE_192K;
+ default:
+ dev_warn(dev, "%s(), rate %u invalid, using %d!!!\n",
+ __func__, rate, MTK_ETDM_RATE_48K);
+ return MTK_ETDM_RATE_48K;
+ }
+}
+
+static int get_etdm_wlen(unsigned int bitwidth)
+{
+ return bitwidth <= 16 ? 16 : 32;
+}
+
+/* dai component */
+/* interconnection */
+
+static const struct snd_kcontrol_new o124_mix[] = {
+ SOC_DAPM_SINGLE_AUTODISABLE("I032_Switch", AFE_CONN124_1, 0, 1, 0),
+};
+
+static const struct snd_kcontrol_new o125_mix[] = {
+ SOC_DAPM_SINGLE_AUTODISABLE("I033_Switch", AFE_CONN125_1, 1, 1, 0),
+};
+
+static const struct snd_soc_dapm_widget mtk_dai_etdm_widgets[] = {
+
+ /* DL */
+ SND_SOC_DAPM_MIXER("I150", SND_SOC_NOPM, 0, 0, NULL, 0),
+ SND_SOC_DAPM_MIXER("I151", SND_SOC_NOPM, 0, 0, NULL, 0),
+ /* UL */
+ SND_SOC_DAPM_MIXER("O124", SND_SOC_NOPM, 0, 0, o124_mix, ARRAY_SIZE(o124_mix)),
+ SND_SOC_DAPM_MIXER("O125", SND_SOC_NOPM, 0, 0, o125_mix, ARRAY_SIZE(o125_mix)),
+};
+
+static const struct snd_soc_dapm_route mtk_dai_etdm_routes[] = {
+ {"I150", NULL, "ETDM Capture"},
+ {"I151", NULL, "ETDM Capture"},
+ {"ETDM Playback", NULL, "O124"},
+ {"ETDM Playback", NULL, "O125"},
+ {"O124", "I032_Switch", "I032"},
+ {"O125", "I033_Switch", "I033"},
+};
+
+/* dai ops */
+static int mtk_dai_etdm_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+ struct mt7986_afe_private *afe_priv = afe->platform_priv;
+ int ret;
+
+ ret = clk_bulk_prepare_enable(afe_priv->num_clks, afe_priv->clks);
+ if (ret)
+ return dev_err_probe(afe->dev, ret, "Failed to enable clocks\n");
+
+ regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_OUT5_PDN_MASK, 0);
+ regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_IN5_PDN_MASK, 0);
+
+ return 0;
+}
+
+static void mtk_dai_etdm_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+ struct mt7986_afe_private *afe_priv = afe->platform_priv;
+
+ regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_OUT5_PDN_MASK,
+ CLK_OUT5_PDN);
+ regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_IN5_PDN_MASK,
+ CLK_IN5_PDN);
+
+ clk_bulk_disable_unprepare(afe_priv->num_clks, afe_priv->clks);
+}
+
+static unsigned int get_etdm_ch_fixup(unsigned int channels)
+{
+ if (channels > 16)
+ return 24;
+ else if (channels > 8)
+ return 16;
+ else if (channels > 4)
+ return 8;
+ else if (channels > 2)
+ return 4;
+ else
+ return 2;
+}
+
+static int mtk_dai_etdm_config(struct mtk_base_afe *afe,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai,
+ int stream)
+{
+ struct mt7986_afe_private *afe_priv = afe->platform_priv;
+ struct mtk_dai_etdm_priv *etdm_data = afe_priv->dai_priv[dai->id];
+ unsigned int rate = params_rate(params);
+ unsigned int etdm_rate = mt7986_etdm_rate_transform(afe->dev, rate);
+ unsigned int afe_rate = mt7986_afe_rate_transform(afe->dev, rate);
+ unsigned int channels = params_channels(params);
+ unsigned int bit_width = params_width(params);
+ unsigned int wlen = get_etdm_wlen(bit_width);
+ unsigned int val = 0;
+ unsigned int mask = 0;
+
+ dev_dbg(afe->dev, "%s(), stream %d, rate %u, bitwidth %u\n",
+ __func__, stream, rate, bit_width);
+
+ /* CON0 */
+ mask |= ETDM_BIT_LEN_MASK;
+ val |= ETDM_BIT_LEN(bit_width);
+ mask |= ETDM_WRD_LEN_MASK;
+ val |= ETDM_WRD_LEN(wlen);
+ mask |= ETDM_FMT_MASK;
+ val |= ETDM_FMT(etdm_data->format);
+ mask |= ETDM_CH_NUM_MASK;
+ val |= ETDM_CH_NUM(get_etdm_ch_fixup(channels));
+ mask |= RELATCH_SRC_MASK;
+ val |= RELATCH_SRC(APLL_CLK);
+
+ switch (stream) {
+ case SNDRV_PCM_STREAM_PLAYBACK:
+ /* set ETDM_OUT5_CON0 */
+ regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, mask, val);
+
+ /* set ETDM_OUT5_CON4 */
+ regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
+ OUT_RELATCH_MASK, OUT_RELATCH(afe_rate));
+ regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
+ OUT_CLK_SRC_MASK, OUT_CLK_SRC(APLL_CLK));
+ regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
+ OUT_SEL_FS_MASK, OUT_SEL_FS(etdm_rate));
+
+ /* set ETDM_OUT5_CON5 */
+ regmap_update_bits(afe->regmap, ETDM_OUT5_CON5,
+ ETDM_CLK_DIV_MASK, ETDM_CLK_DIV);
+ break;
+ case SNDRV_PCM_STREAM_CAPTURE:
+ /* set ETDM_IN5_CON0 */
+ regmap_update_bits(afe->regmap, ETDM_IN5_CON0, mask, val);
+ regmap_update_bits(afe->regmap, ETDM_IN5_CON0,
+ ETDM_SYNC_MASK, ETDM_SYNC);
+
+ /* set ETDM_IN5_CON2 */
+ regmap_update_bits(afe->regmap, ETDM_IN5_CON2,
+ IN_CLK_SRC_MASK, IN_CLK_SRC(APLL_CLK));
+
+ /* set ETDM_IN5_CON3 */
+ regmap_update_bits(afe->regmap, ETDM_IN5_CON3,
+ IN_SEL_FS_MASK, IN_SEL_FS(etdm_rate));
+
+ /* set ETDM_IN5_CON4 */
+ regmap_update_bits(afe->regmap, ETDM_IN5_CON4,
+ IN_RELATCH_MASK, IN_RELATCH(afe_rate));
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int mtk_dai_etdm_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);
+
+ mtk_dai_etdm_config(afe, params, dai, SNDRV_PCM_STREAM_PLAYBACK);
+ mtk_dai_etdm_config(afe, params, dai, SNDRV_PCM_STREAM_CAPTURE);
+
+ return 0;
+}
+
+static int mtk_dai_etdm_trigger(struct snd_pcm_substream *substream, int cmd,
+ struct snd_soc_dai *dai)
+{
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+
+ dev_dbg(afe->dev, "%s(), cmd %d, dai id %d\n", __func__, cmd, dai->id);
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ regmap_update_bits(afe->regmap, ETDM_IN5_CON0, ETDM_EN_MASK,
+ ETDM_EN);
+ regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, ETDM_EN_MASK,
+ ETDM_EN);
+ break;
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ regmap_update_bits(afe->regmap, ETDM_IN5_CON0, ETDM_EN_MASK,
+ 0);
+ regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, ETDM_EN_MASK,
+ 0);
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int mtk_dai_etdm_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+ struct mt7986_afe_private *afe_priv = afe->platform_priv;
+ struct mtk_dai_etdm_priv *etdm_data;
+ void *priv_data;
+
+ switch (dai->id) {
+ case MT7986_DAI_ETDM:
+ break;
+ default:
+ dev_warn(afe->dev, "%s(), id %d not support\n",
+ __func__, dai->id);
+ return -EINVAL;
+ }
+
+ priv_data = devm_kzalloc(afe->dev, sizeof(struct mtk_dai_etdm_priv),
+ GFP_KERNEL);
+ if (!priv_data)
+ return -ENOMEM;
+
+ afe_priv->dai_priv[dai->id] = priv_data;
+ etdm_data = afe_priv->dai_priv[dai->id];
+
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S:
+ etdm_data->format = MTK_DAI_ETDM_FORMAT_I2S;
+ break;
+ case SND_SOC_DAIFMT_DSP_A:
+ etdm_data->format = MTK_DAI_ETDM_FORMAT_DSPA;
+ break;
+ case SND_SOC_DAIFMT_DSP_B:
+ etdm_data->format = MTK_DAI_ETDM_FORMAT_DSPB;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_NB_NF:
+ etdm_data->bck_inv = false;
+ etdm_data->lrck_inv = false;
+ break;
+ case SND_SOC_DAIFMT_NB_IF:
+ etdm_data->bck_inv = false;
+ etdm_data->lrck_inv = true;
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ etdm_data->bck_inv = true;
+ etdm_data->lrck_inv = false;
+ break;
+ case SND_SOC_DAIFMT_IB_IF:
+ etdm_data->bck_inv = true;
+ etdm_data->lrck_inv = true;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+ case SND_SOC_DAIFMT_CBM_CFM:
+ etdm_data->slave_mode = true;
+ break;
+ case SND_SOC_DAIFMT_CBS_CFS:
+ etdm_data->slave_mode = false;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct snd_soc_dai_ops mtk_dai_etdm_ops = {
+ .startup = mtk_dai_etdm_startup,
+ .shutdown = mtk_dai_etdm_shutdown,
+ .hw_params = mtk_dai_etdm_hw_params,
+ .trigger = mtk_dai_etdm_trigger,
+ .set_fmt = mtk_dai_etdm_set_fmt,
+};
+
+/* dai driver */
+#define MTK_ETDM_RATES (SNDRV_PCM_RATE_8000_48000 |\
+ SNDRV_PCM_RATE_88200 |\
+ SNDRV_PCM_RATE_96000 |\
+ SNDRV_PCM_RATE_176400 |\
+ SNDRV_PCM_RATE_192000)
+
+#define MTK_ETDM_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
+ SNDRV_PCM_FMTBIT_S24_LE |\
+ SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_driver mtk_dai_etdm_driver[] = {
+ {
+ .name = "ETDM",
+ .id = MT7986_DAI_ETDM,
+ .capture = {
+ .stream_name = "ETDM Capture",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = MTK_ETDM_RATES,
+ .formats = MTK_ETDM_FORMATS,
+ },
+ .playback = {
+ .stream_name = "ETDM Playback",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = MTK_ETDM_RATES,
+ .formats = MTK_ETDM_FORMATS,
+ },
+ .ops = &mtk_dai_etdm_ops,
+ .symmetric_rate = 1,
+ .symmetric_sample_bits = 1,
+ },
+};
+
+int mt7986_dai_etdm_register(struct mtk_base_afe *afe)
+{
+ struct mtk_base_afe_dai *dai;
+
+ dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL);
+ if (!dai)
+ return -ENOMEM;
+
+ list_add(&dai->list, &afe->sub_dais);
+
+ dai->dai_drivers = mtk_dai_etdm_driver;
+ dai->num_dai_drivers = ARRAY_SIZE(mtk_dai_etdm_driver);
+
+ dai->dapm_widgets = mtk_dai_etdm_widgets;
+ dai->num_dapm_widgets = ARRAY_SIZE(mtk_dai_etdm_widgets);
+ dai->dapm_routes = mtk_dai_etdm_routes;
+ dai->num_dapm_routes = ARRAY_SIZE(mtk_dai_etdm_routes);
+
+ return 0;
+}
--
2.18.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 2/6] ASoC: mediatek: mt7986: support etdm in platform driver
2023-07-28 9:08 ` [PATCH v3 2/6] ASoC: mediatek: mt7986: support etdm in platform driver Maso Huang
@ 2023-08-04 9:57 ` Alexandre Mergnat
2023-08-08 7:49 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: Alexandre Mergnat @ 2023-08-04 9:57 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Jaroslav Kysela, Takashi Iwai,
Trevor Wu, Arnd Bergmann, Mars Chen, Allen-KH Cheng, alsa-devel,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
Hi Maso,
On 28/07/2023 11:08, Maso Huang wrote:
> Add mt7986 etdm dai driver support.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> sound/soc/mediatek/mt7986/mt7986-dai-etdm.c | 420 ++++++++++++++++++++
> 1 file changed, 420 insertions(+)
> create mode 100644 sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
>
> diff --git a/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c b/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
> new file mode 100644
> index 000000000000..dc094e25ddb4
> --- /dev/null
> +++ b/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
> @@ -0,0 +1,420 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MediaTek ALSA SoC Audio DAI eTDM Control
> + *
> + * Copyright (c) 2021 MediaTek Inc.
2023
> + * Author: Vic Wu <vic.wu@mediatek.com>
Authors
> + * Maso Huang <maso.huang@mediatek.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/regmap.h>
> +#include <sound/pcm_params.h>
> +#include "mt7986-afe-common.h"
> +#include "mt7986-reg.h"
> +
> +enum {
> + HOPPING_CLK = 0,
> + APLL_CLK = 1,
> +};
Is there a reason to use enum instead of define ?
> +
> +enum {
> + MTK_DAI_ETDM_FORMAT_I2S = 0,
> + MTK_DAI_ETDM_FORMAT_DSPA = 4,
> + MTK_DAI_ETDM_FORMAT_DSPB = 5,
> +};
Same question here.
> +
> +enum {
> + ETDM_IN5 = 2,
> + ETDM_OUT5 = 10,
> +};
I don't find where these enum are used.
If there aren't used, please remove them.
> +
> +enum {
> + MTK_ETDM_RATE_8K = 0,
> + MTK_ETDM_RATE_12K = 1,
> + MTK_ETDM_RATE_16K = 2,
> + MTK_ETDM_RATE_24K = 3,
> + MTK_ETDM_RATE_32K = 4,
> + MTK_ETDM_RATE_48K = 5,
> + MTK_ETDM_RATE_96K = 7,
> + MTK_ETDM_RATE_192K = 9,
> + MTK_ETDM_RATE_11K = 16,
> + MTK_ETDM_RATE_22K = 17,
> + MTK_ETDM_RATE_44K = 18,
> + MTK_ETDM_RATE_88K = 19,
> + MTK_ETDM_RATE_176K = 20,
> +};
> +
> +struct mtk_dai_etdm_priv {
> + bool bck_inv;
> + bool lrck_inv;
> + bool slave_mode;
> + unsigned int format;
> +};
> +
> +static unsigned int mt7986_etdm_rate_transform(struct device *dev, unsigned int rate)
> +{
> + switch (rate) {
> + case 8000:
> + return MTK_ETDM_RATE_8K;
> + case 11025:
> + return MTK_ETDM_RATE_11K;
> + case 12000:
> + return MTK_ETDM_RATE_12K;
> + case 16000:
> + return MTK_ETDM_RATE_16K;
> + case 22050:
> + return MTK_ETDM_RATE_22K;
> + case 24000:
> + return MTK_ETDM_RATE_24K;
> + case 32000:
> + return MTK_ETDM_RATE_32K;
> + case 44100:
> + return MTK_ETDM_RATE_44K;
> + case 48000:
> + return MTK_ETDM_RATE_48K;
> + case 88200:
> + return MTK_ETDM_RATE_88K;
> + case 96000:
> + return MTK_ETDM_RATE_96K;
> + case 176400:
> + return MTK_ETDM_RATE_176K;
> + case 192000:
> + return MTK_ETDM_RATE_192K;
> + default:
> + dev_warn(dev, "%s(), rate %u invalid, using %d!!!\n",
> + __func__, rate, MTK_ETDM_RATE_48K);
> + return MTK_ETDM_RATE_48K;
> + }
> +}
> +
> +static int get_etdm_wlen(unsigned int bitwidth)
> +{
> + return bitwidth <= 16 ? 16 : 32;
> +}
> +
> +/* dai component */
> +/* interconnection */
> +
> +static const struct snd_kcontrol_new o124_mix[] = {
> + SOC_DAPM_SINGLE_AUTODISABLE("I032_Switch", AFE_CONN124_1, 0, 1, 0),
> +};
> +
> +static const struct snd_kcontrol_new o125_mix[] = {
> + SOC_DAPM_SINGLE_AUTODISABLE("I033_Switch", AFE_CONN125_1, 1, 1, 0),
> +};
> +
> +static const struct snd_soc_dapm_widget mtk_dai_etdm_widgets[] = {
> +
> + /* DL */
> + SND_SOC_DAPM_MIXER("I150", SND_SOC_NOPM, 0, 0, NULL, 0),
> + SND_SOC_DAPM_MIXER("I151", SND_SOC_NOPM, 0, 0, NULL, 0),
> + /* UL */
> + SND_SOC_DAPM_MIXER("O124", SND_SOC_NOPM, 0, 0, o124_mix, ARRAY_SIZE(o124_mix)),
> + SND_SOC_DAPM_MIXER("O125", SND_SOC_NOPM, 0, 0, o125_mix, ARRAY_SIZE(o125_mix)),
> +};
> +
> +static const struct snd_soc_dapm_route mtk_dai_etdm_routes[] = {
> + {"I150", NULL, "ETDM Capture"},
> + {"I151", NULL, "ETDM Capture"},
> + {"ETDM Playback", NULL, "O124"},
> + {"ETDM Playback", NULL, "O125"},
> + {"O124", "I032_Switch", "I032"},
> + {"O125", "I033_Switch", "I033"},
> +};
> +
> +/* dai ops */
> +static int mtk_dai_etdm_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt7986_afe_private *afe_priv = afe->platform_priv;
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(afe_priv->num_clks, afe_priv->clks);
> + if (ret)
> + return dev_err_probe(afe->dev, ret, "Failed to enable clocks\n");
> +
> + regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_OUT5_PDN_MASK, 0);
> + regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_IN5_PDN_MASK, 0);
> +
> + return 0;
> +}
> +
> +static void mtk_dai_etdm_shutdown(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt7986_afe_private *afe_priv = afe->platform_priv;
> +
> + regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_OUT5_PDN_MASK,
> + CLK_OUT5_PDN);
> + regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_IN5_PDN_MASK,
> + CLK_IN5_PDN);
> +
> + clk_bulk_disable_unprepare(afe_priv->num_clks, afe_priv->clks);
> +}
> +
> +static unsigned int get_etdm_ch_fixup(unsigned int channels)
> +{
> + if (channels > 16)
> + return 24;
> + else if (channels > 8)
> + return 16;
> + else if (channels > 4)
> + return 8;
> + else if (channels > 2)
> + return 4;
> + else
> + return 2;
> +}
> +
> +static int mtk_dai_etdm_config(struct mtk_base_afe *afe,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai,
> + int stream)
> +{
> + struct mt7986_afe_private *afe_priv = afe->platform_priv;
> + struct mtk_dai_etdm_priv *etdm_data = afe_priv->dai_priv[dai->id];
> + unsigned int rate = params_rate(params);
> + unsigned int etdm_rate = mt7986_etdm_rate_transform(afe->dev, rate);
> + unsigned int afe_rate = mt7986_afe_rate_transform(afe->dev, rate);
> + unsigned int channels = params_channels(params);
> + unsigned int bit_width = params_width(params);
> + unsigned int wlen = get_etdm_wlen(bit_width);
> + unsigned int val = 0;
> + unsigned int mask = 0;
> +
> + dev_dbg(afe->dev, "%s(), stream %d, rate %u, bitwidth %u\n",
> + __func__, stream, rate, bit_width);
> +
> + /* CON0 */
> + mask |= ETDM_BIT_LEN_MASK;
> + val |= ETDM_BIT_LEN(bit_width);
> + mask |= ETDM_WRD_LEN_MASK;
> + val |= ETDM_WRD_LEN(wlen);
> + mask |= ETDM_FMT_MASK;
> + val |= ETDM_FMT(etdm_data->format);
> + mask |= ETDM_CH_NUM_MASK;
> + val |= ETDM_CH_NUM(get_etdm_ch_fixup(channels));
> + mask |= RELATCH_SRC_MASK;
> + val |= RELATCH_SRC(APLL_CLK);
Why don't use bitfield helper to increase readability and reduce the
number of define from mt7986-reg.h ?
val |= FIELD_PREP(ETDM_BIT_LEN_MASK, bit_width-1) ;
Then you can remove all ETDM_*****(x) and ETDM_*****_SFT these from your
header file for all field:
#define ETDM_BIT_LEN(x) (((x) - 1) << 11)
#define ETDM_BIT_LEN_SFT 11
Please, do it for all bit operations in all your commits.
> +
> + switch (stream) {
> + case SNDRV_PCM_STREAM_PLAYBACK:
> + /* set ETDM_OUT5_CON0 */
> + regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, mask, val);
> +
> + /* set ETDM_OUT5_CON4 */
> + regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
> + OUT_RELATCH_MASK, OUT_RELATCH(afe_rate));
> + regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
> + OUT_CLK_SRC_MASK, OUT_CLK_SRC(APLL_CLK));
> + regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
> + OUT_SEL_FS_MASK, OUT_SEL_FS(etdm_rate));
> +
> + /* set ETDM_OUT5_CON5 */
> + regmap_update_bits(afe->regmap, ETDM_OUT5_CON5,
> + ETDM_CLK_DIV_MASK, ETDM_CLK_DIV);
> + break;
> + case SNDRV_PCM_STREAM_CAPTURE:
> + /* set ETDM_IN5_CON0 */
> + regmap_update_bits(afe->regmap, ETDM_IN5_CON0, mask, val);
> + regmap_update_bits(afe->regmap, ETDM_IN5_CON0,
> + ETDM_SYNC_MASK, ETDM_SYNC);
> +
> + /* set ETDM_IN5_CON2 */
> + regmap_update_bits(afe->regmap, ETDM_IN5_CON2,
> + IN_CLK_SRC_MASK, IN_CLK_SRC(APLL_CLK));
> +
> + /* set ETDM_IN5_CON3 */
> + regmap_update_bits(afe->regmap, ETDM_IN5_CON3,
> + IN_SEL_FS_MASK, IN_SEL_FS(etdm_rate));
> +
> + /* set ETDM_IN5_CON4 */
> + regmap_update_bits(afe->regmap, ETDM_IN5_CON4,
> + IN_RELATCH_MASK, IN_RELATCH(afe_rate));
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_dai_etdm_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);
> +
> + mtk_dai_etdm_config(afe, params, dai, SNDRV_PCM_STREAM_PLAYBACK);
> + mtk_dai_etdm_config(afe, params, dai, SNDRV_PCM_STREAM_CAPTURE);
> +
> + return 0;
> +}
> +
> +static int mtk_dai_etdm_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> +
> + dev_dbg(afe->dev, "%s(), cmd %d, dai id %d\n", __func__, cmd, dai->id);
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + regmap_update_bits(afe->regmap, ETDM_IN5_CON0, ETDM_EN_MASK,
> + ETDM_EN);
> + regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, ETDM_EN_MASK,
> + ETDM_EN);
> + break;
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + regmap_update_bits(afe->regmap, ETDM_IN5_CON0, ETDM_EN_MASK,
> + 0);
> + regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, ETDM_EN_MASK,
> + 0);
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_dai_etdm_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt7986_afe_private *afe_priv = afe->platform_priv;
> + struct mtk_dai_etdm_priv *etdm_data;
> + void *priv_data;
> +
> + switch (dai->id) {
> + case MT7986_DAI_ETDM:
> + break;
> + default:
> + dev_warn(afe->dev, "%s(), id %d not support\n",
> + __func__, dai->id);
> + return -EINVAL;
> + }
> +
> + priv_data = devm_kzalloc(afe->dev, sizeof(struct mtk_dai_etdm_priv),
> + GFP_KERNEL);
> + if (!priv_data)
> + return -ENOMEM;
> +
> + afe_priv->dai_priv[dai->id] = priv_data;
> + etdm_data = afe_priv->dai_priv[dai->id];
> +
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + etdm_data->format = MTK_DAI_ETDM_FORMAT_I2S;
> + break;
> + case SND_SOC_DAIFMT_DSP_A:
> + etdm_data->format = MTK_DAI_ETDM_FORMAT_DSPA;
> + break;
> + case SND_SOC_DAIFMT_DSP_B:
> + etdm_data->format = MTK_DAI_ETDM_FORMAT_DSPB;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> + case SND_SOC_DAIFMT_NB_NF:
> + etdm_data->bck_inv = false;
> + etdm_data->lrck_inv = false;
> + break;
> + case SND_SOC_DAIFMT_NB_IF:
> + etdm_data->bck_inv = false;
> + etdm_data->lrck_inv = true;
> + break;
> + case SND_SOC_DAIFMT_IB_NF:
> + etdm_data->bck_inv = true;
> + etdm_data->lrck_inv = false;
> + break;
> + case SND_SOC_DAIFMT_IB_IF:
> + etdm_data->bck_inv = true;
> + etdm_data->lrck_inv = true;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> + case SND_SOC_DAIFMT_CBM_CFM:
> + etdm_data->slave_mode = true;
> + break;
> + case SND_SOC_DAIFMT_CBS_CFS:
> + etdm_data->slave_mode = false;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops mtk_dai_etdm_ops = {
> + .startup = mtk_dai_etdm_startup,
> + .shutdown = mtk_dai_etdm_shutdown,
> + .hw_params = mtk_dai_etdm_hw_params,
> + .trigger = mtk_dai_etdm_trigger,
> + .set_fmt = mtk_dai_etdm_set_fmt,
> +};
> +
> +/* dai driver */
> +#define MTK_ETDM_RATES (SNDRV_PCM_RATE_8000_48000 |\
> + SNDRV_PCM_RATE_88200 |\
> + SNDRV_PCM_RATE_96000 |\
> + SNDRV_PCM_RATE_176400 |\
> + SNDRV_PCM_RATE_192000)
> +
> +#define MTK_ETDM_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
> + SNDRV_PCM_FMTBIT_S24_LE |\
> + SNDRV_PCM_FMTBIT_S32_LE)
> +
> +static struct snd_soc_dai_driver mtk_dai_etdm_driver[] = {
> + {
> + .name = "ETDM",
> + .id = MT7986_DAI_ETDM,
> + .capture = {
> + .stream_name = "ETDM Capture",
> + .channels_min = 1,
> + .channels_max = 2,
> + .rates = MTK_ETDM_RATES,
> + .formats = MTK_ETDM_FORMATS,
> + },
> + .playback = {
> + .stream_name = "ETDM Playback",
> + .channels_min = 1,
> + .channels_max = 2,
> + .rates = MTK_ETDM_RATES,
> + .formats = MTK_ETDM_FORMATS,
> + },
> + .ops = &mtk_dai_etdm_ops,
> + .symmetric_rate = 1,
> + .symmetric_sample_bits = 1,
> + },
> +};
> +
> +int mt7986_dai_etdm_register(struct mtk_base_afe *afe)
> +{
> + struct mtk_base_afe_dai *dai;
> +
> + dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL);
> + if (!dai)
> + return -ENOMEM;
> +
> + list_add(&dai->list, &afe->sub_dais);
> +
> + dai->dai_drivers = mtk_dai_etdm_driver;
> + dai->num_dai_drivers = ARRAY_SIZE(mtk_dai_etdm_driver);
> +
> + dai->dapm_widgets = mtk_dai_etdm_widgets;
> + dai->num_dapm_widgets = ARRAY_SIZE(mtk_dai_etdm_widgets);
> + dai->dapm_routes = mtk_dai_etdm_routes;
> + dai->num_dapm_routes = ARRAY_SIZE(mtk_dai_etdm_routes);
> +
> + return 0;
> +}
--
Regards,
Alexandre
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 2/6] ASoC: mediatek: mt7986: support etdm in platform driver
2023-08-04 9:57 ` Alexandre Mergnat
@ 2023-08-08 7:49 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-08-08 7:49 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, amergnat@baylibre.com,
lgirdwood@gmail.com, linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-08-04 at 11:57 +0200, Alexandre Mergnat wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi Maso,
>
> On 28/07/2023 11:08, Maso Huang wrote:
> > Add mt7986 etdm dai driver support.
> >
> > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> > ---
> > sound/soc/mediatek/mt7986/mt7986-dai-etdm.c | 420
> ++++++++++++++++++++
> > 1 file changed, 420 insertions(+)
> > create mode 100644 sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
> >
> > diff --git a/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
> b/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
> > new file mode 100644
> > index 000000000000..dc094e25ddb4
> > --- /dev/null
> > +++ b/sound/soc/mediatek/mt7986/mt7986-dai-etdm.c
> > @@ -0,0 +1,420 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * MediaTek ALSA SoC Audio DAI eTDM Control
> > + *
> > + * Copyright (c) 2021 MediaTek Inc.
>
> 2023
>
OK.
> > + * Author: Vic Wu <vic.wu@mediatek.com>
>
> Authors
>
OK.
> > + * Maso Huang <maso.huang@mediatek.com>
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/regmap.h>
> > +#include <sound/pcm_params.h>
> > +#include "mt7986-afe-common.h"
> > +#include "mt7986-reg.h"
> > +
> > +enum {
> > +HOPPING_CLK = 0,
> > +APLL_CLK = 1,
> > +};
>
> Is there a reason to use enum instead of define ?
>
I'll refine with define in v4 patch.
> > +
> > +enum {
> > +MTK_DAI_ETDM_FORMAT_I2S = 0,
> > +MTK_DAI_ETDM_FORMAT_DSPA = 4,
> > +MTK_DAI_ETDM_FORMAT_DSPB = 5,
> > +};
>
> Same question here.
>
I'll refine with define in v4 patch.
> > +
> > +enum {
> > +ETDM_IN5 = 2,
> > +ETDM_OUT5 = 10,
> > +};
>
> I don't find where these enum are used.
> If there aren't used, please remove them.
>
I'll remove them in v4 patch.
> > +
> > +enum {
> > +MTK_ETDM_RATE_8K = 0,
> > +MTK_ETDM_RATE_12K = 1,
> > +MTK_ETDM_RATE_16K = 2,
> > +MTK_ETDM_RATE_24K = 3,
> > +MTK_ETDM_RATE_32K = 4,
> > +MTK_ETDM_RATE_48K = 5,
> > +MTK_ETDM_RATE_96K = 7,
> > +MTK_ETDM_RATE_192K = 9,
> > +MTK_ETDM_RATE_11K = 16,
> > +MTK_ETDM_RATE_22K = 17,
> > +MTK_ETDM_RATE_44K = 18,
> > +MTK_ETDM_RATE_88K = 19,
> > +MTK_ETDM_RATE_176K = 20,
> > +};
> > +
> > +struct mtk_dai_etdm_priv {
> > +bool bck_inv;
> > +bool lrck_inv;
> > +bool slave_mode;
> > +unsigned int format;
> > +};
> > +
> > +static unsigned int mt7986_etdm_rate_transform(struct device *dev,
> unsigned int rate)
> > +{
> > +switch (rate) {
> > +case 8000:
> > +return MTK_ETDM_RATE_8K;
> > +case 11025:
> > +return MTK_ETDM_RATE_11K;
> > +case 12000:
> > +return MTK_ETDM_RATE_12K;
> > +case 16000:
> > +return MTK_ETDM_RATE_16K;
> > +case 22050:
> > +return MTK_ETDM_RATE_22K;
> > +case 24000:
> > +return MTK_ETDM_RATE_24K;
> > +case 32000:
> > +return MTK_ETDM_RATE_32K;
> > +case 44100:
> > +return MTK_ETDM_RATE_44K;
> > +case 48000:
> > +return MTK_ETDM_RATE_48K;
> > +case 88200:
> > +return MTK_ETDM_RATE_88K;
> > +case 96000:
> > +return MTK_ETDM_RATE_96K;
> > +case 176400:
> > +return MTK_ETDM_RATE_176K;
> > +case 192000:
> > +return MTK_ETDM_RATE_192K;
> > +default:
> > +dev_warn(dev, "%s(), rate %u invalid, using %d!!!\n",
> > + __func__, rate, MTK_ETDM_RATE_48K);
> > +return MTK_ETDM_RATE_48K;
> > +}
> > +}
> > +
> > +static int get_etdm_wlen(unsigned int bitwidth)
> > +{
> > +return bitwidth <= 16 ? 16 : 32;
> > +}
> > +
> > +/* dai component */
> > +/* interconnection */
> > +
> > +static const struct snd_kcontrol_new o124_mix[] = {
> > +SOC_DAPM_SINGLE_AUTODISABLE("I032_Switch", AFE_CONN124_1, 0, 1,
> 0),
> > +};
> > +
> > +static const struct snd_kcontrol_new o125_mix[] = {
> > +SOC_DAPM_SINGLE_AUTODISABLE("I033_Switch", AFE_CONN125_1, 1, 1,
> 0),
> > +};
> > +
> > +static const struct snd_soc_dapm_widget mtk_dai_etdm_widgets[] = {
> > +
> > +/* DL */
> > +SND_SOC_DAPM_MIXER("I150", SND_SOC_NOPM, 0, 0, NULL, 0),
> > +SND_SOC_DAPM_MIXER("I151", SND_SOC_NOPM, 0, 0, NULL, 0),
> > +/* UL */
> > +SND_SOC_DAPM_MIXER("O124", SND_SOC_NOPM, 0, 0, o124_mix,
> ARRAY_SIZE(o124_mix)),
> > +SND_SOC_DAPM_MIXER("O125", SND_SOC_NOPM, 0, 0, o125_mix,
> ARRAY_SIZE(o125_mix)),
> > +};
> > +
> > +static const struct snd_soc_dapm_route mtk_dai_etdm_routes[] = {
> > +{"I150", NULL, "ETDM Capture"},
> > +{"I151", NULL, "ETDM Capture"},
> > +{"ETDM Playback", NULL, "O124"},
> > +{"ETDM Playback", NULL, "O125"},
> > +{"O124", "I032_Switch", "I032"},
> > +{"O125", "I033_Switch", "I033"},
> > +};
> > +
> > +/* dai ops */
> > +static int mtk_dai_etdm_startup(struct snd_pcm_substream
> *substream,
> > +struct snd_soc_dai *dai)
> > +{
> > +struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> > +struct mt7986_afe_private *afe_priv = afe->platform_priv;
> > +int ret;
> > +
> > +ret = clk_bulk_prepare_enable(afe_priv->num_clks, afe_priv->clks);
> > +if (ret)
> > +return dev_err_probe(afe->dev, ret, "Failed to enable clocks\n");
> > +
> > +regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_OUT5_PDN_MASK,
> 0);
> > +regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_IN5_PDN_MASK,
> 0);
> > +
> > +return 0;
> > +}
> > +
> > +static void mtk_dai_etdm_shutdown(struct snd_pcm_substream
> *substream,
> > + struct snd_soc_dai *dai)
> > +{
> > +struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> > +struct mt7986_afe_private *afe_priv = afe->platform_priv;
> > +
> > +regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_OUT5_PDN_MASK,
> > + CLK_OUT5_PDN);
> > +regmap_update_bits(afe->regmap, AUDIO_TOP_CON2, CLK_IN5_PDN_MASK,
> > + CLK_IN5_PDN);
> > +
> > +clk_bulk_disable_unprepare(afe_priv->num_clks, afe_priv->clks);
> > +}
> > +
> > +static unsigned int get_etdm_ch_fixup(unsigned int channels)
> > +{
> > +if (channels > 16)
> > +return 24;
> > +else if (channels > 8)
> > +return 16;
> > +else if (channels > 4)
> > +return 8;
> > +else if (channels > 2)
> > +return 4;
> > +else
> > +return 2;
> > +}
> > +
> > +static int mtk_dai_etdm_config(struct mtk_base_afe *afe,
> > + struct snd_pcm_hw_params *params,
> > + struct snd_soc_dai *dai,
> > + int stream)
> > +{
> > +struct mt7986_afe_private *afe_priv = afe->platform_priv;
> > +struct mtk_dai_etdm_priv *etdm_data = afe_priv->dai_priv[dai->id];
> > +unsigned int rate = params_rate(params);
> > +unsigned int etdm_rate = mt7986_etdm_rate_transform(afe->dev,
> rate);
> > +unsigned int afe_rate = mt7986_afe_rate_transform(afe->dev, rate);
> > +unsigned int channels = params_channels(params);
> > +unsigned int bit_width = params_width(params);
> > +unsigned int wlen = get_etdm_wlen(bit_width);
> > +unsigned int val = 0;
> > +unsigned int mask = 0;
> > +
> > +dev_dbg(afe->dev, "%s(), stream %d, rate %u, bitwidth %u\n",
> > + __func__, stream, rate, bit_width);
> > +
> > +/* CON0 */
> > +mask |= ETDM_BIT_LEN_MASK;
> > +val |= ETDM_BIT_LEN(bit_width);
> > +mask |= ETDM_WRD_LEN_MASK;
> > +val |= ETDM_WRD_LEN(wlen);
> > +mask |= ETDM_FMT_MASK;
> > +val |= ETDM_FMT(etdm_data->format);
> > +mask |= ETDM_CH_NUM_MASK;
> > +val |= ETDM_CH_NUM(get_etdm_ch_fixup(channels));
> > +mask |= RELATCH_SRC_MASK;
> > +val |= RELATCH_SRC(APLL_CLK);
>
> Why don't use bitfield helper to increase readability and reduce the
> number of define from mt7986-reg.h ?
> val |= FIELD_PREP(ETDM_BIT_LEN_MASK, bit_width-1);
>
> Then you can remove all ETDM_*****(x) and ETDM_*****_SFT these from
> your
> header file for all field:
> #define ETDM_BIT_LEN(x) (((x) - 1) << 11)
> #define ETDM_BIT_LEN_SFT 11
>
> Please, do it for all bit operations in all your commits.
>
OK, I'll refine them in v4 patch.
> > +
> > +switch (stream) {
> > +case SNDRV_PCM_STREAM_PLAYBACK:
> > +/* set ETDM_OUT5_CON0 */
> > +regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, mask, val);
> > +
> > +/* set ETDM_OUT5_CON4 */
> > +regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
> > + OUT_RELATCH_MASK, OUT_RELATCH(afe_rate));
> > +regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
> > + OUT_CLK_SRC_MASK, OUT_CLK_SRC(APLL_CLK));
> > +regmap_update_bits(afe->regmap, ETDM_OUT5_CON4,
> > + OUT_SEL_FS_MASK, OUT_SEL_FS(etdm_rate));
> > +
> > +/* set ETDM_OUT5_CON5 */
> > +regmap_update_bits(afe->regmap, ETDM_OUT5_CON5,
> > + ETDM_CLK_DIV_MASK, ETDM_CLK_DIV);
> > +break;
> > +case SNDRV_PCM_STREAM_CAPTURE:
> > +/* set ETDM_IN5_CON0 */
> > +regmap_update_bits(afe->regmap, ETDM_IN5_CON0, mask, val);
> > +regmap_update_bits(afe->regmap, ETDM_IN5_CON0,
> > + ETDM_SYNC_MASK, ETDM_SYNC);
> > +
> > +/* set ETDM_IN5_CON2 */
> > +regmap_update_bits(afe->regmap, ETDM_IN5_CON2,
> > + IN_CLK_SRC_MASK, IN_CLK_SRC(APLL_CLK));
> > +
> > +/* set ETDM_IN5_CON3 */
> > +regmap_update_bits(afe->regmap, ETDM_IN5_CON3,
> > + IN_SEL_FS_MASK, IN_SEL_FS(etdm_rate));
> > +
> > +/* set ETDM_IN5_CON4 */
> > +regmap_update_bits(afe->regmap, ETDM_IN5_CON4,
> > + IN_RELATCH_MASK, IN_RELATCH(afe_rate));
> > +break;
> > +default:
> > +break;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int mtk_dai_etdm_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);
> > +
> > +mtk_dai_etdm_config(afe, params, dai, SNDRV_PCM_STREAM_PLAYBACK);
> > +mtk_dai_etdm_config(afe, params, dai, SNDRV_PCM_STREAM_CAPTURE);
> > +
> > +return 0;
> > +}
> > +
> > +static int mtk_dai_etdm_trigger(struct snd_pcm_substream
> *substream, int cmd,
> > +struct snd_soc_dai *dai)
> > +{
> > +struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> > +
> > +dev_dbg(afe->dev, "%s(), cmd %d, dai id %d\n", __func__, cmd, dai-
> >id);
> > +switch (cmd) {
> > +case SNDRV_PCM_TRIGGER_START:
> > +case SNDRV_PCM_TRIGGER_RESUME:
> > +regmap_update_bits(afe->regmap, ETDM_IN5_CON0, ETDM_EN_MASK,
> > + ETDM_EN);
> > +regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, ETDM_EN_MASK,
> > + ETDM_EN);
> > +break;
> > +case SNDRV_PCM_TRIGGER_STOP:
> > +case SNDRV_PCM_TRIGGER_SUSPEND:
> > +regmap_update_bits(afe->regmap, ETDM_IN5_CON0, ETDM_EN_MASK,
> > + 0);
> > +regmap_update_bits(afe->regmap, ETDM_OUT5_CON0, ETDM_EN_MASK,
> > + 0);
> > +break;
> > +default:
> > +break;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int mtk_dai_etdm_set_fmt(struct snd_soc_dai *dai, unsigned
> int fmt)
> > +{
> > +struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> > +struct mt7986_afe_private *afe_priv = afe->platform_priv;
> > +struct mtk_dai_etdm_priv *etdm_data;
> > +void *priv_data;
> > +
> > +switch (dai->id) {
> > +case MT7986_DAI_ETDM:
> > +break;
> > +default:
> > +dev_warn(afe->dev, "%s(), id %d not support\n",
> > + __func__, dai->id);
> > +return -EINVAL;
> > +}
> > +
> > +priv_data = devm_kzalloc(afe->dev, sizeof(struct
> mtk_dai_etdm_priv),
> > + GFP_KERNEL);
> > +if (!priv_data)
> > +return -ENOMEM;
> > +
> > +afe_priv->dai_priv[dai->id] = priv_data;
> > +etdm_data = afe_priv->dai_priv[dai->id];
> > +
> > +switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> > +case SND_SOC_DAIFMT_I2S:
> > +etdm_data->format = MTK_DAI_ETDM_FORMAT_I2S;
> > +break;
> > +case SND_SOC_DAIFMT_DSP_A:
> > +etdm_data->format = MTK_DAI_ETDM_FORMAT_DSPA;
> > +break;
> > +case SND_SOC_DAIFMT_DSP_B:
> > +etdm_data->format = MTK_DAI_ETDM_FORMAT_DSPB;
> > +break;
> > +default:
> > +return -EINVAL;
> > +}
> > +
> > +switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> > +case SND_SOC_DAIFMT_NB_NF:
> > +etdm_data->bck_inv = false;
> > +etdm_data->lrck_inv = false;
> > +break;
> > +case SND_SOC_DAIFMT_NB_IF:
> > +etdm_data->bck_inv = false;
> > +etdm_data->lrck_inv = true;
> > +break;
> > +case SND_SOC_DAIFMT_IB_NF:
> > +etdm_data->bck_inv = true;
> > +etdm_data->lrck_inv = false;
> > +break;
> > +case SND_SOC_DAIFMT_IB_IF:
> > +etdm_data->bck_inv = true;
> > +etdm_data->lrck_inv = true;
> > +break;
> > +default:
> > +return -EINVAL;
> > +}
> > +
> > +switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> > +case SND_SOC_DAIFMT_CBM_CFM:
> > +etdm_data->slave_mode = true;
> > +break;
> > +case SND_SOC_DAIFMT_CBS_CFS:
> > +etdm_data->slave_mode = false;
> > +break;
> > +default:
> > +return -EINVAL;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static const struct snd_soc_dai_ops mtk_dai_etdm_ops = {
> > +.startup = mtk_dai_etdm_startup,
> > +.shutdown = mtk_dai_etdm_shutdown,
> > +.hw_params = mtk_dai_etdm_hw_params,
> > +.trigger = mtk_dai_etdm_trigger,
> > +.set_fmt = mtk_dai_etdm_set_fmt,
> > +};
> > +
> > +/* dai driver */
> > +#define MTK_ETDM_RATES (SNDRV_PCM_RATE_8000_48000 |\
> > +SNDRV_PCM_RATE_88200 |\
> > +SNDRV_PCM_RATE_96000 |\
> > +SNDRV_PCM_RATE_176400 |\
> > +SNDRV_PCM_RATE_192000)
> > +
> > +#define MTK_ETDM_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
> > + SNDRV_PCM_FMTBIT_S24_LE |\
> > + SNDRV_PCM_FMTBIT_S32_LE)
> > +
> > +static struct snd_soc_dai_driver mtk_dai_etdm_driver[] = {
> > +{
> > +.name = "ETDM",
> > +.id = MT7986_DAI_ETDM,
> > +.capture = {
> > +.stream_name = "ETDM Capture",
> > +.channels_min = 1,
> > +.channels_max = 2,
> > +.rates = MTK_ETDM_RATES,
> > +.formats = MTK_ETDM_FORMATS,
> > +},
> > +.playback = {
> > +.stream_name = "ETDM Playback",
> > +.channels_min = 1,
> > +.channels_max = 2,
> > +.rates = MTK_ETDM_RATES,
> > +.formats = MTK_ETDM_FORMATS,
> > +},
> > +.ops = &mtk_dai_etdm_ops,
> > +.symmetric_rate = 1,
> > +.symmetric_sample_bits = 1,
> > +},
> > +};
> > +
> > +int mt7986_dai_etdm_register(struct mtk_base_afe *afe)
> > +{
> > +struct mtk_base_afe_dai *dai;
> > +
> > +dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL);
> > +if (!dai)
> > +return -ENOMEM;
> > +
> > +list_add(&dai->list, &afe->sub_dais);
> > +
> > +dai->dai_drivers = mtk_dai_etdm_driver;
> > +dai->num_dai_drivers = ARRAY_SIZE(mtk_dai_etdm_driver);
> > +
> > +dai->dapm_widgets = mtk_dai_etdm_widgets;
> > +dai->num_dapm_widgets = ARRAY_SIZE(mtk_dai_etdm_widgets);
> > +dai->dapm_routes = mtk_dai_etdm_routes;
> > +dai->num_dapm_routes = ARRAY_SIZE(mtk_dai_etdm_routes);
> > +
> > +return 0;
> > +}
>
> --
> Regards,
> Alexandre
Hi Alexandre,
Thanks for your review.
I'll refine all of them in v4 patch :)
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 3/6] ASoC: mediatek: mt7986: add platform driver
2023-07-28 9:08 [PATCH v3 0/6] ASoC: mediatek: Add support for MT7986 SoC Maso Huang
2023-07-28 9:08 ` [PATCH v3 1/6] ASoC: mediatek: mt7986: add common header Maso Huang
2023-07-28 9:08 ` [PATCH v3 2/6] ASoC: mediatek: mt7986: support etdm in platform driver Maso Huang
@ 2023-07-28 9:08 ` Maso Huang
2023-07-28 9:38 ` AngeloGioacchino Del Regno
2023-07-28 9:08 ` [PATCH v3 4/6] ASoC: mediatek: mt7986: add machine driver with wm8960 Maso Huang
` (2 subsequent siblings)
5 siblings, 1 reply; 33+ messages in thread
From: Maso Huang @ 2023-07-28 9:08 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Maso Huang
Add mt7986 platform driver.
Signed-off-by: Maso Huang <maso.huang@mediatek.com>
---
sound/soc/mediatek/Kconfig | 10 +
sound/soc/mediatek/Makefile | 1 +
sound/soc/mediatek/mt7986/Makefile | 8 +
sound/soc/mediatek/mt7986/mt7986-afe-pcm.c | 622 +++++++++++++++++++++
4 files changed, 641 insertions(+)
create mode 100644 sound/soc/mediatek/mt7986/Makefile
create mode 100644 sound/soc/mediatek/mt7986/mt7986-afe-pcm.c
diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig
index 90db67e0ce4f..558827755a8d 100644
--- a/sound/soc/mediatek/Kconfig
+++ b/sound/soc/mediatek/Kconfig
@@ -54,6 +54,16 @@ config SND_SOC_MT6797_MT6351
Select Y if you have such device.
If unsure select "N".
+config SND_SOC_MT7986
+ tristate "ASoC support for Mediatek MT7986 chip"
+ depends on ARCH_MEDIATEK
+ select SND_SOC_MEDIATEK
+ help
+ This adds ASoC platform driver support for MediaTek MT7986 chip
+ that can be used with other codecs.
+ Select Y if you have such device.
+ If unsure select "N".
+
config SND_SOC_MT8173
tristate "ASoC support for Mediatek MT8173 chip"
depends on ARCH_MEDIATEK
diff --git a/sound/soc/mediatek/Makefile b/sound/soc/mediatek/Makefile
index 3de38cfc69e5..3938e7f75c2e 100644
--- a/sound/soc/mediatek/Makefile
+++ b/sound/soc/mediatek/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_SND_SOC_MEDIATEK) += common/
obj-$(CONFIG_SND_SOC_MT2701) += mt2701/
obj-$(CONFIG_SND_SOC_MT6797) += mt6797/
+obj-$(CONFIG_SND_SOC_MT7986) += mt7986/
obj-$(CONFIG_SND_SOC_MT8173) += mt8173/
obj-$(CONFIG_SND_SOC_MT8183) += mt8183/
obj-$(CONFIG_SND_SOC_MT8186) += mt8186/
diff --git a/sound/soc/mediatek/mt7986/Makefile b/sound/soc/mediatek/mt7986/Makefile
new file mode 100644
index 000000000000..de0742a67cae
--- /dev/null
+++ b/sound/soc/mediatek/mt7986/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# platform driver
+snd-soc-mt7986-afe-objs := \
+ mt7986-afe-pcm.o \
+ mt7986-dai-etdm.o
+
+obj-$(CONFIG_SND_SOC_MT7986) += snd-soc-mt7986-afe.o
diff --git a/sound/soc/mediatek/mt7986/mt7986-afe-pcm.c b/sound/soc/mediatek/mt7986/mt7986-afe-pcm.c
new file mode 100644
index 000000000000..0d498bdb95a9
--- /dev/null
+++ b/sound/soc/mediatek/mt7986/mt7986-afe-pcm.c
@@ -0,0 +1,622 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek ALSA SoC AFE platform driver for MT7986
+ *
+ * Copyright (c) 2021 MediaTek Inc.
+ * Author: Vic Wu <vic.wu@mediatek.com>
+ * Maso Huang <maso.huang@mediatek.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/pm_runtime.h>
+
+#include "mt7986-afe-common.h"
+#include "mt7986-reg.h"
+#include "../common/mtk-afe-platform-driver.h"
+#include "../common/mtk-afe-fe-dai.h"
+
+enum {
+ MTK_AFE_RATE_8K = 0,
+ MTK_AFE_RATE_11K = 1,
+ MTK_AFE_RATE_12K = 2,
+ MTK_AFE_RATE_16K = 4,
+ MTK_AFE_RATE_22K = 5,
+ MTK_AFE_RATE_24K = 6,
+ MTK_AFE_RATE_32K = 8,
+ MTK_AFE_RATE_44K = 9,
+ MTK_AFE_RATE_48K = 10,
+ MTK_AFE_RATE_88K = 13,
+ MTK_AFE_RATE_96K = 14,
+ MTK_AFE_RATE_176K = 17,
+ MTK_AFE_RATE_192K = 18,
+};
+
+enum {
+ CLK_INFRA_AUD_BUS_CK = 0,
+ CLK_INFRA_AUD_26M_CK,
+ CLK_INFRA_AUD_L_CK,
+ CLK_INFRA_AUD_AUD_CK,
+ CLK_INFRA_AUD_EG2_CK,
+ CLK_NUM
+};
+
+static const char *aud_clks[CLK_NUM] = {
+ [CLK_INFRA_AUD_BUS_CK] = "aud_bus_ck",
+ [CLK_INFRA_AUD_26M_CK] = "aud_26m_ck",
+ [CLK_INFRA_AUD_L_CK] = "aud_l_ck",
+ [CLK_INFRA_AUD_AUD_CK] = "aud_aud_ck",
+ [CLK_INFRA_AUD_EG2_CK] = "aud_eg2_ck",
+};
+
+unsigned int mt7986_afe_rate_transform(struct device *dev, unsigned int rate)
+{
+ switch (rate) {
+ case 8000:
+ return MTK_AFE_RATE_8K;
+ case 11025:
+ return MTK_AFE_RATE_11K;
+ case 12000:
+ return MTK_AFE_RATE_12K;
+ case 16000:
+ return MTK_AFE_RATE_16K;
+ case 22050:
+ return MTK_AFE_RATE_22K;
+ case 24000:
+ return MTK_AFE_RATE_24K;
+ case 32000:
+ return MTK_AFE_RATE_32K;
+ case 44100:
+ return MTK_AFE_RATE_44K;
+ case 48000:
+ return MTK_AFE_RATE_48K;
+ case 88200:
+ return MTK_AFE_RATE_88K;
+ case 96000:
+ return MTK_AFE_RATE_96K;
+ case 176400:
+ return MTK_AFE_RATE_176K;
+ case 192000:
+ return MTK_AFE_RATE_192K;
+ default:
+ dev_warn(dev, "%s(), rate %u invalid, using %d!!!\n",
+ __func__, rate, MTK_AFE_RATE_48K);
+ return MTK_AFE_RATE_48K;
+ }
+}
+
+static const struct snd_pcm_hardware mt7986_afe_hardware = {
+ .info = SNDRV_PCM_INFO_MMAP |
+ SNDRV_PCM_INFO_INTERLEAVED |
+ SNDRV_PCM_INFO_MMAP_VALID,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE |
+ SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S32_LE,
+ .period_bytes_min = 256,
+ .period_bytes_max = 4 * 48 * 1024,
+ .periods_min = 2,
+ .periods_max = 256,
+ .buffer_bytes_max = 8 * 48 * 1024,
+ .fifo_size = 0,
+};
+
+static int mt7986_memif_fs(struct snd_pcm_substream *substream,
+ unsigned int rate)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
+ struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
+
+ return mt7986_afe_rate_transform(afe->dev, rate);
+}
+
+static int mt7986_irq_fs(struct snd_pcm_substream *substream,
+ unsigned int rate)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
+ struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
+
+ return mt7986_afe_rate_transform(afe->dev, rate);
+}
+
+#define MTK_PCM_RATES (SNDRV_PCM_RATE_8000_48000 |\
+ SNDRV_PCM_RATE_88200 |\
+ SNDRV_PCM_RATE_96000 |\
+ SNDRV_PCM_RATE_176400 |\
+ SNDRV_PCM_RATE_192000)
+
+#define MTK_PCM_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
+ SNDRV_PCM_FMTBIT_S24_LE |\
+ SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_driver mt7986_memif_dai_driver[] = {
+ /* FE DAIs: memory intefaces to CPU */
+ {
+ .name = "DL1",
+ .id = MT7986_MEMIF_DL1,
+ .playback = {
+ .stream_name = "DL1",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = MTK_PCM_RATES,
+ .formats = MTK_PCM_FORMATS,
+ },
+ .ops = &mtk_afe_fe_ops,
+ },
+ {
+ .name = "UL1",
+ .id = MT7986_MEMIF_VUL12,
+ .capture = {
+ .stream_name = "UL1",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = MTK_PCM_RATES,
+ .formats = MTK_PCM_FORMATS,
+ },
+ .ops = &mtk_afe_fe_ops,
+ },
+};
+
+static const struct snd_kcontrol_new o018_mix[] = {
+ SOC_DAPM_SINGLE_AUTODISABLE("I150_Switch", AFE_CONN018_4, 22, 1, 0),
+};
+
+static const struct snd_kcontrol_new o019_mix[] = {
+ SOC_DAPM_SINGLE_AUTODISABLE("I151_Switch", AFE_CONN019_4, 23, 1, 0),
+};
+
+static const struct snd_soc_dapm_widget mt7986_memif_widgets[] = {
+ /* DL */
+ SND_SOC_DAPM_MIXER("I032", SND_SOC_NOPM, 0, 0, NULL, 0),
+ SND_SOC_DAPM_MIXER("I033", SND_SOC_NOPM, 0, 0, NULL, 0),
+
+ /* UL */
+ SND_SOC_DAPM_MIXER("O018", SND_SOC_NOPM, 0, 0,
+ o018_mix, ARRAY_SIZE(o018_mix)),
+ SND_SOC_DAPM_MIXER("O019", SND_SOC_NOPM, 0, 0,
+ o019_mix, ARRAY_SIZE(o019_mix)),
+};
+
+static const struct snd_soc_dapm_route mt7986_memif_routes[] = {
+ {"I032", NULL, "DL1"},
+ {"I033", NULL, "DL1"},
+ {"UL1", NULL, "O018"},
+ {"UL1", NULL, "O019"},
+ {"O018", "I150_Switch", "I150"},
+ {"O019", "I151_Switch", "I151"},
+};
+
+static const struct snd_soc_component_driver mt7986_afe_pcm_dai_component = {
+ .name = "mt7986-afe-pcm-dai",
+};
+
+static const struct mtk_base_memif_data memif_data[MT7986_MEMIF_NUM] = {
+ [MT7986_MEMIF_DL1] = {
+ .name = "DL1",
+ .id = MT7986_MEMIF_DL1,
+ .reg_ofs_base = AFE_DL0_BASE,
+ .reg_ofs_cur = AFE_DL0_CUR,
+ .reg_ofs_end = AFE_DL0_END,
+ .reg_ofs_base_msb = AFE_DL0_BASE_MSB,
+ .reg_ofs_cur_msb = AFE_DL0_CUR_MSB,
+ .reg_ofs_end_msb = AFE_DL0_END_MSB,
+ .fs_reg = AFE_DL0_CON0,
+ .fs_shift = DL0_MODE_SFT,
+ .fs_maskbit = DL0_MODE_MASK,
+ .mono_reg = AFE_DL0_CON0,
+ .mono_shift = DL0_MONO_SFT,
+ .enable_reg = AFE_DL0_CON0,
+ .enable_shift = DL0_ON_SFT,
+ .hd_reg = AFE_DL0_CON0,
+ .hd_shift = DL0_HD_MODE_SFT,
+ .hd_align_reg = AFE_DL0_CON0,
+ .hd_align_mshift = DL0_HALIGN_SFT,
+ .pbuf_reg = AFE_DL0_CON0,
+ .pbuf_shift = DL0_PBUF_SIZE_SFT,
+ .minlen_reg = AFE_DL0_CON0,
+ .minlen_shift = DL0_MINLEN_SFT,
+ },
+ [MT7986_MEMIF_VUL12] = {
+ .name = "VUL12",
+ .id = MT7986_MEMIF_VUL12,
+ .reg_ofs_base = AFE_VUL0_BASE,
+ .reg_ofs_cur = AFE_VUL0_CUR,
+ .reg_ofs_end = AFE_VUL0_END,
+ .reg_ofs_base_msb = AFE_VUL0_BASE_MSB,
+ .reg_ofs_cur_msb = AFE_VUL0_CUR_MSB,
+ .reg_ofs_end_msb = AFE_VUL0_END_MSB,
+ .fs_reg = AFE_VUL0_CON0,
+ .fs_shift = VUL0_MODE_SFT,
+ .fs_maskbit = VUL0_MODE_MASK,
+ .mono_reg = AFE_VUL0_CON0,
+ .mono_shift = VUL0_MONO_SFT,
+ .enable_reg = AFE_VUL0_CON0,
+ .enable_shift = VUL0_ON_SFT,
+ .hd_reg = AFE_VUL0_CON0,
+ .hd_shift = VUL0_HD_MODE_SFT,
+ .hd_align_reg = AFE_VUL0_CON0,
+ .hd_align_mshift = VUL0_HALIGN_SFT,
+ },
+};
+
+static const struct mtk_base_irq_data irq_data[MT7986_IRQ_NUM] = {
+ [MT7986_IRQ_0] = {
+ .id = MT7986_IRQ_0,
+ .irq_cnt_reg = AFE_IRQ0_MCU_CFG1,
+ .irq_cnt_shift = AFE_IRQ_CNT_SHIFT,
+ .irq_cnt_maskbit = AFE_IRQ_CNT_MASK,
+ .irq_fs_reg = AFE_IRQ0_MCU_CFG0,
+ .irq_fs_shift = IRQ_MCU_MODE_SFT,
+ .irq_fs_maskbit = IRQ_MCU_MODE_MASK,
+ .irq_en_reg = AFE_IRQ0_MCU_CFG0,
+ .irq_en_shift = IRQ_MCU_ON_SFT,
+ .irq_clr_reg = AFE_IRQ_MCU_CLR,
+ .irq_clr_shift = IRQ0_MCU_CLR_SFT,
+ },
+ [MT7986_IRQ_1] = {
+ .id = MT7986_IRQ_1,
+ .irq_cnt_reg = AFE_IRQ1_MCU_CFG1,
+ .irq_cnt_shift = AFE_IRQ_CNT_SHIFT,
+ .irq_cnt_maskbit = AFE_IRQ_CNT_MASK,
+ .irq_fs_reg = AFE_IRQ1_MCU_CFG0,
+ .irq_fs_shift = IRQ_MCU_MODE_SFT,
+ .irq_fs_maskbit = IRQ_MCU_MODE_MASK,
+ .irq_en_reg = AFE_IRQ1_MCU_CFG0,
+ .irq_en_shift = IRQ_MCU_ON_SFT,
+ .irq_clr_reg = AFE_IRQ_MCU_CLR,
+ .irq_clr_shift = IRQ1_MCU_CLR_SFT,
+ },
+ [MT7986_IRQ_2] = {
+ .id = MT7986_IRQ_2,
+ .irq_cnt_reg = AFE_IRQ2_MCU_CFG1,
+ .irq_cnt_shift = AFE_IRQ_CNT_SHIFT,
+ .irq_cnt_maskbit = AFE_IRQ_CNT_MASK,
+ .irq_fs_reg = AFE_IRQ2_MCU_CFG0,
+ .irq_fs_shift = IRQ_MCU_MODE_SFT,
+ .irq_fs_maskbit = IRQ_MCU_MODE_MASK,
+ .irq_en_reg = AFE_IRQ2_MCU_CFG0,
+ .irq_en_shift = IRQ_MCU_ON_SFT,
+ .irq_clr_reg = AFE_IRQ_MCU_CLR,
+ .irq_clr_shift = IRQ2_MCU_CLR_SFT,
+ },
+};
+
+static bool mt7986_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+ /*
+ * Those auto-gen regs are read-only, so put it as volatile because
+ * volatile registers cannot be cached, which means that they cannot
+ * be set when power is off
+ */
+
+ switch (reg) {
+ case AFE_DL0_CUR_MSB:
+ case AFE_DL0_CUR:
+ case AFE_DL0_RCH_MON:
+ case AFE_DL0_LCH_MON:
+ case AFE_VUL0_CUR_MSB:
+ case AFE_VUL0_CUR:
+ case AFE_IRQ_MCU_STATUS:
+ case AFE_MEMIF_RD_MON:
+ case AFE_MEMIF_WR_MON:
+ return true;
+ default:
+ return false;
+ };
+}
+
+static const struct regmap_config mt7986_afe_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .volatile_reg = mt7986_is_volatile_reg,
+ .max_register = AFE_MAX_REGISTER,
+ .num_reg_defaults_raw = ((AFE_MAX_REGISTER / 4) + 1),
+};
+
+static int mt7986_init_clock(struct mtk_base_afe *afe)
+{
+ struct mt7986_afe_private *afe_priv = afe->platform_priv;
+ int ret, i;
+
+ afe_priv->clks = devm_kcalloc(afe->dev, CLK_NUM,
+ sizeof(*afe_priv->clks), GFP_KERNEL);
+ if (!afe_priv->clks)
+ return -ENOMEM;
+ afe_priv->num_clks = CLK_NUM;
+
+ for (i = 0; i < afe_priv->num_clks; i++)
+ afe_priv->clks[i].id = aud_clks[i];
+
+ ret = devm_clk_bulk_get(afe->dev, afe_priv->num_clks, afe_priv->clks);
+ if (ret)
+ return dev_err_probe(afe->dev, ret, "Failed to get clocks\n");
+
+ return 0;
+}
+
+static irqreturn_t mt7986_afe_irq_handler(int irq_id, void *dev)
+{
+ struct mtk_base_afe *afe = dev;
+ struct mtk_base_afe_irq *irq;
+ u32 mcu_en, status, status_mcu;
+ int i, ret;
+ irqreturn_t irq_ret = IRQ_HANDLED;
+
+ /* get irq that is sent to MCU */
+ regmap_read(afe->regmap, AFE_IRQ_MCU_EN, &mcu_en);
+
+ ret = regmap_read(afe->regmap, AFE_IRQ_MCU_STATUS, &status);
+ /* only care IRQ which is sent to MCU */
+ status_mcu = status & mcu_en & AFE_IRQ_STATUS_BITS;
+
+ if (ret || status_mcu == 0) {
+ dev_err(afe->dev, "%s(), irq status err, ret %d, status 0x%x, mcu_en 0x%x\n",
+ __func__, ret, status, mcu_en);
+
+ irq_ret = IRQ_NONE;
+ goto err_irq;
+ }
+
+ for (i = 0; i < MT7986_MEMIF_NUM; i++) {
+ struct mtk_base_afe_memif *memif = &afe->memif[i];
+
+ if (!memif->substream)
+ continue;
+
+ if (memif->irq_usage < 0)
+ continue;
+
+ irq = &afe->irqs[memif->irq_usage];
+
+ if (status_mcu & (1 << irq->irq_data->irq_en_shift))
+ snd_pcm_period_elapsed(memif->substream);
+ }
+
+err_irq:
+ /* clear irq */
+ regmap_write(afe->regmap, AFE_IRQ_MCU_CLR, status_mcu);
+
+ return irq_ret;
+}
+
+static int mt7986_afe_runtime_suspend(struct device *dev)
+{
+ struct mtk_base_afe *afe = dev_get_drvdata(dev);
+ struct mt7986_afe_private *afe_priv = afe->platform_priv;
+
+ if (!afe->regmap || afe_priv->pm_runtime_bypass_reg_ctl)
+ goto skip_regmap;
+
+ /* disable clk*/
+ regmap_update_bits(afe->regmap, AUDIO_TOP_CON4, 0x3fff, 0x3fff);
+ regmap_update_bits(afe->regmap, AUDIO_ENGEN_CON0, AUD_APLL2_EN_MASK, 0);
+ regmap_update_bits(afe->regmap, AUDIO_ENGEN_CON0, AUD_26M_EN_MASK, 0);
+
+ /* make sure all irq status are cleared, twice intended */
+ regmap_update_bits(afe->regmap, AFE_IRQ_MCU_CLR, 0xffff, 0xffff);
+
+skip_regmap:
+ clk_bulk_disable_unprepare(afe_priv->num_clks, afe_priv->clks);
+
+ return 0;
+}
+
+static int mt7986_afe_runtime_resume(struct device *dev)
+{
+ struct mtk_base_afe *afe = dev_get_drvdata(dev);
+ struct mt7986_afe_private *afe_priv = afe->platform_priv;
+ int ret;
+
+ ret = clk_bulk_prepare_enable(afe_priv->num_clks, afe_priv->clks);
+ if (ret)
+ return dev_err_probe(afe->dev, ret, "Failed to enable clocks\n");
+
+ if (!afe->regmap || afe_priv->pm_runtime_bypass_reg_ctl)
+ return 0;
+
+ /* enable clk*/
+ regmap_update_bits(afe->regmap, AUDIO_TOP_CON4, 0x3fff, 0);
+ regmap_update_bits(afe->regmap, AUDIO_ENGEN_CON0, AUD_APLL2_EN_MASK,
+ AUD_APLL2_EN);
+ regmap_update_bits(afe->regmap, AUDIO_ENGEN_CON0, AUD_26M_EN_MASK,
+ AUD_26M_EN);
+
+ return 0;
+}
+
+static int mt7986_afe_component_probe(struct snd_soc_component *component)
+{
+ return mtk_afe_add_sub_dai_control(component);
+}
+
+static const struct snd_soc_component_driver mt7986_afe_component = {
+ .name = AFE_PCM_NAME,
+ .probe = mt7986_afe_component_probe,
+ .pointer = mtk_afe_pcm_pointer,
+ .pcm_construct = mtk_afe_pcm_new,
+};
+
+static int mt7986_dai_memif_register(struct mtk_base_afe *afe)
+{
+ struct mtk_base_afe_dai *dai;
+
+ dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL);
+ if (!dai)
+ return -ENOMEM;
+
+ list_add(&dai->list, &afe->sub_dais);
+
+ dai->dai_drivers = mt7986_memif_dai_driver;
+ dai->num_dai_drivers = ARRAY_SIZE(mt7986_memif_dai_driver);
+
+ dai->dapm_widgets = mt7986_memif_widgets;
+ dai->num_dapm_widgets = ARRAY_SIZE(mt7986_memif_widgets);
+ dai->dapm_routes = mt7986_memif_routes;
+ dai->num_dapm_routes = ARRAY_SIZE(mt7986_memif_routes);
+
+ return 0;
+}
+
+typedef int (*dai_register_cb)(struct mtk_base_afe *);
+static const dai_register_cb dai_register_cbs[] = {
+ mt7986_dai_etdm_register,
+ mt7986_dai_memif_register,
+};
+
+static int mt7986_afe_pcm_dev_probe(struct platform_device *pdev)
+{
+ struct mtk_base_afe *afe;
+ struct mt7986_afe_private *afe_priv;
+ struct device *dev;
+ int i, irq_id, ret;
+
+ afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL);
+ if (!afe)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, afe);
+
+ afe->platform_priv = devm_kzalloc(&pdev->dev, sizeof(*afe_priv),
+ GFP_KERNEL);
+ if (!afe->platform_priv)
+ return -ENOMEM;
+
+ afe_priv = afe->platform_priv;
+ afe->dev = &pdev->dev;
+ dev = afe->dev;
+
+ afe->base_addr = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(afe->base_addr))
+ return PTR_ERR(afe->base_addr);
+
+ /* initial audio related clock */
+ ret = mt7986_init_clock(afe);
+ if (ret)
+ return dev_err_probe(dev, ret, "Cannot initialize clocks\n");
+
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ /* enable clock for regcache get default value from hw */
+ afe_priv->pm_runtime_bypass_reg_ctl = true;
+ pm_runtime_get_sync(&pdev->dev);
+
+ afe->regmap = devm_regmap_init_mmio(&pdev->dev, afe->base_addr,
+ &mt7986_afe_regmap_config);
+
+ pm_runtime_put_sync(&pdev->dev);
+ if (IS_ERR(afe->regmap))
+ return PTR_ERR(afe->regmap);
+
+ afe_priv->pm_runtime_bypass_reg_ctl = false;
+
+ /* init memif */
+ afe->memif_size = MT7986_MEMIF_NUM;
+ afe->memif = devm_kcalloc(dev, afe->memif_size, sizeof(*afe->memif),
+ GFP_KERNEL);
+ if (!afe->memif)
+ return -ENOMEM;
+
+ for (i = 0; i < afe->memif_size; i++) {
+ afe->memif[i].data = &memif_data[i];
+ afe->memif[i].irq_usage = -1;
+ }
+
+ mutex_init(&afe->irq_alloc_lock);
+
+ /* irq initialize */
+ afe->irqs_size = MT7986_IRQ_NUM;
+ afe->irqs = devm_kcalloc(dev, afe->irqs_size, sizeof(*afe->irqs),
+ GFP_KERNEL);
+ if (!afe->irqs)
+ return -ENOMEM;
+
+ for (i = 0; i < afe->irqs_size; i++)
+ afe->irqs[i].irq_data = &irq_data[i];
+
+ /* request irq */
+ irq_id = platform_get_irq(pdev, 0);
+ if (irq_id < 0) {
+ ret = irq_id;
+ return dev_err_probe(dev, ret, "%pOFn no irq found\n", dev->of_node);
+ }
+ ret = devm_request_irq(dev, irq_id, mt7986_afe_irq_handler,
+ IRQF_TRIGGER_NONE, "asys-isr", (void *)afe);
+ if (ret)
+ return dev_err_probe(dev, ret, "could not request_irq for asys-isr\n");
+
+ /* init sub_dais */
+ INIT_LIST_HEAD(&afe->sub_dais);
+
+ for (i = 0; i < ARRAY_SIZE(dai_register_cbs); i++) {
+ ret = dai_register_cbs[i](afe);
+ if (ret)
+ return dev_err_probe(dev, ret, "dai register i %d fail\n", i);
+ }
+
+ /* init dai_driver and component_driver */
+ ret = mtk_afe_combine_sub_dai(afe);
+ if (ret)
+ return dev_err_probe(dev, ret, "mtk_afe_combine_sub_dai fail\n");
+
+ afe->mtk_afe_hardware = &mt7986_afe_hardware;
+ afe->memif_fs = mt7986_memif_fs;
+ afe->irq_fs = mt7986_irq_fs;
+
+ afe->runtime_resume = mt7986_afe_runtime_resume;
+ afe->runtime_suspend = mt7986_afe_runtime_suspend;
+
+ /* register component */
+ ret = devm_snd_soc_register_component(&pdev->dev,
+ &mt7986_afe_component,
+ NULL, 0);
+ if (ret)
+ return dev_err_probe(dev, ret, "err_platform\n");
+
+ ret = devm_snd_soc_register_component(afe->dev,
+ &mt7986_afe_pcm_dai_component,
+ afe->dai_drivers,
+ afe->num_dai_drivers);
+ if (ret)
+ return dev_err_probe(dev, ret, "err_dai_component\n");
+
+ return 0;
+}
+
+static void mt7986_afe_pcm_dev_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+ if (!pm_runtime_status_suspended(&pdev->dev))
+ mt7986_afe_runtime_suspend(&pdev->dev);
+}
+
+static const struct of_device_id mt7986_afe_pcm_dt_match[] = {
+ { .compatible = "mediatek,mt7986-afe" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mt7986_afe_pcm_dt_match);
+
+static const struct dev_pm_ops mt7986_afe_pm_ops = {
+ SET_RUNTIME_PM_OPS(mt7986_afe_runtime_suspend,
+ mt7986_afe_runtime_resume, NULL)
+};
+
+static struct platform_driver mt7986_afe_pcm_driver = {
+ .driver = {
+ .name = "mt7986-audio",
+ .of_match_table = mt7986_afe_pcm_dt_match,
+ .pm = &mt7986_afe_pm_ops,
+ },
+ .probe = mt7986_afe_pcm_dev_probe,
+ .remove_new = mt7986_afe_pcm_dev_remove,
+};
+module_platform_driver(mt7986_afe_pcm_driver);
+
+MODULE_DESCRIPTION("MediaTek SoC AFE platform driver for ALSA MT7986");
+MODULE_AUTHOR("Vic Wu <vic.wu@mediatek.com>");
+MODULE_LICENSE("GPL");
--
2.18.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 3/6] ASoC: mediatek: mt7986: add platform driver
2023-07-28 9:08 ` [PATCH v3 3/6] ASoC: mediatek: mt7986: add " Maso Huang
@ 2023-07-28 9:38 ` AngeloGioacchino Del Regno
2023-07-28 10:13 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-28 9:38 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Il 28/07/23 11:08, Maso Huang ha scritto:
> Add mt7986 platform driver.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> ---
> sound/soc/mediatek/Kconfig | 10 +
> sound/soc/mediatek/Makefile | 1 +
> sound/soc/mediatek/mt7986/Makefile | 8 +
> sound/soc/mediatek/mt7986/mt7986-afe-pcm.c | 622 +++++++++++++++++++++
> 4 files changed, 641 insertions(+)
> create mode 100644 sound/soc/mediatek/mt7986/Makefile
> create mode 100644 sound/soc/mediatek/mt7986/mt7986-afe-pcm.c
>
..snip..
> + /* register component */
> + ret = devm_snd_soc_register_component(&pdev->dev,
> + &mt7986_afe_component,
> + NULL, 0);
> + if (ret)
> + return dev_err_probe(dev, ret, "err_platform\n");
I know I only said about using dev_err_probe(), but "err_platform" doesn't
mean anything!
Please write a human readable error message, like "Cannot register AFE component\n"
> +
> + ret = devm_snd_soc_register_component(afe->dev,
> + &mt7986_afe_pcm_dai_component,
> + afe->dai_drivers,
> + afe->num_dai_drivers);
> + if (ret)
> + return dev_err_probe(dev, ret, "err_dai_component\n");
And the same here, "Cannot register PCM DAI component\n"
After which:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/6] ASoC: mediatek: mt7986: add platform driver
2023-07-28 9:38 ` AngeloGioacchino Del Regno
@ 2023-07-28 10:13 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-07-28 10:13 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-07-28 at 11:38 +0200, AngeloGioacchino Del Regno wrote:
> Il 28/07/23 11:08, Maso Huang ha scritto:
> > Add mt7986 platform driver.
> >
> > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > ---
> > sound/soc/mediatek/Kconfig | 10 +
> > sound/soc/mediatek/Makefile | 1 +
> > sound/soc/mediatek/mt7986/Makefile | 8 +
> > sound/soc/mediatek/mt7986/mt7986-afe-pcm.c | 622
> > +++++++++++++++++++++
> > 4 files changed, 641 insertions(+)
> > create mode 100644 sound/soc/mediatek/mt7986/Makefile
> > create mode 100644 sound/soc/mediatek/mt7986/mt7986-afe-pcm.c
> >
>
> ..snip..
>
> > + /* register component */
> > + ret = devm_snd_soc_register_component(&pdev->dev,
> > + &mt7986_afe_component,
> > + NULL, 0);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "err_platform\n");
>
> I know I only said about using dev_err_probe(), but "err_platform"
> doesn't
> mean anything!
>
> Please write a human readable error message, like "Cannot register
> AFE component\n"
>
> > +
> > + ret = devm_snd_soc_register_component(afe->dev,
> > + &mt7986_afe_pcm_dai_compo
> > nent,
> > + afe->dai_drivers,
> > + afe->num_dai_drivers);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "err_dai_component\n");
>
> And the same here, "Cannot register PCM DAI component\n"
>
> After which:
>
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
Hi Angelo,
Thanks for your review.
I'll refine it with human readable message!
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 4/6] ASoC: mediatek: mt7986: add machine driver with wm8960
2023-07-28 9:08 [PATCH v3 0/6] ASoC: mediatek: Add support for MT7986 SoC Maso Huang
` (2 preceding siblings ...)
2023-07-28 9:08 ` [PATCH v3 3/6] ASoC: mediatek: mt7986: add " Maso Huang
@ 2023-07-28 9:08 ` Maso Huang
2023-07-28 9:49 ` AngeloGioacchino Del Regno
2023-07-28 9:08 ` [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document Maso Huang
2023-07-28 9:08 ` [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document Maso Huang
5 siblings, 1 reply; 33+ messages in thread
From: Maso Huang @ 2023-07-28 9:08 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Maso Huang
Add support for mt7986 board with wm8960.
Signed-off-by: Maso Huang <maso.huang@mediatek.com>
---
sound/soc/mediatek/Kconfig | 10 ++
sound/soc/mediatek/mt7986/Makefile | 1 +
sound/soc/mediatek/mt7986/mt7986-wm8960.c | 184 ++++++++++++++++++++++
3 files changed, 195 insertions(+)
create mode 100644 sound/soc/mediatek/mt7986/mt7986-wm8960.c
diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig
index 558827755a8d..8d1bc8814486 100644
--- a/sound/soc/mediatek/Kconfig
+++ b/sound/soc/mediatek/Kconfig
@@ -64,6 +64,16 @@ config SND_SOC_MT7986
Select Y if you have such device.
If unsure select "N".
+config SND_SOC_MT7986_WM8960
+ tristate "ASoc Audio driver for MT7986 with WM8960 codec"
+ depends on SND_SOC_MT7986 && I2C
+ select SND_SOC_WM8960
+ help
+ This adds support for ASoC machine driver for MediaTek MT7986
+ boards with the WM8960 codecs.
+ Select Y if you have such device.
+ If unsure select "N".
+
config SND_SOC_MT8173
tristate "ASoC support for Mediatek MT8173 chip"
depends on ARCH_MEDIATEK
diff --git a/sound/soc/mediatek/mt7986/Makefile b/sound/soc/mediatek/mt7986/Makefile
index de0742a67cae..fc4c82559b29 100644
--- a/sound/soc/mediatek/mt7986/Makefile
+++ b/sound/soc/mediatek/mt7986/Makefile
@@ -6,3 +6,4 @@ snd-soc-mt7986-afe-objs := \
mt7986-dai-etdm.o
obj-$(CONFIG_SND_SOC_MT7986) += snd-soc-mt7986-afe.o
+obj-$(CONFIG_SND_SOC_MT7986_WM8960) += mt7986-wm8960.o
diff --git a/sound/soc/mediatek/mt7986/mt7986-wm8960.c b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
new file mode 100644
index 000000000000..a880fcb8662e
--- /dev/null
+++ b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mt7986-wm8960.c -- MT7986-WM8960 ALSA SoC machine driver
+ *
+ * Copyright (c) 2021 MediaTek Inc.
+ * Author: Vic Wu <vic.wu@mediatek.com>
+ * Maso Huang <maso.huang@mediatek.com>
+ */
+
+#include <linux/module.h>
+#include <sound/soc.h>
+
+#include "mt7986-afe-common.h"
+
+struct mt7986_wm8960_priv {
+ struct device_node *platform_node;
+ struct device_node *codec_node;
+};
+
+static const struct snd_soc_dapm_widget mt7986_wm8960_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone", NULL),
+ SND_SOC_DAPM_MIC("AMIC", NULL),
+};
+
+static const struct snd_kcontrol_new mt7986_wm8960_controls[] = {
+ SOC_DAPM_PIN_SWITCH("Headphone"),
+ SOC_DAPM_PIN_SWITCH("AMIC"),
+};
+
+SND_SOC_DAILINK_DEFS(playback,
+ DAILINK_COMP_ARRAY(COMP_CPU("DL1")),
+ DAILINK_COMP_ARRAY(COMP_DUMMY()),
+ DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(capture,
+ DAILINK_COMP_ARRAY(COMP_CPU("UL1")),
+ DAILINK_COMP_ARRAY(COMP_DUMMY()),
+ DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(codec,
+ DAILINK_COMP_ARRAY(COMP_CPU("ETDM")),
+ DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "wm8960-hifi")),
+ DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+static struct snd_soc_dai_link mt7986_wm8960_dai_links[] = {
+ /* FE */
+ {
+ .name = "wm8960-playback",
+ .stream_name = "wm8960-playback",
+ .trigger = {SND_SOC_DPCM_TRIGGER_POST,
+ SND_SOC_DPCM_TRIGGER_POST},
+ .dynamic = 1,
+ .dpcm_playback = 1,
+ SND_SOC_DAILINK_REG(playback),
+ },
+ {
+ .name = "wm8960-capture",
+ .stream_name = "wm8960-capture",
+ .trigger = {SND_SOC_DPCM_TRIGGER_POST,
+ SND_SOC_DPCM_TRIGGER_POST},
+ .dynamic = 1,
+ .dpcm_capture = 1,
+ SND_SOC_DAILINK_REG(capture),
+ },
+ /* BE */
+ {
+ .name = "wm8960-codec",
+ .no_pcm = 1,
+ .dai_fmt = SND_SOC_DAIFMT_I2S |
+ SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS |
+ SND_SOC_DAIFMT_GATED,
+ .dpcm_playback = 1,
+ .dpcm_capture = 1,
+ SND_SOC_DAILINK_REG(codec),
+ },
+};
+
+static struct snd_soc_card mt7986_wm8960_card = {
+ .name = "mt7986-wm8960",
+ .owner = THIS_MODULE,
+ .dai_link = mt7986_wm8960_dai_links,
+ .num_links = ARRAY_SIZE(mt7986_wm8960_dai_links),
+ .controls = mt7986_wm8960_controls,
+ .num_controls = ARRAY_SIZE(mt7986_wm8960_controls),
+ .dapm_widgets = mt7986_wm8960_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(mt7986_wm8960_widgets),
+};
+
+static int mt7986_wm8960_machine_probe(struct platform_device *pdev)
+{
+ struct snd_soc_card *card = &mt7986_wm8960_card;
+ struct snd_soc_dai_link *dai_link;
+ struct mt7986_wm8960_priv *priv;
+ int ret, i;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->platform_node = of_parse_phandle(pdev->dev.of_node,
+ "mediatek,platform", 0);
+ if (!priv->platform_node) {
+ dev_err(&pdev->dev, "Property 'platform' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ for_each_card_prelinks(card, i, dai_link) {
+ if (dai_link->platforms->name)
+ continue;
+ dai_link->platforms->of_node = priv->platform_node;
+ }
+
+ card->dev = &pdev->dev;
+
+ priv->codec_node = of_parse_phandle(pdev->dev.of_node,
+ "mediatek,audio-codec", 0);
+ if (!priv->codec_node) {
+ dev_err(&pdev->dev,
+ "Property 'audio-codec' missing or invalid\n");
+ of_node_put(priv->platform_node);
+ return -EINVAL;
+ }
+
+ for_each_card_prelinks(card, i, dai_link) {
+ if (dai_link->codecs->name)
+ continue;
+ dai_link->codecs->of_node = priv->codec_node;
+ }
+
+ ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
+ if (ret) {
+ dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
+ goto err_of_node_put;
+ }
+
+ ret = devm_snd_soc_register_card(&pdev->dev, card);
+ if (ret) {
+ dev_err(&pdev->dev, "%s snd_soc_register_card fail %d\n",
+ __func__, ret);
+ goto err_of_node_put;
+ }
+
+err_of_node_put:
+ of_node_put(priv->codec_node);
+ of_node_put(priv->platform_node);
+ return ret;
+}
+
+static void mt7986_wm8960_machine_remove(struct platform_device *pdev)
+{
+ struct snd_soc_card *card = platform_get_drvdata(pdev);
+ struct mt7986_wm8960_priv *priv = snd_soc_card_get_drvdata(card);
+
+ of_node_put(priv->codec_node);
+ of_node_put(priv->platform_node);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id mt7986_wm8960_machine_dt_match[] = {
+ {.compatible = "mediatek,mt7986-wm8960-machine",},
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mt7986_wm8960_machine_dt_match);
+#endif
+
+static struct platform_driver mt7986_wm8960_machine = {
+ .driver = {
+ .name = "mt7986-wm8960",
+#ifdef CONFIG_OF
+ .of_match_table = mt7986_wm8960_machine_dt_match,
+#endif
+ },
+ .probe = mt7986_wm8960_machine_probe,
+ .remove_new = mt7986_wm8960_machine_remove,
+};
+
+module_platform_driver(mt7986_wm8960_machine);
+
+/* Module information */
+MODULE_DESCRIPTION("MT7986 WM8960 ALSA SoC machine driver");
+MODULE_AUTHOR("Vic Wu <vic.wu@mediatek.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("mt7986 wm8960 soc card");
--
2.18.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 4/6] ASoC: mediatek: mt7986: add machine driver with wm8960
2023-07-28 9:08 ` [PATCH v3 4/6] ASoC: mediatek: mt7986: add machine driver with wm8960 Maso Huang
@ 2023-07-28 9:49 ` AngeloGioacchino Del Regno
2023-07-28 9:52 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 33+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-28 9:49 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Il 28/07/23 11:08, Maso Huang ha scritto:
> Add support for mt7986 board with wm8960.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> ---
> sound/soc/mediatek/Kconfig | 10 ++
> sound/soc/mediatek/mt7986/Makefile | 1 +
> sound/soc/mediatek/mt7986/mt7986-wm8960.c | 184 ++++++++++++++++++++++
> 3 files changed, 195 insertions(+)
> create mode 100644 sound/soc/mediatek/mt7986/mt7986-wm8960.c
>
> diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig
> index 558827755a8d..8d1bc8814486 100644
> --- a/sound/soc/mediatek/Kconfig
> +++ b/sound/soc/mediatek/Kconfig
> @@ -64,6 +64,16 @@ config SND_SOC_MT7986
> Select Y if you have such device.
> If unsure select "N".
>
> +config SND_SOC_MT7986_WM8960
> + tristate "ASoc Audio driver for MT7986 with WM8960 codec"
> + depends on SND_SOC_MT7986 && I2C
> + select SND_SOC_WM8960
> + help
> + This adds support for ASoC machine driver for MediaTek MT7986
> + boards with the WM8960 codecs.
> + Select Y if you have such device.
> + If unsure select "N".
> +
> config SND_SOC_MT8173
> tristate "ASoC support for Mediatek MT8173 chip"
> depends on ARCH_MEDIATEK
> diff --git a/sound/soc/mediatek/mt7986/Makefile b/sound/soc/mediatek/mt7986/Makefile
> index de0742a67cae..fc4c82559b29 100644
> --- a/sound/soc/mediatek/mt7986/Makefile
> +++ b/sound/soc/mediatek/mt7986/Makefile
> @@ -6,3 +6,4 @@ snd-soc-mt7986-afe-objs := \
> mt7986-dai-etdm.o
>
> obj-$(CONFIG_SND_SOC_MT7986) += snd-soc-mt7986-afe.o
> +obj-$(CONFIG_SND_SOC_MT7986_WM8960) += mt7986-wm8960.o
> diff --git a/sound/soc/mediatek/mt7986/mt7986-wm8960.c b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
> new file mode 100644
> index 000000000000..a880fcb8662e
> --- /dev/null
> +++ b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
..snip..
> +static int mt7986_wm8960_machine_probe(struct platform_device *pdev)
> +{
> + struct snd_soc_card *card = &mt7986_wm8960_card;
> + struct snd_soc_dai_link *dai_link;
> + struct mt7986_wm8960_priv *priv;
> + int ret, i;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->platform_node = of_parse_phandle(pdev->dev.of_node,
> + "mediatek,platform", 0);
> + if (!priv->platform_node) {
> + dev_err(&pdev->dev, "Property 'platform' missing or invalid\n");
> + return -EINVAL;
> + }
> +
> + for_each_card_prelinks(card, i, dai_link) {
> + if (dai_link->platforms->name)
> + continue;
> + dai_link->platforms->of_node = priv->platform_node;
> + }
> +
> + card->dev = &pdev->dev;
> +
> + priv->codec_node = of_parse_phandle(pdev->dev.of_node,
> + "mediatek,audio-codec", 0);
> + if (!priv->codec_node) {
> + dev_err(&pdev->dev,
> + "Property 'audio-codec' missing or invalid\n");
> + of_node_put(priv->platform_node);
> + return -EINVAL;
> + }
> +
> + for_each_card_prelinks(card, i, dai_link) {
> + if (dai_link->codecs->name)
> + continue;
> + dai_link->codecs->of_node = priv->codec_node;
> + }
> +
> + ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
> + if (ret) {
> + dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
> + goto err_of_node_put;
> + }
> +
> + ret = devm_snd_soc_register_card(&pdev->dev, card);
> + if (ret) {
> + dev_err(&pdev->dev, "%s snd_soc_register_card fail %d\n",
> + __func__, ret);
> + goto err_of_node_put;
> + }
> +
> +err_of_node_put:
> + of_node_put(priv->codec_node);
> + of_node_put(priv->platform_node);
> + return ret;
> +}
> +
> +static void mt7986_wm8960_machine_remove(struct platform_device *pdev)
> +{
> + struct snd_soc_card *card = platform_get_drvdata(pdev);
> + struct mt7986_wm8960_priv *priv = snd_soc_card_get_drvdata(card);
> +
> + of_node_put(priv->codec_node);
> + of_node_put(priv->platform_node);
> +}
> +
> +#ifdef CONFIG_OF
Your probe function *relies on* devicetree, and you're adding an ifdef for
CONFIG_OF? That wouldn't make sense, would it? ;-)
> +static const struct of_device_id mt7986_wm8960_machine_dt_match[] = {
> + {.compatible = "mediatek,mt7986-wm8960-machine",},
please...
{ .compatible = "mediatek,mt7986-wm8960-machine" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mt7986_wm8960_machine_dt_match);
> +#endif
> +
> +static struct platform_driver mt7986_wm8960_machine = {
> + .driver = {
> + .name = "mt7986-wm8960",
> +#ifdef CONFIG_OF
Check `struct device_driver`: const struct of_device_id *of_match_table is
always present, there's no ifdef.... and that's done in order to avoid seeing
a bunch of ifdefs in drivers...
...so, why is this callback enclosed in an ifdef?
Please drop all those ifdefs.
After addressing those last comments, you can get my
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Regards,
Angelo
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 4/6] ASoC: mediatek: mt7986: add machine driver with wm8960
2023-07-28 9:49 ` AngeloGioacchino Del Regno
@ 2023-07-28 9:52 ` AngeloGioacchino Del Regno
2023-07-31 6:50 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-28 9:52 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Il 28/07/23 11:49, AngeloGioacchino Del Regno ha scritto:
> Il 28/07/23 11:08, Maso Huang ha scritto:
>> Add support for mt7986 board with wm8960.
>>
>> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
>> ---
>> sound/soc/mediatek/Kconfig | 10 ++
>> sound/soc/mediatek/mt7986/Makefile | 1 +
>> sound/soc/mediatek/mt7986/mt7986-wm8960.c | 184 ++++++++++++++++++++++
>> 3 files changed, 195 insertions(+)
>> create mode 100644 sound/soc/mediatek/mt7986/mt7986-wm8960.c
>>
>> diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig
>> index 558827755a8d..8d1bc8814486 100644
>> --- a/sound/soc/mediatek/Kconfig
>> +++ b/sound/soc/mediatek/Kconfig
>> @@ -64,6 +64,16 @@ config SND_SOC_MT7986
>> Select Y if you have such device.
>> If unsure select "N".
>> +config SND_SOC_MT7986_WM8960
>> + tristate "ASoc Audio driver for MT7986 with WM8960 codec"
>> + depends on SND_SOC_MT7986 && I2C
>> + select SND_SOC_WM8960
>> + help
>> + This adds support for ASoC machine driver for MediaTek MT7986
>> + boards with the WM8960 codecs.
>> + Select Y if you have such device.
>> + If unsure select "N".
>> +
>> config SND_SOC_MT8173
>> tristate "ASoC support for Mediatek MT8173 chip"
>> depends on ARCH_MEDIATEK
>> diff --git a/sound/soc/mediatek/mt7986/Makefile b/sound/soc/mediatek/mt7986/Makefile
>> index de0742a67cae..fc4c82559b29 100644
>> --- a/sound/soc/mediatek/mt7986/Makefile
>> +++ b/sound/soc/mediatek/mt7986/Makefile
>> @@ -6,3 +6,4 @@ snd-soc-mt7986-afe-objs := \
>> mt7986-dai-etdm.o
>> obj-$(CONFIG_SND_SOC_MT7986) += snd-soc-mt7986-afe.o
>> +obj-$(CONFIG_SND_SOC_MT7986_WM8960) += mt7986-wm8960.o
>> diff --git a/sound/soc/mediatek/mt7986/mt7986-wm8960.c
>> b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
>> new file mode 100644
>> index 000000000000..a880fcb8662e
>> --- /dev/null
>> +++ b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
>
> ..snip..
>
>> +static int mt7986_wm8960_machine_probe(struct platform_device *pdev)
>> +{
>> + struct snd_soc_card *card = &mt7986_wm8960_card;
>> + struct snd_soc_dai_link *dai_link;
>> + struct mt7986_wm8960_priv *priv;
>> + int ret, i;
>> +
>> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + priv->platform_node = of_parse_phandle(pdev->dev.of_node,
>> + "mediatek,platform", 0);
>> + if (!priv->platform_node) {
>> + dev_err(&pdev->dev, "Property 'platform' missing or invalid\n");
>> + return -EINVAL;
>> + }
>> +
>> + for_each_card_prelinks(card, i, dai_link) {
>> + if (dai_link->platforms->name)
>> + continue;
>> + dai_link->platforms->of_node = priv->platform_node;
>> + }
>> +
>> + card->dev = &pdev->dev;
>> +
>> + priv->codec_node = of_parse_phandle(pdev->dev.of_node,
>> + "mediatek,audio-codec", 0);
>> + if (!priv->codec_node) {
>> + dev_err(&pdev->dev,
>> + "Property 'audio-codec' missing or invalid\n");
>> + of_node_put(priv->platform_node);
>> + return -EINVAL;
>> + }
>> +
>> + for_each_card_prelinks(card, i, dai_link) {
>> + if (dai_link->codecs->name)
>> + continue;
>> + dai_link->codecs->of_node = priv->codec_node;
>> + }
>> +
>> + ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
>> + goto err_of_node_put;
>> + }
>> +
>> + ret = devm_snd_soc_register_card(&pdev->dev, card);
>> + if (ret) {
>> + dev_err(&pdev->dev, "%s snd_soc_register_card fail %d\n",
>> + __func__, ret);
>> + goto err_of_node_put;
>> + }
>> +
>> +err_of_node_put:
>> + of_node_put(priv->codec_node);
>> + of_node_put(priv->platform_node);
>> + return ret;
>> +}
>> +
>> +static void mt7986_wm8960_machine_remove(struct platform_device *pdev)
>> +{
>> + struct snd_soc_card *card = platform_get_drvdata(pdev);
>> + struct mt7986_wm8960_priv *priv = snd_soc_card_get_drvdata(card);
>> +
>> + of_node_put(priv->codec_node);
>> + of_node_put(priv->platform_node);
>> +}
>> +
>> +#ifdef CONFIG_OF
>
> Your probe function *relies on* devicetree, and you're adding an ifdef for
> CONFIG_OF? That wouldn't make sense, would it? ;-)
>
>> +static const struct of_device_id mt7986_wm8960_machine_dt_match[] = {
>> + {.compatible = "mediatek,mt7986-wm8960-machine",},
>
> please...
>
> { .compatible = "mediatek,mt7986-wm8960-machine" },
Actually, I noticed that just after sending the review.
Can you also please change the compatible, as "machine" doesn't really
mean "sound"? :-)
.compatible = "mediatek,mt7986-wm8960-sound"
Thanks!
>
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, mt7986_wm8960_machine_dt_match);
>> +#endif
>> +
>> +static struct platform_driver mt7986_wm8960_machine = {
>> + .driver = {
>> + .name = "mt7986-wm8960",
>> +#ifdef CONFIG_OF
>
> Check `struct device_driver`: const struct of_device_id *of_match_table is
> always present, there's no ifdef.... and that's done in order to avoid seeing
> a bunch of ifdefs in drivers...
>
> ...so, why is this callback enclosed in an ifdef?
>
> Please drop all those ifdefs.
>
>
> After addressing those last comments, you can get my
>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> Regards,
> Angelo
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 4/6] ASoC: mediatek: mt7986: add machine driver with wm8960
2023-07-28 9:52 ` AngeloGioacchino Del Regno
@ 2023-07-31 6:50 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-07-31 6:50 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-07-28 at 11:52 +0200, AngeloGioacchino Del Regno wrote:
> Il 28/07/23 11:49, AngeloGioacchino Del Regno ha scritto:
> > Il 28/07/23 11:08, Maso Huang ha scritto:
> > > Add support for mt7986 board with wm8960.
> > >
> > > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > > ---
> > > sound/soc/mediatek/Kconfig | 10 ++
> > > sound/soc/mediatek/mt7986/Makefile | 1 +
> > > sound/soc/mediatek/mt7986/mt7986-wm8960.c | 184
> > > ++++++++++++++++++++++
> > > 3 files changed, 195 insertions(+)
> > > create mode 100644 sound/soc/mediatek/mt7986/mt7986-wm8960.c
> > >
> > > diff --git a/sound/soc/mediatek/Kconfig
> > > b/sound/soc/mediatek/Kconfig
> > > index 558827755a8d..8d1bc8814486 100644
> > > --- a/sound/soc/mediatek/Kconfig
> > > +++ b/sound/soc/mediatek/Kconfig
> > > @@ -64,6 +64,16 @@ config SND_SOC_MT7986
> > > Select Y if you have such device.
> > > If unsure select "N".
> > > +config SND_SOC_MT7986_WM8960
> > > + tristate "ASoc Audio driver for MT7986 with WM8960 codec"
> > > + depends on SND_SOC_MT7986 && I2C
> > > + select SND_SOC_WM8960
> > > + help
> > > + This adds support for ASoC machine driver for MediaTek
> > > MT7986
> > > + boards with the WM8960 codecs.
> > > + Select Y if you have such device.
> > > + If unsure select "N".
> > > +
> > > config SND_SOC_MT8173
> > > tristate "ASoC support for Mediatek MT8173 chip"
> > > depends on ARCH_MEDIATEK
> > > diff --git a/sound/soc/mediatek/mt7986/Makefile
> > > b/sound/soc/mediatek/mt7986/Makefile
> > > index de0742a67cae..fc4c82559b29 100644
> > > --- a/sound/soc/mediatek/mt7986/Makefile
> > > +++ b/sound/soc/mediatek/mt7986/Makefile
> > > @@ -6,3 +6,4 @@ snd-soc-mt7986-afe-objs := \
> > > mt7986-dai-etdm.o
> > > obj-$(CONFIG_SND_SOC_MT7986) += snd-soc-mt7986-afe.o
> > > +obj-$(CONFIG_SND_SOC_MT7986_WM8960) += mt7986-wm8960.o
> > > diff --git a/sound/soc/mediatek/mt7986/mt7986-wm8960.c
> > > b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
> > > new file mode 100644
> > > index 000000000000..a880fcb8662e
> > > --- /dev/null
> > > +++ b/sound/soc/mediatek/mt7986/mt7986-wm8960.c
> >
> > ..snip..
> >
> > > +static int mt7986_wm8960_machine_probe(struct platform_device
> > > *pdev)
> > > +{
> > > + struct snd_soc_card *card = &mt7986_wm8960_card;
> > > + struct snd_soc_dai_link *dai_link;
> > > + struct mt7986_wm8960_priv *priv;
> > > + int ret, i;
> > > +
> > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > > + if (!priv)
> > > + return -ENOMEM;
> > > +
> > > + priv->platform_node = of_parse_phandle(pdev->dev.of_node,
> > > + "mediatek,platform", 0);
> > > + if (!priv->platform_node) {
> > > + dev_err(&pdev->dev, "Property 'platform' missing or
> > > invalid\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + for_each_card_prelinks(card, i, dai_link) {
> > > + if (dai_link->platforms->name)
> > > + continue;
> > > + dai_link->platforms->of_node = priv->platform_node;
> > > + }
> > > +
> > > + card->dev = &pdev->dev;
> > > +
> > > + priv->codec_node = of_parse_phandle(pdev->dev.of_node,
> > > + "mediatek,audio-codec", 0);
> > > + if (!priv->codec_node) {
> > > + dev_err(&pdev->dev,
> > > + "Property 'audio-codec' missing or invalid\n");
> > > + of_node_put(priv->platform_node);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + for_each_card_prelinks(card, i, dai_link) {
> > > + if (dai_link->codecs->name)
> > > + continue;
> > > + dai_link->codecs->of_node = priv->codec_node;
> > > + }
> > > +
> > > + ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "failed to parse audio-routing:
> > > %d\n", ret);
> > > + goto err_of_node_put;
> > > + }
> > > +
> > > + ret = devm_snd_soc_register_card(&pdev->dev, card);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "%s snd_soc_register_card fail
> > > %d\n",
> > > + __func__, ret);
> > > + goto err_of_node_put;
> > > + }
> > > +
> > > +err_of_node_put:
> > > + of_node_put(priv->codec_node);
> > > + of_node_put(priv->platform_node);
> > > + return ret;
> > > +}
> > > +
> > > +static void mt7986_wm8960_machine_remove(struct platform_device
> > > *pdev)
> > > +{
> > > + struct snd_soc_card *card = platform_get_drvdata(pdev);
> > > + struct mt7986_wm8960_priv *priv =
> > > snd_soc_card_get_drvdata(card);
> > > +
> > > + of_node_put(priv->codec_node);
> > > + of_node_put(priv->platform_node);
> > > +}
> > > +
> > > +#ifdef CONFIG_OF
> >
> > Your probe function *relies on* devicetree, and you're adding an
> > ifdef for
> > CONFIG_OF? That wouldn't make sense, would it? ;-)
> >
Hi Angelo,
Thanks for your review.
It seems we don't need CONFIG_OF here, so I'll remove it in v4 patch.
> > > +static const struct of_device_id
> > > mt7986_wm8960_machine_dt_match[] = {
> > > + {.compatible = "mediatek,mt7986-wm8960-machine",},
> >
> > please...
> >
> > { .compatible = "mediatek,mt7986-wm8960-machine" },
>
> Actually, I noticed that just after sending the review.
>
> Can you also please change the compatible, as "machine" doesn't
> really
> mean "sound"? :-)
>
> .compatible = "mediatek,mt7986-wm8960-sound"
>
> Thanks!
>
For compatible suffix, I'll discuss with you in PATCH v3[5/6] mail, and
will refine this with [5/6] mail conclusion :)
> >
> > > + { /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mt7986_wm8960_machine_dt_match);
> > > +#endif
> > > +
> > > +static struct platform_driver mt7986_wm8960_machine = {
> > > + .driver = {
> > > + .name = "mt7986-wm8960",
> > > +#ifdef CONFIG_OF
> >
> > Check `struct device_driver`: const struct of_device_id
> > *of_match_table is
> > always present, there's no ifdef.... and that's done in order to
> > avoid seeing
> > a bunch of ifdefs in drivers...
> >
> > ...so, why is this callback enclosed in an ifdef?
> >
> > Please drop all those ifdefs.
> >
> >
> > After addressing those last comments, you can get my
> >
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> >
> > Regards,
> > Angelo
>
>
No more needed for the ifdef, so I'll drop them in v4 patch :)
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-28 9:08 [PATCH v3 0/6] ASoC: mediatek: Add support for MT7986 SoC Maso Huang
` (3 preceding siblings ...)
2023-07-28 9:08 ` [PATCH v3 4/6] ASoC: mediatek: mt7986: add machine driver with wm8960 Maso Huang
@ 2023-07-28 9:08 ` Maso Huang
2023-07-28 9:55 ` AngeloGioacchino Del Regno
2023-07-28 12:49 ` Krzysztof Kozlowski
2023-07-28 9:08 ` [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document Maso Huang
5 siblings, 2 replies; 33+ messages in thread
From: Maso Huang @ 2023-07-28 9:08 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Maso Huang
Add document for mt7986 board with wm8960.
Signed-off-by: Maso Huang <maso.huang@mediatek.com>
---
.../sound/mediatek,mt7986-wm8960.yaml | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
new file mode 100644
index 000000000000..76394f7e5502
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MT7986 sound card with WM8960 codec
+
+maintainers:
+ - Maso Huang <maso.huang@mediatek.com>
+
+properties:
+ compatible:
+ const: mediatek,mt7986-wm8960-machine
+
+ mediatek,platform:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: The phandle of MT7986 platform.
+
+ audio-routing:
+ $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+ description:
+ A list of the connections between audio components. Each entry is a
+ sink/source pair of strings. Valid names could be the input or output
+ widgets of audio components, power supplies, MicBias of codec and the
+ software switch.
+
+ mediatek,audio-codec:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: The phandle of wm8960 codec.
+
+additionalProperties: false
+
+required:
+ - compatible
+ - mediatek,platform
+ - audio-routing
+ - mediatek,audio-codec
+
+examples:
+ - |
+ sound {
+ compatible = "mediatek,mt7986-wm8960-machine";
+ mediatek,platform = <&afe>;
+ audio-routing =
+ "Headphone", "HP_L",
+ "Headphone", "HP_R",
+ "LINPUT1", "AMIC",
+ "RINPUT1", "AMIC";
+ mediatek,audio-codec = <&wm8960>;
+ };
+
+...
--
2.18.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-28 9:08 ` [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document Maso Huang
@ 2023-07-28 9:55 ` AngeloGioacchino Del Regno
2023-07-28 10:16 ` Maso Huang (黃加竹)
2023-07-28 12:49 ` Krzysztof Kozlowski
1 sibling, 1 reply; 33+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-28 9:55 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Il 28/07/23 11:08, Maso Huang ha scritto:
> Add document for mt7986 board with wm8960.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> ---
> .../sound/mediatek,mt7986-wm8960.yaml | 53 +++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> new file mode 100644
> index 000000000000..76394f7e5502
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MT7986 sound card with WM8960 codec
> +
> +maintainers:
> + - Maso Huang <maso.huang@mediatek.com>
> +
> +properties:
> + compatible:
> + const: mediatek,mt7986-wm8960-machine
mediatek,mt7986-wm8960-sound looks better.
After which,
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-28 9:55 ` AngeloGioacchino Del Regno
@ 2023-07-28 10:16 ` Maso Huang (黃加竹)
2023-07-31 6:42 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-07-28 10:16 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-07-28 at 11:55 +0200, AngeloGioacchino Del Regno wrote:
> Il 28/07/23 11:08, Maso Huang ha scritto:
> > Add document for mt7986 board with wm8960.
> >
> > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > ---
> > .../sound/mediatek,mt7986-wm8960.yaml | 53
> > +++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > wm8960.yaml
> > b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > wm8960.yaml
> > new file mode 100644
> > index 000000000000..76394f7e5502
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > wm8960.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPPRaROt0$
> >
> > +$schema:
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPttZILAo$
> >
> > +
> > +title: MediaTek MT7986 sound card with WM8960 codec
> > +
> > +maintainers:
> > + - Maso Huang <maso.huang@mediatek.com>
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,mt7986-wm8960-machine
>
> mediatek,mt7986-wm8960-sound looks better.
>
> After which,
>
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
>
>
Hi Angelo,
Thanks for your review.
I'll change the compatible to "mediatek,mt7986-wm8960-sound" in v4
patch.
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-28 10:16 ` Maso Huang (黃加竹)
@ 2023-07-31 6:42 ` Maso Huang (黃加竹)
2023-07-31 10:44 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-07-31 6:42 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-07-28 at 18:16 +0800, Maso Huang wrote:
> On Fri, 2023-07-28 at 11:55 +0200, AngeloGioacchino Del Regno wrote:
> > Il 28/07/23 11:08, Maso Huang ha scritto:
> > > Add document for mt7986 board with wm8960.
> > >
> > > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > > ---
> > > .../sound/mediatek,mt7986-wm8960.yaml | 53
> > > +++++++++++++++++++
> > > 1 file changed, 53 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > wm8960.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > wm8960.yaml
> > > b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > wm8960.yaml
> > > new file mode 100644
> > > index 000000000000..76394f7e5502
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > wm8960.yaml
> > > @@ -0,0 +1,53 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id:
> > >
https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPPRaROt0$
> > >
> > >
> > > +$schema:
> > >
https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPttZILAo$
> > >
> > >
> > > +
> > > +title: MediaTek MT7986 sound card with WM8960 codec
> > > +
> > > +maintainers:
> > > + - Maso Huang <maso.huang@mediatek.com>
> > > +
> > > +properties:
> > > + compatible:
> > > + const: mediatek,mt7986-wm8960-machine
> >
> > mediatek,mt7986-wm8960-sound looks better.
> >
> > After which,
> >
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> >
> >
>
> Hi Angelo,
>
> Thanks for your review.
> I'll change the compatible to "mediatek,mt7986-wm8960-sound" in v4
> patch.
>
> Best regards,
> Maso
>
Hi Angelo,
One more question for this compatible.
The suffix "machine" means alsa machine driver for mt7986-wm8960. It
might be better to use "machine" here.
Or you prefer "sound" than "machine"?
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-31 6:42 ` Maso Huang (黃加竹)
@ 2023-07-31 10:44 ` AngeloGioacchino Del Regno
2023-07-31 12:38 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-31 10:44 UTC (permalink / raw)
To: Maso Huang (黃加竹),
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de, alsa-devel@alsa-project.org
Il 31/07/23 08:42, Maso Huang (黃加竹) ha scritto:
> On Fri, 2023-07-28 at 18:16 +0800, Maso Huang wrote:
>> On Fri, 2023-07-28 at 11:55 +0200, AngeloGioacchino Del Regno wrote:
>>> Il 28/07/23 11:08, Maso Huang ha scritto:
>>>> Add document for mt7986 board with wm8960.
>>>>
>>>> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
>>>> ---
>>>> .../sound/mediatek,mt7986-wm8960.yaml | 53
>>>> +++++++++++++++++++
>>>> 1 file changed, 53 insertions(+)
>>>> create mode 100644
>>>> Documentation/devicetree/bindings/sound/mediatek,mt7986-
>>>> wm8960.yaml
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/sound/mediatek,mt7986-
>>>> wm8960.yaml
>>>> b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
>>>> wm8960.yaml
>>>> new file mode 100644
>>>> index 000000000000..76394f7e5502
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
>>>> wm8960.yaml
>>>> @@ -0,0 +1,53 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id:
>>>>
> https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPPRaROt0$
>>>>
>>>>
>>>> +$schema:
>>>>
> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPttZILAo$
>>>>
>>>>
>>>> +
>>>> +title: MediaTek MT7986 sound card with WM8960 codec
>>>> +
>>>> +maintainers:
>>>> + - Maso Huang <maso.huang@mediatek.com>
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + const: mediatek,mt7986-wm8960-machine
>>>
>>> mediatek,mt7986-wm8960-sound looks better.
>>>
>>> After which,
>>>
>>> Reviewed-by: AngeloGioacchino Del Regno <
>>> angelogioacchino.delregno@collabora.com>
>>>
>>>
>>
>> Hi Angelo,
>>
>> Thanks for your review.
>> I'll change the compatible to "mediatek,mt7986-wm8960-sound" in v4
>> patch.
>>
>> Best regards,
>> Maso
>>
>
> Hi Angelo,
>
> One more question for this compatible.
>
> The suffix "machine" means alsa machine driver for mt7986-wm8960. It
> might be better to use "machine" here.
>
> Or you prefer "sound" than "machine"?
>
> Best regards,
> Maso
I prefer "sound" because of consistency with other MediaTek machine driver
compatible strings.
Regards,
Angelo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-31 10:44 ` AngeloGioacchino Del Regno
@ 2023-07-31 12:38 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-07-31 12:38 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
robh+dt@kernel.org, chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Mon, 2023-07-31 at 12:44 +0200, AngeloGioacchino Del Regno wrote:
> Il 31/07/23 08:42, Maso Huang (黃加竹) ha scritto:
> > On Fri, 2023-07-28 at 18:16 +0800, Maso Huang wrote:
> > > On Fri, 2023-07-28 at 11:55 +0200, AngeloGioacchino Del Regno
> > > wrote:
> > > > Il 28/07/23 11:08, Maso Huang ha scritto:
> > > > > Add document for mt7986 board with wm8960.
> > > > >
> > > > > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > > > > ---
> > > > > .../sound/mediatek,mt7986-wm8960.yaml | 53
> > > > > +++++++++++++++++++
> > > > > 1 file changed, 53 insertions(+)
> > > > > create mode 100644
> > > > > Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > > > wm8960.yaml
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > > > wm8960.yaml
> > > > > b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > > > wm8960.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..76394f7e5502
> > > > > --- /dev/null
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > > > > wm8960.yaml
> > > > > @@ -0,0 +1,53 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id:
> > > > >
> >
> >
https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPPRaROt0$
> > > > >
> > > > >
> > > > > +$schema:
> > > > >
> >
> >
https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!lu4Z6pJeRiL7-8l4T3ptqUCM54FnHTBiyh5KWBNqSjl6mOOI7WmzHpWEd-ZSZ-2NJ4Cs9PPaAF_75ywo2SKW16MPttZILAo$
> > > > >
> > > > >
> > > > > +
> > > > > +title: MediaTek MT7986 sound card with WM8960 codec
> > > > > +
> > > > > +maintainers:
> > > > > + - Maso Huang <maso.huang@mediatek.com>
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + const: mediatek,mt7986-wm8960-machine
> > > >
> > > > mediatek,mt7986-wm8960-sound looks better.
> > > >
> > > > After which,
> > > >
> > > > Reviewed-by: AngeloGioacchino Del Regno <
> > > > angelogioacchino.delregno@collabora.com>
> > > >
> > > >
> > >
> > > Hi Angelo,
> > >
> > > Thanks for your review.
> > > I'll change the compatible to "mediatek,mt7986-wm8960-sound" in
> > > v4
> > > patch.
> > >
> > > Best regards,
> > > Maso
> > >
> >
> > Hi Angelo,
> >
> > One more question for this compatible.
> >
> > The suffix "machine" means alsa machine driver for mt7986-wm8960.
> > It
> > might be better to use "machine" here.
> >
> > Or you prefer "sound" than "machine"?
> >
> > Best regards,
> > Maso
>
> I prefer "sound" because of consistency with other MediaTek machine
> driver
> compatible strings.
>
> Regards,
> Angelo
Hi Angelo,
No problem, I'll use "mediatek,mt7986-wm8960-sound" as compatible in v4
patch :)
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-28 9:08 ` [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document Maso Huang
2023-07-28 9:55 ` AngeloGioacchino Del Regno
@ 2023-07-28 12:49 ` Krzysztof Kozlowski
2023-07-31 7:31 ` Maso Huang (黃加竹)
1 sibling, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-28 12:49 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Jaroslav Kysela, Takashi Iwai,
Trevor Wu, Arnd Bergmann, Mars Chen, Allen-KH Cheng, alsa-devel,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
On 28/07/2023 11:08, Maso Huang wrote:
> Add document for mt7986 board with wm8960.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> ---
> .../sound/mediatek,mt7986-wm8960.yaml | 53 +++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> new file mode 100644
> index 000000000000..76394f7e5502
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MT7986 sound card with WM8960 codec
> +
> +maintainers:
> + - Maso Huang <maso.huang@mediatek.com>
> +
> +properties:
> + compatible:
> + const: mediatek,mt7986-wm8960-machine
> +
> + mediatek,platform:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: The phandle of MT7986 platform.
> +
> + audio-routing:
> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> + description:
> + A list of the connections between audio components. Each entry is a
> + sink/source pair of strings. Valid names could be the input or output
> + widgets of audio components, power supplies, MicBias of codec and the
> + software switch.
> +
> + mediatek,audio-codec:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: The phandle of wm8960 codec.
> +
How did you implement Rob's comment? Or did you just ignore it?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-28 12:49 ` Krzysztof Kozlowski
@ 2023-07-31 7:31 ` Maso Huang (黃加竹)
2023-07-31 8:14 ` Krzysztof Kozlowski
0 siblings, 1 reply; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-07-31 7:31 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
krzysztof.kozlowski@linaro.org, broonie@kernel.org,
Allen-KH Cheng (程冠勲), conor+dt@kernel.org,
tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-07-28 at 14:49 +0200, Krzysztof Kozlowski wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On 28/07/2023 11:08, Maso Huang wrote:
> > Add document for mt7986 board with wm8960.
> >
> > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > ---
> > .../sound/mediatek,mt7986-wm8960.yaml | 53
> +++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> >
> > diff --git
> a/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
> > new file mode 100644
> > index 000000000000..76394f7e5502
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> wm8960.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek MT7986 sound card with WM8960 codec
> > +
> > +maintainers:
> > + - Maso Huang <maso.huang@mediatek.com>
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,mt7986-wm8960-machine
> > +
> > + mediatek,platform:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: The phandle of MT7986 platform.
> > +
> > + audio-routing:
> > + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> > + description:
> > + A list of the connections between audio components. Each
> entry is a
> > + sink/source pair of strings. Valid names could be the input
> or output
> > + widgets of audio components, power supplies, MicBias of
> codec and the
> > + software switch.
> > +
> > + mediatek,audio-codec:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: The phandle of wm8960 codec.
> > +
>
> How did you implement Rob's comment? Or did you just ignore it?
>
> Best regards,
> Krzysztof
>
Hi Krzysztof,
Sorry, I did not mean to ignore Rob's comment.
I waited for some suggestion in mail below, but it seems Rob was a
little busy.
https://lore.kernel.org/lkml/8c6316e79e40406e4d46709f602dcb14a4c00562.camel@mediatek.com/
After gentle ping last week and receiving your advice, I thought that
means to send the v3 patch and might discuss dtbingding in v3 series.
So sorry for misunderstanding it, I'll check the details with Rob in v3
series then refine it in v4.
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-31 7:31 ` Maso Huang (黃加竹)
@ 2023-07-31 8:14 ` Krzysztof Kozlowski
2023-07-31 14:56 ` Krzysztof Kozlowski
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-31 8:14 UTC (permalink / raw)
To: Maso Huang (黃加竹),
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org, broonie@kernel.org,
Allen-KH Cheng (程冠勲), conor+dt@kernel.org,
tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On 31/07/2023 09:31, Maso Huang (黃加竹) wrote:
> On Fri, 2023-07-28 at 14:49 +0200, Krzysztof Kozlowski wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> On 28/07/2023 11:08, Maso Huang wrote:
>>> Add document for mt7986 board with wm8960.
>>>
>>> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
>>> ---
>>> .../sound/mediatek,mt7986-wm8960.yaml | 53
>> +++++++++++++++++++
>>> 1 file changed, 53 insertions(+)
>>> create mode 100644
>> Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
>> b/Documentation/devicetree/bindings/sound/mediatek,mt7986-wm8960.yaml
>>> new file mode 100644
>>> index 000000000000..76394f7e5502
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
>> wm8960.yaml
>>> @@ -0,0 +1,53 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id:
>> http://devicetree.org/schemas/sound/mediatek,mt7986-wm8960.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek MT7986 sound card with WM8960 codec
>>> +
>>> +maintainers:
>>> + - Maso Huang <maso.huang@mediatek.com>
>>> +
>>> +properties:
>>> + compatible:
>>> + const: mediatek,mt7986-wm8960-machine
>>> +
>>> + mediatek,platform:
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> + description: The phandle of MT7986 platform.
>>> +
>>> + audio-routing:
>>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>>> + description:
>>> + A list of the connections between audio components. Each
>> entry is a
>>> + sink/source pair of strings. Valid names could be the input
>> or output
>>> + widgets of audio components, power supplies, MicBias of
>> codec and the
>>> + software switch.
>>> +
>>> + mediatek,audio-codec:
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> + description: The phandle of wm8960 codec.
>>> +
>>
>> How did you implement Rob's comment? Or did you just ignore it?
>>
>> Best regards,
>> Krzysztof
>>
>
> Hi Krzysztof,
>
> Sorry, I did not mean to ignore Rob's comment.
> I waited for some suggestion in mail below, but it seems Rob was a
> little busy.
>
> https://lore.kernel.org/lkml/8c6316e79e40406e4d46709f602dcb14a4c00562.camel@mediatek.com/
>
> After gentle ping last week and receiving your advice, I thought that
> means to send the v3 patch and might discuss dtbingding in v3 series.
>
> So sorry for misunderstanding it, I'll check the details with Rob in v3
> series then refine it in v4.
The problem is that you did not reference in this patch any ongoing
discussion and further questions, so comment looks like addressed, while
it was not.
Rob said:
"in a common schema and reference them "
You said:
"common part yaml and reference to it"
so I think you both agreed on the same.
The advice would be to create common binding which is then referenced by
other and your bindings. However if you start doing it, you will notice
that it is impossible, because you have conflicting types for
"audio-codec", so you cannot have one definition.
This leads to the point - property is probably wrong and you need
dai-link with sound-dai property, just like most cards are doing.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-31 8:14 ` Krzysztof Kozlowski
@ 2023-07-31 14:56 ` Krzysztof Kozlowski
2023-08-04 7:08 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-31 14:56 UTC (permalink / raw)
To: Maso Huang (黃加竹),
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org, broonie@kernel.org,
Allen-KH Cheng (程冠勲), conor+dt@kernel.org,
tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On 31/07/2023 10:14, Krzysztof Kozlowski wrote:
>>>> + mediatek,audio-codec:
>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>> + description: The phandle of wm8960 codec.
>>>> +
>>>
>>> How did you implement Rob's comment? Or did you just ignore it?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> Hi Krzysztof,
>>
>> Sorry, I did not mean to ignore Rob's comment.
>> I waited for some suggestion in mail below, but it seems Rob was a
>> little busy.
>>
>> https://lore.kernel.org/lkml/8c6316e79e40406e4d46709f602dcb14a4c00562.camel@mediatek.com/
>>
>> After gentle ping last week and receiving your advice, I thought that
>> means to send the v3 patch and might discuss dtbingding in v3 series.
>>
>> So sorry for misunderstanding it, I'll check the details with Rob in v3
>> series then refine it in v4.
>
> The problem is that you did not reference in this patch any ongoing
> discussion and further questions, so comment looks like addressed, while
> it was not.
>
> Rob said:
> "in a common schema and reference them "
> You said:
> "common part yaml and reference to it"
> so I think you both agreed on the same.
>
> The advice would be to create common binding which is then referenced by
> other and your bindings. However if you start doing it, you will notice
> that it is impossible, because you have conflicting types for
> "audio-codec", so you cannot have one definition.
>
> This leads to the point - property is probably wrong and you need
> dai-link with sound-dai property, just like most cards are doing.
BTW, might be useful for you, just sent:
https://lore.kernel.org/linux-devicetree/20230731094303.185067-1-krzysztof.kozlowski@linaro.org/T/#t
Anyway you need dai-links, I think.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document
2023-07-31 14:56 ` Krzysztof Kozlowski
@ 2023-08-04 7:08 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-08-04 7:08 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
robh+dt@kernel.org, chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
krzysztof.kozlowski@linaro.org, broonie@kernel.org,
Allen-KH Cheng (程冠勲), conor+dt@kernel.org,
tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Mon, 2023-07-31 at 16:56 +0200, Krzysztof Kozlowski wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On 31/07/2023 10:14, Krzysztof Kozlowski wrote:
> >>>> + mediatek,audio-codec:
> >>>> + $ref: /schemas/types.yaml#/definitions/phandle
> >>>> + description: The phandle of wm8960 codec.
> >>>> +
> >>>
> >>> How did you implement Rob's comment? Or did you just ignore it?
> >>>
> >>> Best regards,
> >>> Krzysztof
> >>>
> >>
> >> Hi Krzysztof,
> >>
> >> Sorry, I did not mean to ignore Rob's comment.
> >> I waited for some suggestion in mail below, but it seems Rob was a
> >> little busy.
> >>
> >>
> https://lore.kernel.org/lkml/8c6316e79e40406e4d46709f602dcb14a4c00562.camel@mediatek.com/
> >>
> >> After gentle ping last week and receiving your advice, I thought
> that
> >> means to send the v3 patch and might discuss dtbingding in v3
> series.
> >>
> >> So sorry for misunderstanding it, I'll check the details with Rob
> in v3
> >> series then refine it in v4.
> >
> > The problem is that you did not reference in this patch any ongoing
> > discussion and further questions, so comment looks like addressed,
> while
> > it was not.
> >
> > Rob said:
> > "in a common schema and reference them "
> > You said:
> > "common part yaml and reference to it"
> > so I think you both agreed on the same.
> >
> > The advice would be to create common binding which is then
> referenced by
> > other and your bindings. However if you start doing it, you will
> notice
> > that it is impossible, because you have conflicting types for
> > "audio-codec", so you cannot have one definition.
> >
> > This leads to the point - property is probably wrong and you need
> > dai-link with sound-dai property, just like most cards are doing.
>
> BTW, might be useful for you, just sent:
>
https://lore.kernel.org/linux-devicetree/20230731094303.185067-1-krzysztof.kozlowski@linaro.org/T/#t
>
> Anyway you need dai-links, I think.
>
> Best regards,
> Krzysztof
>
Hi Krzysztof,
Thanks for your useful info!
I'll reference to sound-card-common.yaml for audio-routing.
For mediatek,platform and mediatek,audio-codec, I would like to replace
them with cpu and codec as properties directly like below.
cpu {
sound-dai <&ate>;
};
codec {
sound-dai <&wm8960>;
};
Is that OK?
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-07-28 9:08 [PATCH v3 0/6] ASoC: mediatek: Add support for MT7986 SoC Maso Huang
` (4 preceding siblings ...)
2023-07-28 9:08 ` [PATCH v3 5/6] ASoC: dt-bindings: mediatek,mt7986-wm8960: add mt7986-wm8960 document Maso Huang
@ 2023-07-28 9:08 ` Maso Huang
2023-07-28 10:00 ` AngeloGioacchino Del Regno
2023-07-28 12:51 ` Krzysztof Kozlowski
5 siblings, 2 replies; 33+ messages in thread
From: Maso Huang @ 2023-07-28 9:08 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: Maso Huang
Add mt7986 audio afe document.
Signed-off-by: Maso Huang <maso.huang@mediatek.com>
---
.../bindings/sound/mediatek,mt7986-afe.yaml | 89 +++++++++++++++++++
1 file changed, 89 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
new file mode 100644
index 000000000000..ebb151c6400f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mediatek,mt7986-afe.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek AFE PCM controller for MT7986
+
+maintainers:
+ - Maso Huang <maso.huang@mediatek.com>
+
+properties:
+ compatible:
+ oneOf:
+ - const: mediatek,mt7986-afe
+ - items:
+ - enum:
+ - mediatek,mt7981-afe
+ - mediatek,mt7988-afe
+ - const: mediatek,mt7986-afe
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 5
+ items:
+ - description: audio bus clock
+ - description: audio 26M clock
+ - description: audio intbus clock
+ - description: audio hopping clock
+ - description: audio pll clock
+ - description: mux for pcm_mck
+ - description: audio i2s/pcm mck
+
+ clock-names:
+ minItems: 5
+ items:
+ - const: bus_ck
+ - const: 26m_ck
+ - const: l_ck
+ - const: aud_ck
+ - const: eg2_ck
+ - const: sel
+ - const: i2s_m
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - assigned-clocks
+ - assigned-clock-parents
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/clock/mt7986-clk.h>
+
+ afe@11210000 {
+ compatible = "mediatek,mt7986-afe";
+ reg = <0x11210000 0x9000>;
+ interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&infracfg_ao CLK_INFRA_AUD_BUS_CK>,
+ <&infracfg_ao CLK_INFRA_AUD_26M_CK>,
+ <&infracfg_ao CLK_INFRA_AUD_L_CK>,
+ <&infracfg_ao CLK_INFRA_AUD_AUD_CK>,
+ <&infracfg_ao CLK_INFRA_AUD_EG2_CK>;
+ clock-names = "bus_ck",
+ "26m_ck",
+ "l_ck",
+ "aud_ck",
+ "eg2_ck";
+ assigned-clocks = <&topckgen CLK_TOP_A1SYS_SEL>,
+ <&topckgen CLK_TOP_AUD_L_SEL>,
+ <&topckgen CLK_TOP_A_TUNER_SEL>;
+ assigned-clock-parents = <&topckgen CLK_TOP_APLL2_D4>,
+ <&apmixedsys CLK_APMIXED_APLL2>,
+ <&topckgen CLK_TOP_APLL2_D4>;
+ };
+
+...
--
2.18.0
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-07-28 9:08 ` [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document Maso Huang
@ 2023-07-28 10:00 ` AngeloGioacchino Del Regno
2023-07-28 10:19 ` Maso Huang (黃加竹)
2023-07-28 12:51 ` Krzysztof Kozlowski
1 sibling, 1 reply; 33+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-28 10:00 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
Il 28/07/23 11:08, Maso Huang ha scritto:
> Add mt7986 audio afe document.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> ---
> .../bindings/sound/mediatek,mt7986-afe.yaml | 89 +++++++++++++++++++
> 1 file changed, 89 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
> new file mode 100644
> index 000000000000..ebb151c6400f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mediatek,mt7986-afe.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek AFE PCM controller for MT7986
> +
> +maintainers:
> + - Maso Huang <maso.huang@mediatek.com>
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: mediatek,mt7986-afe
> + - items:
> + - enum:
> + - mediatek,mt7981-afe
> + - mediatek,mt7988-afe
> + - const: mediatek,mt7986-afe
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + minItems: 5
> + items:
> + - description: audio bus clock
> + - description: audio 26M clock
> + - description: audio intbus clock
> + - description: audio hopping clock
> + - description: audio pll clock
> + - description: mux for pcm_mck
> + - description: audio i2s/pcm mck
> +
> + clock-names:
> + minItems: 5
> + items:
> + - const: bus_ck
> + - const: 26m_ck
> + - const: l_ck
> + - const: aud_ck
> + - const: eg2_ck
> + - const: sel
> + - const: i2s_m
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - assigned-clocks
> + - assigned-clock-parents
assigned-clocks and assigned-clock-parents are not a *required* property,
as that depends on a number of things and someone *may* want to omit it.
Besides, that's related to how the drivers interact with / setup the hardware
and not with the hardware itself... you can keep the assigned-clocks and
assigned-clock-parents in your examples, but again they're definitely not
required properties.
After fixing,
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Regards,
Angelo
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/clock/mt7986-clk.h>
> +
> + afe@11210000 {
> + compatible = "mediatek,mt7986-afe";
> + reg = <0x11210000 0x9000>;
> + interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&infracfg_ao CLK_INFRA_AUD_BUS_CK>,
> + <&infracfg_ao CLK_INFRA_AUD_26M_CK>,
> + <&infracfg_ao CLK_INFRA_AUD_L_CK>,
> + <&infracfg_ao CLK_INFRA_AUD_AUD_CK>,
> + <&infracfg_ao CLK_INFRA_AUD_EG2_CK>;
> + clock-names = "bus_ck",
> + "26m_ck",
> + "l_ck",
> + "aud_ck",
> + "eg2_ck";
> + assigned-clocks = <&topckgen CLK_TOP_A1SYS_SEL>,
> + <&topckgen CLK_TOP_AUD_L_SEL>,
> + <&topckgen CLK_TOP_A_TUNER_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_APLL2_D4>,
> + <&apmixedsys CLK_APMIXED_APLL2>,
> + <&topckgen CLK_TOP_APLL2_D4>;
> + };
> +
> +...
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-07-28 10:00 ` AngeloGioacchino Del Regno
@ 2023-07-28 10:19 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-07-28 10:19 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Allen-KH Cheng (程冠勲), broonie@kernel.org,
conor+dt@kernel.org, tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-07-28 at 12:00 +0200, AngeloGioacchino Del Regno wrote:
> Il 28/07/23 11:08, Maso Huang ha scritto:
> > Add mt7986 audio afe document.
> >
> > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> > ---
> > .../bindings/sound/mediatek,mt7986-afe.yaml | 89
> > +++++++++++++++++++
> > 1 file changed, 89 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
> > b/Documentation/devicetree/bindings/sound/mediatek,mt7986-afe.yaml
> > new file mode 100644
> > index 000000000000..ebb151c6400f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt7986-
> > afe.yaml
> > @@ -0,0 +1,89 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt7986-afe.yaml*__;Iw!!CTRNKA9wMg0ARbw!nwtJIMuoaT5RiUlFU-0ExLhvWHgz9nyXm0Jxk9RfoaKqIKzS8_JTvdIW9AhOkTIpVjN9Jv3L7Aj4nXzTEeuHK2-Wdq69mo0$
> >
> > +$schema:
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!nwtJIMuoaT5RiUlFU-0ExLhvWHgz9nyXm0Jxk9RfoaKqIKzS8_JTvdIW9AhOkTIpVjN9Jv3L7Aj4nXzTEeuHK2-Wo1ebvx8$
> >
> > +
> > +title: MediaTek AFE PCM controller for MT7986
> > +
> > +maintainers:
> > + - Maso Huang <maso.huang@mediatek.com>
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - const: mediatek,mt7986-afe
> > + - items:
> > + - enum:
> > + - mediatek,mt7981-afe
> > + - mediatek,mt7988-afe
> > + - const: mediatek,mt7986-afe
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 5
> > + items:
> > + - description: audio bus clock
> > + - description: audio 26M clock
> > + - description: audio intbus clock
> > + - description: audio hopping clock
> > + - description: audio pll clock
> > + - description: mux for pcm_mck
> > + - description: audio i2s/pcm mck
> > +
> > + clock-names:
> > + minItems: 5
> > + items:
> > + - const: bus_ck
> > + - const: 26m_ck
> > + - const: l_ck
> > + - const: aud_ck
> > + - const: eg2_ck
> > + - const: sel
> > + - const: i2s_m
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - assigned-clocks
> > + - assigned-clock-parents
>
> assigned-clocks and assigned-clock-parents are not a *required*
> property,
> as that depends on a number of things and someone *may* want to omit
> it.
>
> Besides, that's related to how the drivers interact with / setup the
> hardware
> and not with the hardware itself... you can keep the assigned-clocks
> and
> assigned-clock-parents in your examples, but again they're definitely
> not
> required properties.
>
> After fixing,
>
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
>
> Regards,
> Angelo
>
Hi Angelo,
Thanks for your review.
I'll remove these in v4 patch!
Best regards,
Maso
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/clock/mt7986-clk.h>
> > +
> > + afe@11210000 {
> > + compatible = "mediatek,mt7986-afe";
> > + reg = <0x11210000 0x9000>;
> > + interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&infracfg_ao CLK_INFRA_AUD_BUS_CK>,
> > + <&infracfg_ao CLK_INFRA_AUD_26M_CK>,
> > + <&infracfg_ao CLK_INFRA_AUD_L_CK>,
> > + <&infracfg_ao CLK_INFRA_AUD_AUD_CK>,
> > + <&infracfg_ao CLK_INFRA_AUD_EG2_CK>;
> > + clock-names = "bus_ck",
> > + "26m_ck",
> > + "l_ck",
> > + "aud_ck",
> > + "eg2_ck";
> > + assigned-clocks = <&topckgen CLK_TOP_A1SYS_SEL>,
> > + <&topckgen CLK_TOP_AUD_L_SEL>,
> > + <&topckgen CLK_TOP_A_TUNER_SEL>;
> > + assigned-clock-parents = <&topckgen CLK_TOP_APLL2_D4>,
> > + <&apmixedsys CLK_APMIXED_APLL2>,
> > + <&topckgen CLK_TOP_APLL2_D4>;
> > + };
> > +
> > +...
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-07-28 9:08 ` [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document Maso Huang
2023-07-28 10:00 ` AngeloGioacchino Del Regno
@ 2023-07-28 12:51 ` Krzysztof Kozlowski
2023-07-28 13:26 ` Mark Brown
2023-08-01 8:25 ` Maso Huang (黃加竹)
1 sibling, 2 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-28 12:51 UTC (permalink / raw)
To: Maso Huang, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Jaroslav Kysela, Takashi Iwai,
Trevor Wu, Arnd Bergmann, Mars Chen, Allen-KH Cheng, alsa-devel,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek
On 28/07/2023 11:08, Maso Huang wrote:
> Add mt7986 audio afe document.
>
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
Thank you for your patch. There is something to discuss/improve.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - assigned-clocks
> + - assigned-clock-parents
You should constrain your clocks per variants. I doubt that they are
really so flexible/optional on each SoC... or maybe missing clocks are
result of unimplemented parts in the driver? But then this should not
really affect bindings. Bindings still should require such clocks. Your
DTS can always provide a <0>, if needed.
> +
> +additionalProperties: false
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-07-28 12:51 ` Krzysztof Kozlowski
@ 2023-07-28 13:26 ` Mark Brown
2023-08-01 8:25 ` Maso Huang (黃加竹)
1 sibling, 0 replies; 33+ messages in thread
From: Mark Brown @ 2023-07-28 13:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Maso Huang, Liam Girdwood, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Trevor Wu, Arnd Bergmann,
Mars Chen, Allen-KH Cheng, alsa-devel, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek
[-- Attachment #1: Type: text/plain, Size: 880 bytes --]
On Fri, Jul 28, 2023 at 02:51:26PM +0200, Krzysztof Kozlowski wrote:
> On 28/07/2023 11:08, Maso Huang wrote:
> > + - assigned-clocks
> > + - assigned-clock-parents
> You should constrain your clocks per variants. I doubt that they are
> really so flexible/optional on each SoC... or maybe missing clocks are
> result of unimplemented parts in the driver? But then this should not
> really affect bindings. Bindings still should require such clocks. Your
> DTS can always provide a <0>, if needed.
Depending on what the clocks are some of them might genuinely be
optional, it's fairly common for audio devices to have multiple clock
inputs and be able to select between them depending on system
requirements or to have bidirectional clock pins which may be either a
provider or consumer depending on system configuration. No idea how
that applies with this specific device.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-07-28 12:51 ` Krzysztof Kozlowski
2023-07-28 13:26 ` Mark Brown
@ 2023-08-01 8:25 ` Maso Huang (黃加竹)
2023-08-04 9:45 ` Krzysztof Kozlowski
1 sibling, 1 reply; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-08-01 8:25 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org,
krzysztof.kozlowski@linaro.org, broonie@kernel.org,
Allen-KH Cheng (程冠勲), conor+dt@kernel.org,
tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-07-28 at 14:51 +0200, Krzysztof Kozlowski wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On 28/07/2023 11:08, Maso Huang wrote:
> > Add mt7986 audio afe document.
> >
> > Signed-off-by: Maso Huang <maso.huang@mediatek.com>
>
> Thank you for your patch. There is something to discuss/improve.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - assigned-clocks
> > + - assigned-clock-parents
>
> You should constrain your clocks per variants. I doubt that they are
> really so flexible/optional on each SoC... or maybe missing clocks
> are
> result of unimplemented parts in the driver? But then this should not
> really affect bindings. Bindings still should require such clocks.
> Your
> DTS can always provide a <0>, if needed.
>
>
Hi Krzysztof,
After internal check, assigned-clocks and assigned-clock-parents are
not required on this SoC.
Maybe we can just drop these two options?
Best regards,
Maso
> > +
> > +additionalProperties: false
>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-08-01 8:25 ` Maso Huang (黃加竹)
@ 2023-08-04 9:45 ` Krzysztof Kozlowski
2023-08-04 10:29 ` Maso Huang (黃加竹)
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-04 9:45 UTC (permalink / raw)
To: Maso Huang (黃加竹),
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
linux-mediatek@lists.infradead.org, broonie@kernel.org,
Allen-KH Cheng (程冠勲), conor+dt@kernel.org,
tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On 01/08/2023 10:25, Maso Huang (黃加竹) wrote:
> On Fri, 2023-07-28 at 14:51 +0200, Krzysztof Kozlowski wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> On 28/07/2023 11:08, Maso Huang wrote:
>>> Add mt7986 audio afe document.
>>>
>>> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - interrupts
>>> + - clocks
>>> + - clock-names
>>> + - assigned-clocks
>>> + - assigned-clock-parents
>>
>> You should constrain your clocks per variants. I doubt that they are
>> really so flexible/optional on each SoC... or maybe missing clocks
>> are
>> result of unimplemented parts in the driver? But then this should not
>> really affect bindings. Bindings still should require such clocks.
>> Your
>> DTS can always provide a <0>, if needed.
>>
>>
>
> Hi Krzysztof,
>
> After internal check, assigned-clocks and assigned-clock-parents are
> not required on this SoC.
> Maybe we can just drop these two options?
It's separate issue, but yes - why requiring them?
I wrote about missing constraints for your clocks in the bindings.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 6/6] ASoC: dt-bindings: mediatek,mt7986-afe: add audio afe document
2023-08-04 9:45 ` Krzysztof Kozlowski
@ 2023-08-04 10:29 ` Maso Huang (黃加竹)
0 siblings, 0 replies; 33+ messages in thread
From: Maso Huang (黃加竹) @ 2023-08-04 10:29 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
robh+dt@kernel.org, chenxiangrui@huaqin.corp-partner.google.com,
Trevor Wu (吳文良), devicetree@vger.kernel.org,
krzysztof.kozlowski@linaro.org, broonie@kernel.org,
Allen-KH Cheng (程冠勲), conor+dt@kernel.org,
tiwai@suse.com, lgirdwood@gmail.com,
linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
perex@perex.cz, arnd@arndb.de,
angelogioacchino.delregno@collabora.com,
alsa-devel@alsa-project.org
On Fri, 2023-08-04 at 11:45 +0200, Krzysztof Kozlowski wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On 01/08/2023 10:25, Maso Huang (黃加竹) wrote:
> > On Fri, 2023-07-28 at 14:51 +0200, Krzysztof Kozlowski wrote:
> >>
> >> External email : Please do not click links or open attachments
> until
> >> you have verified the sender or the content.
> >> On 28/07/2023 11:08, Maso Huang wrote:
> >>> Add mt7986 audio afe document.
> >>>
> >>> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> >>
> >> Thank you for your patch. There is something to discuss/improve.
> >>
> >>> +
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - interrupts
> >>> + - clocks
> >>> + - clock-names
> >>> + - assigned-clocks
> >>> + - assigned-clock-parents
> >>
> >> You should constrain your clocks per variants. I doubt that they
> are
> >> really so flexible/optional on each SoC... or maybe missing clocks
> >> are
> >> result of unimplemented parts in the driver? But then this should
> not
> >> really affect bindings. Bindings still should require such clocks.
> >> Your
> >> DTS can always provide a <0>, if needed.
> >>
> >>
> >
> > Hi Krzysztof,
> >
> > After internal check, assigned-clocks and assigned-clock-parents
> are
> > not required on this SoC.
> > Maybe we can just drop these two options?
>
> It's separate issue, but yes - why requiring them?
>
> I wrote about missing constraints for your clocks in the bindings.
>
> Best regards,
> Krzysztof
>
Hi Krzysztof,
OK, I'll remove assigned-clock and assigned-clock-parents.
And constrain the clocks for different SoC in the binding in v4 patch.
Best regards,
Maso
^ permalink raw reply [flat|nested] 33+ messages in thread