| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGXv+5EZ99i74_pTp2wKR1ni28K9fwbqo_67CFXwwiN13DB71w@mail.gmail.com>
Date: Wed, 30 Jul 2025 16:42:48 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: "Darren.Ye" <darren.ye@...iatek.com>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>, Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>, linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v6 03/10] ASoC: mediatek: mt8196: support audio clock control
On Tue, Jul 8, 2025 at 7:34 PM Darren.Ye <darren.ye@...iatek.com> wrote:
>
> From: Darren Ye <darren.ye@...iatek.com>
>
> Add audio clock wrapper and audio tuner control.
>
> Signed-off-by: Darren Ye <darren.ye@...iatek.com>
> ---
> sound/soc/mediatek/mt8196/mt8196-afe-clk.c | 728 +++++++++++++++++++++
> sound/soc/mediatek/mt8196/mt8196-afe-clk.h | 80 +++
> 2 files changed, 808 insertions(+)
> create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-clk.c
> create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-clk.h
>
> diff --git a/sound/soc/mediatek/mt8196/mt8196-afe-clk.c b/sound/soc/mediatek/mt8196/mt8196-afe-clk.c
> new file mode 100644
> index 000000000000..00f47b485812
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8196/mt8196-afe-clk.c
> @@ -0,0 +1,728 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mt8196-afe-clk.c -- Mediatek 8196 afe clock ctrl
> + *
> + * Copyright (c) 2024 MediaTek Inc.
> + * Author: Darren Ye <darren.ye@...iatek.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
Please add an empty line here for separation.
> +#include "mt8196-afe-common.h"
> +#include "mt8196-afe-clk.h"
> +
> +static const char *aud_clks[MT8196_CLK_NUM] = {
> + /* vlp clk */
> + [MT8196_CLK_VLP_MUX_AUDIOINTBUS] = "top_aud_intbus",
> + [MT8196_CLK_VLP_MUX_AUD_ENG1] = "top_aud_eng1",
> + [MT8196_CLK_VLP_MUX_AUD_ENG2] = "top_aud_eng2",
> + [MT8196_CLK_VLP_MUX_AUDIO_H] = "top_aud_h",
> + [MT8196_CLK_VLP_CLK26M] = "vlp_clk26m",
> + /* pll */
> + [MT8196_CLK_TOP_APLL1_CK] = "apll1",
> + [MT8196_CLK_TOP_APLL2_CK] = "apll2",
> + /* divider */
> + [MT8196_CLK_TOP_APLL1_D4] = "apll1_d4",
> + [MT8196_CLK_TOP_APLL2_D4] = "apll2_d4",
> + [MT8196_CLK_TOP_APLL12_DIV_I2SIN0] = "apll12_div_i2sin0",
> + [MT8196_CLK_TOP_APLL12_DIV_I2SIN1] = "apll12_div_i2sin1",
> + [MT8196_CLK_TOP_APLL12_DIV_FMI2S] = "apll12_div_fmi2s",
> + [MT8196_CLK_TOP_APLL12_DIV_TDMOUT_M] = "apll12_div_tdmout_m",
> + [MT8196_CLK_TOP_APLL12_DIV_TDMOUT_B] = "apll12_div_tdmout_b",
> + /* mux */
> + [MT8196_CLK_TOP_MUX_AUD_1] = "top_apll1",
> + [MT8196_CLK_TOP_MUX_AUD_2] = "top_apll2",
> + [MT8196_CLK_TOP_I2SIN0_M_SEL] = "top_i2sin0",
> + [MT8196_CLK_TOP_I2SIN1_M_SEL] = "top_i2sin1",
> + [MT8196_CLK_TOP_FMI2S_M_SEL] = "top_fmi2s",
> + [MT8196_CLK_TOP_TDMOUT_M_SEL] = "top_dptx",
> + [MT8196_CLK_TOP_ADSP_SEL] = "top_adsp",
> + /* top 26m*/
> + [MT8196_CLK_TOP_CLK26M] = "clk26m",
> +};
> +
> +int mt8196_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk)
> +{
> + int ret;
> +
> + if (clk) {
There's no need to check the validity of the pointer. The clk prepare
and enable APIs can take NULL pointers and become no-ops.
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_dbg(afe->dev, "failed to enable clk\n");
This should be a visible error.
> + return ret;
> + }
> + } else {
> + dev_dbg(afe->dev, "NULL clk\n");
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mt8196_afe_enable_clk);
> +
> +void mt8196_afe_disable_clk(struct mtk_base_afe *afe, struct clk *clk)
> +{
> + if (clk)
> + clk_disable_unprepare(clk);
> + else
> + dev_dbg(afe->dev, "NULL clk\n");
> +}
> +EXPORT_SYMBOL_GPL(mt8196_afe_disable_clk);
> +
> +static int mt8196_afe_set_clk_rate(struct mtk_base_afe *afe, struct clk *clk,
> + unsigned int rate)
> +{
> + int ret;
> +
> + if (clk) {
> + ret = clk_set_rate(clk, rate);
> + if (ret) {
> + dev_dbg(afe->dev, "failed to set clk rate\n");
This should be a visible error.
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int mt8196_afe_set_clk_parent(struct mtk_base_afe *afe, struct clk *clk,
> + struct clk *parent)
> +{
> + int ret;
> +
> + if (clk && parent) {
> + ret = clk_set_parent(clk, parent);
> + if (ret) {
> + dev_dbg(afe->dev, "failed to set clk parent %d\n", ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
Per our offline discussions, explicitly setting clock parents are not
needed.
> +
> +static unsigned int get_top_cg_reg(unsigned int cg_type)
> +{
> + switch (cg_type) {
> + case MT8196_AUDIO_26M_EN_ON:
> + case MT8196_AUDIO_F3P25M_EN_ON:
> + case MT8196_AUDIO_APLL1_EN_ON:
> + case MT8196_AUDIO_APLL2_EN_ON:
> + return AUDIO_ENGEN_CON0;
> + case MT8196_CG_AUDIO_HOPPING_CK:
> + case MT8196_CG_AUDIO_F26M_CK:
> + case MT8196_CG_APLL1_CK:
> + case MT8196_CG_APLL2_CK:
> + case MT8196_PDN_APLL_TUNER2:
> + case MT8196_PDN_APLL_TUNER1:
> + return AUDIO_TOP_CON4;
> + default:
> + return 0;
> + }
> +}
> +
> +static unsigned int get_top_cg_mask(unsigned int cg_type)
> +{
> + switch (cg_type) {
> + case MT8196_AUDIO_26M_EN_ON:
> + return AUDIO_26M_EN_ON_MASK_SFT;
> + case MT8196_AUDIO_F3P25M_EN_ON:
> + return AUDIO_F3P25M_EN_ON_MASK_SFT;
> + case MT8196_AUDIO_APLL1_EN_ON:
> + return AUDIO_APLL1_EN_ON_MASK_SFT;
> + case MT8196_AUDIO_APLL2_EN_ON:
> + return AUDIO_APLL2_EN_ON_MASK_SFT;
> + case MT8196_CG_AUDIO_HOPPING_CK:
> + return CG_AUDIO_HOPPING_CK_MASK_SFT;
> + case MT8196_CG_AUDIO_F26M_CK:
> + return CG_AUDIO_F26M_CK_MASK_SFT;
> + case MT8196_CG_APLL1_CK:
> + return CG_APLL1_CK_MASK_SFT;
> + case MT8196_CG_APLL2_CK:
> + return CG_APLL2_CK_MASK_SFT;
> + case MT8196_PDN_APLL_TUNER2:
> + return PDN_APLL_TUNER2_MASK_SFT;
> + case MT8196_PDN_APLL_TUNER1:
> + return PDN_APLL_TUNER1_MASK_SFT;
> + default:
> + return 0;
> + }
> +}
> +
> +static unsigned int get_top_cg_on_val(unsigned int cg_type)
> +{
> + switch (cg_type) {
> + case MT8196_AUDIO_26M_EN_ON:
> + case MT8196_AUDIO_F3P25M_EN_ON:
> + case MT8196_AUDIO_APLL1_EN_ON:
> + case MT8196_AUDIO_APLL2_EN_ON:
> + return get_top_cg_mask(cg_type);
> + case MT8196_CG_AUDIO_HOPPING_CK:
> + case MT8196_CG_AUDIO_F26M_CK:
> + case MT8196_CG_APLL1_CK:
> + case MT8196_CG_APLL2_CK:
> + case MT8196_PDN_APLL_TUNER2:
> + case MT8196_PDN_APLL_TUNER1:
> + return 0;
> + default:
> + return 0;
> + }
> +}
> +
> +static unsigned int get_top_cg_off_val(unsigned int cg_type)
> +{
> + switch (cg_type) {
> + case MT8196_AUDIO_26M_EN_ON:
> + case MT8196_AUDIO_F3P25M_EN_ON:
> + case MT8196_AUDIO_APLL1_EN_ON:
> + case MT8196_AUDIO_APLL2_EN_ON:
> + return 0;
> + case MT8196_CG_AUDIO_HOPPING_CK:
> + case MT8196_CG_AUDIO_F26M_CK:
> + case MT8196_CG_APLL1_CK:
> + case MT8196_CG_APLL2_CK:
> + case MT8196_PDN_APLL_TUNER2:
> + case MT8196_PDN_APLL_TUNER1:
> + return get_top_cg_mask(cg_type);
> + default:
> + return get_top_cg_mask(cg_type);
> + }
> +}
> +
> +static int mt8196_afe_enable_top_cg(struct mtk_base_afe *afe, unsigned int cg_type)
> +{
> + unsigned int reg = get_top_cg_reg(cg_type);
> + unsigned int mask = get_top_cg_mask(cg_type);
> + unsigned int val = get_top_cg_on_val(cg_type);
> +
> + if (!afe->regmap) {
> + dev_warn(afe->dev, "skip regmap\n");
> + return 0;
This should be a fatal error.
> + }
> +
> + dev_dbg(afe->dev, "reg: 0x%x, mask: 0x%x, val: 0x%x\n", reg, mask, val);
> + regmap_update_bits(afe->regmap, reg, mask, val);
Should check the return value, since it could fail because these are set as
volatile registers and cannot be updated in cache-only state.
> + return 0;
> +}
> +
> +static int mt8196_afe_disable_top_cg(struct mtk_base_afe *afe, unsigned int cg_type)
> +{
> + unsigned int reg = get_top_cg_reg(cg_type);
> + unsigned int mask = get_top_cg_mask(cg_type);
> + unsigned int val = get_top_cg_off_val(cg_type);
> +
> + if (!afe->regmap) {
> + dev_warn(afe->dev, "skip regmap\n");
> + return 0;
> + }
> +
> + dev_dbg(afe->dev, "reg: 0x%x, mask: 0x%x, val: 0x%x\n", reg, mask, val);
> + regmap_update_bits(afe->regmap, reg, mask, val);
Same here.
> +
> + return 0;
> +}
> +
> +static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> + int ret = 0;
> +
> + dev_dbg(afe->dev, "enable: %d\n", enable);
> +
> + if (enable) {
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1],
> + afe_priv->clk[MT8196_CLK_TOP_APLL1_CK]);
> + if (ret)
> + return ret;
> +
> + /* 180.6336 / 4 = 45.1584MHz */
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG1]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG1],
> + afe_priv->clk[MT8196_CLK_TOP_APLL1_D4]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H],
> + afe_priv->clk[MT8196_CLK_TOP_APLL1_CK]);
> + if (ret)
> + return ret;
> + } else {
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG1],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
> + if (ret)
> + return ret;
> +
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG1]);
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1],
> + afe_priv->clk[MT8196_CLK_TOP_CLK26M]);
> + if (ret)
> + return ret;
> +
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1]);
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> + }
We've talked about this offline. FTR there's no need to enable intermediate
clocks. When the leaf clock is enabled, the CCF also enables all connected
parents. There's also no need to set parents explicitly. When the clock
rate of the leaf clock gets set, the CCF will take care to reparent it
or the sub-tree to the most appropriate clock parent.
> + return 0;
> +}
> +
> +static int apll2_mux_setting(struct mtk_base_afe *afe, bool enable)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> + int ret = 0;
> +
> + dev_dbg(afe->dev, "enable: %d\n", enable);
> +
> + if (enable) {
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2],
> + afe_priv->clk[MT8196_CLK_TOP_APLL2_CK]);
> + if (ret)
> + return ret;
> +
> + /* 196.608 / 4 = 49.152MHz */
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG2]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG2],
> + afe_priv->clk[MT8196_CLK_TOP_APLL2_D4]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H],
> + afe_priv->clk[MT8196_CLK_TOP_APLL2_CK]);
> + if (ret)
> + return ret;
> + } else {
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG2],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
> + if (ret)
> + return ret;
> +
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUD_ENG2]);
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2],
> + afe_priv->clk[MT8196_CLK_TOP_CLK26M]);
> + if (ret)
> + return ret;
> +
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2]);
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> + }
Same for this function.
> + return 0;
> +}
> +
> +static int mt8196_afe_disable_apll(struct mtk_base_afe *afe)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> + int ret = 0;
> +
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1]);
> + if (ret)
> + goto clk_ck_mux_aud1_err;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1],
> + afe_priv->clk[MT8196_CLK_TOP_CLK26M]);
> + if (ret)
> + goto clk_ck_mux_aud1_parent_err;
> +
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2]);
> + if (ret)
> + goto clk_ck_mux_aud2_err;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2],
> + afe_priv->clk[MT8196_CLK_TOP_CLK26M]);
> + if (ret)
> + goto clk_ck_mux_aud2_parent_err;
> +
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1]);
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2]);
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
Same here. There's no need to toggle all the intermediate clocks. And since
everything is getting disabled, what parent is selected shouldn't matter.
> + return 0;
> +
> +clk_ck_mux_aud2_parent_err:
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_2]);
> +clk_ck_mux_aud2_err:
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1],
> + afe_priv->clk[MT8196_CLK_TOP_APLL1_CK]);
> +clk_ck_mux_aud1_parent_err:
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_MUX_AUD_1]);
> +clk_ck_mux_aud1_err:
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> +
> + return ret;
> +}
> +
> +static void mt8196_afe_apll_init(struct mtk_base_afe *afe)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> +
> + if (!afe_priv->vlp_ck) {
> + dev_warn(afe->dev, "vlp_ck regmap is null ptr\n");
> + return;
> + }
> +
> + regmap_write(afe_priv->vlp_ck, VLP_APLL1_TUNER_CON0, VLP_APLL1_TUNER_CON0_VALUE);
> + regmap_write(afe_priv->vlp_ck, VLP_APLL2_TUNER_CON0, VLP_APLL2_TUNER_CON0_VALUE);
Per offline discussion, this should be moved to the vlp clk driver.
This was already mentioned to the clk patch owners in a recent review.
> +}
[...]
> +/* mck */
> +struct mt8196_mck_div {
> + int m_sel_id;
> + int div_clk_id;
> +};
> +
> +static const struct mt8196_mck_div mck_div[MT8196_MCK_NUM] = {
> + [MT8196_I2SIN0_MCK] = {
> + .m_sel_id = MT8196_CLK_TOP_I2SIN0_M_SEL,
> + .div_clk_id = MT8196_CLK_TOP_APLL12_DIV_I2SIN0,
> + },
> + [MT8196_I2SIN1_MCK] = {
> + .m_sel_id = MT8196_CLK_TOP_I2SIN1_M_SEL,
> + .div_clk_id = MT8196_CLK_TOP_APLL12_DIV_I2SIN1,
> + },
> + [MT8196_FMI2S_MCK] = {
> + .m_sel_id = MT8196_CLK_TOP_FMI2S_M_SEL,
> + .div_clk_id = MT8196_CLK_TOP_APLL12_DIV_FMI2S,
> + },
> + [MT8196_TDMOUT_MCK] = {
> + .m_sel_id = MT8196_CLK_TOP_TDMOUT_M_SEL,
> + .div_clk_id = MT8196_CLK_TOP_APLL12_DIV_TDMOUT_M,
> + },
> + [MT8196_TDMOUT_BCK] = {
> + .m_sel_id = -1,
> + .div_clk_id = MT8196_CLK_TOP_APLL12_DIV_TDMOUT_B,
> + },
> +};
In the upstream clk patch submission, the mux and divider have been
combined. So this part could be simplified a bit. Also...
> +int mt8196_mck_enable(struct mtk_base_afe *afe, int mck_id, int rate)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> + int apll = mt8196_get_apll_by_rate(afe, rate);
> + int apll_clk_id = apll == MT8196_APLL1 ?
> + MT8196_CLK_TOP_MUX_AUD_1 : MT8196_CLK_TOP_MUX_AUD_2;
> + int m_sel_id;
> + int div_clk_id;
> + int ret;
> +
> + dev_dbg(afe->dev, "mck_id: %d, rate: %d\n", mck_id, rate);
> +
> + if (mck_id >= MT8196_MCK_NUM || mck_id < 0)
> + return -EINVAL;
> +
> + m_sel_id = mck_div[mck_id].m_sel_id;
> + div_clk_id = mck_div[mck_id].div_clk_id;
> +
> + /* select apll */
> + if (m_sel_id >= 0) {
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[m_sel_id]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_parent(afe, afe_priv->clk[m_sel_id],
> + afe_priv->clk[apll_clk_id]);
This part would be taken care of by the framework as well. There's no
need to do it explicitly.
> + if (ret)
> + return ret;
> + }
> +
> + /* enable div, set rate */
> + if (div_clk_id < 0) {
> + dev_err(afe->dev, "invalid div_clk_id %d\n", div_clk_id);
> + return -EINVAL;
> + }
> + if (div_clk_id == MT8196_CLK_TOP_APLL12_DIV_TDMOUT_B)
> + rate = rate * 16;
rate *= 16;
> +
> + ret = mt8196_afe_enable_clk(afe, afe_priv->clk[div_clk_id]);
> + if (ret)
> + return ret;
> +
> + ret = mt8196_afe_set_clk_rate(afe, afe_priv->clk[div_clk_id], rate);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +int mt8196_mck_disable(struct mtk_base_afe *afe, int mck_id)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> + int m_sel_id;
> + int div_clk_id;
> +
> + dev_dbg(afe->dev, "mck_id: %d.\n", mck_id);
> +
> + if (mck_id < 0) {
> + dev_err(afe->dev, "mck_id = %d < 0\n", mck_id);
> + return -EINVAL;
> + }
> +
> + m_sel_id = mck_div[mck_id].m_sel_id;
> + div_clk_id = mck_div[mck_id].div_clk_id;
> +
> + if (div_clk_id < 0) {
> + dev_err(afe->dev, "div_clk_id = %d < 0\n",
> + div_clk_id);
> + return -EINVAL;
> + }
> +
> + mt8196_afe_disable_clk(afe, afe_priv->clk[div_clk_id]);
> +
> + if (m_sel_id >= 0)
> + mt8196_afe_disable_clk(afe, afe_priv->clk[m_sel_id]);
> +
> + return 0;
> +}
> +
> +int mt8196_afe_enable_reg_rw_clk(struct mtk_base_afe *afe)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> +
> + /* bus clock for AFE external access, like DRAM */
> + mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_ADSP_SEL]);
> +
> + /* bus clock for AFE internal access, like AFE SRAM */
> + mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIOINTBUS]);
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIOINTBUS],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
If you are setting it to 26M, then it probably doesn't matter what parent
it uses? I would just drop this.
> + /* enable audio vlp clock source */
> + mt8196_afe_enable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
Same here.
> +
> + /* AFE hw clock */
> + /* IPM2.0: USE HOPPING & 26M */
> + /* set in the regmap_register_patch */
> + return 0;
> +}
> +
> +int mt8196_afe_disable_reg_rw_clk(struct mtk_base_afe *afe)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> +
> + /* IPM2.0: Use HOPPING & 26M */
> + /* set in the regmap_register_patch */
> +
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
There's no point in selecting a parent on a clock that is going to be disabled.
> +
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIO_H]);
> + mt8196_afe_set_clk_parent(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIOINTBUS],
> + afe_priv->clk[MT8196_CLK_VLP_CLK26M]);
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_VLP_MUX_AUDIOINTBUS]);
> + mt8196_afe_disable_clk(afe, afe_priv->clk[MT8196_CLK_TOP_ADSP_SEL]);
> + return 0;
> +}
> +
> +int mt8196_afe_enable_main_clock(struct mtk_base_afe *afe)
> +{
> + mt8196_afe_enable_top_cg(afe, MT8196_AUDIO_26M_EN_ON);
> + return 0;
> +}
> +
> +int mt8196_afe_disable_main_clock(struct mtk_base_afe *afe)
> +{
> + mt8196_afe_disable_top_cg(afe, MT8196_AUDIO_26M_EN_ON);
> + return 0;
> +}
> +
> +int mt8196_init_clock(struct mtk_base_afe *afe)
> +{
> + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> + int ret = 0;
> + int i = 0;
> +
> + afe_priv->clk = devm_kcalloc(afe->dev, MT8196_CLK_NUM, sizeof(*afe_priv->clk),
> + GFP_KERNEL);
> + if (!afe_priv->clk)
> + return -ENOMEM;
> +
> + for (i = 0; i < MT8196_CLK_NUM; i++) {
> + afe_priv->clk[i] = devm_clk_get(afe->dev, aud_clks[i]);
> + if (IS_ERR(afe_priv->clk[i])) {
> + dev_err(afe->dev, "devm_clk_get %s fail\n", aud_clks[i]);
> + return PTR_ERR(afe_priv->clk[i]);
> + }
> + }
> +
> + afe_priv->vlp_ck = syscon_regmap_lookup_by_phandle(afe->dev->of_node,
> + "vlpcksys");
> + if (IS_ERR(afe_priv->vlp_ck)) {
> + dev_err(afe->dev, "Cannot find vlpcksys\n");
> + return PTR_ERR(afe_priv->vlp_ck);
> + }
As mentioned, the tuner bits will be moved to the clk driver, so this
bit is no longer needed.
> +
> + mt8196_afe_apll_init(afe);
> +
> + ret = mt8196_afe_disable_apll(afe);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> diff --git a/sound/soc/mediatek/mt8196/mt8196-afe-clk.h b/sound/soc/mediatek/mt8196/mt8196-afe-clk.h
> new file mode 100644
> index 000000000000..854da3844104
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8196/mt8196-afe-clk.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * mt8196-afe-clk.h -- Mediatek MT8196 AFE Clock Control definitions
> + *
> + * Copyright (c) 2024 MediaTek Inc.
> + * Author: Darren Ye <darren.ye@...iatek.com>
> + */
> +
> +#ifndef _MT8196_AFE_CLOCK_CTRL_H_
> +#define _MT8196_AFE_CLOCK_CTRL_H_
> +
> +/* vlp_cksys_clk: 0x1c016000 */
> +#define VLP_APLL1_TUNER_CON0 0x02a4
> +#define VLP_APLL2_TUNER_CON0 0x02a8
> +
> +/* vlp apll1 tuner default value*/
> +#define VLP_APLL1_TUNER_CON0_VALUE 0x6f28bd4d
> +/* vlp apll2 tuner default value + 1*/
> +#define VLP_APLL2_TUNER_CON0_VALUE 0x78fd5265
> +
> +/* APLL */
> +#define APLL1_W_NAME "APLL1"
> +#define APLL2_W_NAME "APLL2"
> +
> +enum {
> + MT8196_APLL1 = 0,
> + MT8196_APLL2,
> +};
> +
> +enum {
> + /* vlp clk */
> + MT8196_CLK_VLP_MUX_AUDIOINTBUS,
> + MT8196_CLK_VLP_MUX_AUD_ENG1,
> + MT8196_CLK_VLP_MUX_AUD_ENG2,
> + MT8196_CLK_VLP_MUX_AUDIO_H,
> + MT8196_CLK_VLP_CLK26M,
> + /* pll */
> + MT8196_CLK_TOP_APLL1_CK,
> + MT8196_CLK_TOP_APLL2_CK,
> + /* divider */
> + MT8196_CLK_TOP_APLL1_D4,
> + MT8196_CLK_TOP_APLL2_D4,
> + MT8196_CLK_TOP_APLL12_DIV_I2SIN0,
> + MT8196_CLK_TOP_APLL12_DIV_I2SIN1,
> + MT8196_CLK_TOP_APLL12_DIV_FMI2S,
> + MT8196_CLK_TOP_APLL12_DIV_TDMOUT_M,
> + MT8196_CLK_TOP_APLL12_DIV_TDMOUT_B,
> + /* mux */
> + MT8196_CLK_TOP_MUX_AUD_1,
> + MT8196_CLK_TOP_MUX_AUD_2,
> + MT8196_CLK_TOP_I2SIN0_M_SEL,
> + MT8196_CLK_TOP_I2SIN1_M_SEL,
> + MT8196_CLK_TOP_FMI2S_M_SEL,
> + MT8196_CLK_TOP_TDMOUT_M_SEL,
> + MT8196_CLK_TOP_ADSP_SEL,
> + /* top 26m */
> + MT8196_CLK_TOP_CLK26M,
> + MT8196_CLK_NUM,
The list should be reworked based on review comments to the DT bindings
and our offline discussions. Basically only clocks that directly feed
into the hardware, or otherwise have a reason to be referenced should
be listed.
ChenYu
> +};
> +
> +struct mtk_base_afe;
> +
> +int mt8196_mck_enable(struct mtk_base_afe *afe, int mck_id, int rate);
> +int mt8196_mck_disable(struct mtk_base_afe *afe, int mck_id);
> +int mt8196_get_apll_rate(struct mtk_base_afe *afe, int apll);
> +int mt8196_get_apll_by_rate(struct mtk_base_afe *afe, int rate);
> +int mt8196_get_apll_by_name(struct mtk_base_afe *afe, const char *name);
> +int mt8196_init_clock(struct mtk_base_afe *afe);
> +int mt8196_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk);
> +void mt8196_afe_disable_clk(struct mtk_base_afe *afe, struct clk *clk);
> +int mt8196_apll1_enable(struct mtk_base_afe *afe);
> +void mt8196_apll1_disable(struct mtk_base_afe *afe);
> +int mt8196_apll2_enable(struct mtk_base_afe *afe);
> +void mt8196_apll2_disable(struct mtk_base_afe *afe);
> +int mt8196_afe_enable_main_clock(struct mtk_base_afe *afe);
> +int mt8196_afe_disable_main_clock(struct mtk_base_afe *afe);
> +int mt8196_afe_enable_reg_rw_clk(struct mtk_base_afe *afe);
> +int mt8196_afe_disable_reg_rw_clk(struct mtk_base_afe *afe);
> +
> +#endif
> --
> 2.45.2
>
>
Powered by blists - more mailing lists