| 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: <448db9fa-22c4-4123-892c-8d1fe4e7d562@collabora.com>
Date: Mon, 20 May 2024 12:38:37 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Alexandre Mergnat <amergnat@...libre.com>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Matthias Brugger
<matthias.bgg@...il.com>, Lee Jones <lee@...nel.org>,
Flora Fu <flora.fu@...iatek.com>, Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>, Sumit Semwal <sumit.semwal@...aro.org>,
Christian König <christian.koenig@....com>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>
Cc: 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-media@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v4 05/16] SoC: mediatek: mt8365: support audio clock
control
Il 26/04/24 19:22, Alexandre Mergnat ha scritto:
> Add audio clock wrapper and audio tuner control.
>
> Signed-off-by: Alexandre Mergnat <amergnat@...libre.com>
> ---
> sound/soc/mediatek/mt8365/mt8365-afe-clk.c | 443 +++++++++++++++++++++++++++++
> sound/soc/mediatek/mt8365/mt8365-afe-clk.h | 49 ++++
> 2 files changed, 492 insertions(+)
>
> diff --git a/sound/soc/mediatek/mt8365/mt8365-afe-clk.c b/sound/soc/mediatek/mt8365/mt8365-afe-clk.c
> new file mode 100644
> index 000000000000..3a525dae857c
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8365/mt8365-afe-clk.c
> @@ -0,0 +1,443 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mediatek 8365 AFE clock control
> + *
> + * Copyright (c) 2024 MediaTek Inc.
> + * Authors: Jia Zeng <jia.zeng@...iatek.com>
> + * Alexandre Mergnat <amergnat@...libre.com>
> + */
> +
> +#include "mt8365-afe-clk.h"
> +#include "mt8365-afe-common.h"
> +#include "mt8365-reg.h"
> +#include "../common/mtk-base-afe.h"
> +#include <linux/device.h>
> +#include <linux/mfd/syscon.h>
> +
> +static const char *aud_clks[MT8365_CLK_NUM] = {
> + [MT8365_CLK_TOP_AUD_SEL] = "top_audio_sel",
> + [MT8365_CLK_AUD_I2S0_M] = "audio_i2s0_m",
> + [MT8365_CLK_AUD_I2S1_M] = "audio_i2s1_m",
> + [MT8365_CLK_AUD_I2S2_M] = "audio_i2s2_m",
> + [MT8365_CLK_AUD_I2S3_M] = "audio_i2s3_m",
> + [MT8365_CLK_ENGEN1] = "engen1",
> + [MT8365_CLK_ENGEN2] = "engen2",
> + [MT8365_CLK_AUD1] = "aud1",
> + [MT8365_CLK_AUD2] = "aud2",
> + [MT8365_CLK_I2S0_M_SEL] = "i2s0_m_sel",
> + [MT8365_CLK_I2S1_M_SEL] = "i2s1_m_sel",
> + [MT8365_CLK_I2S2_M_SEL] = "i2s2_m_sel",
> + [MT8365_CLK_I2S3_M_SEL] = "i2s3_m_sel",
> + [MT8365_CLK_CLK26M] = "top_clk26m_clk",
> +};
> +
> +int mt8365_afe_init_audio_clk(struct mtk_base_afe *afe)
> +{
> + size_t i;
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> +
> + for (i = 0; i < ARRAY_SIZE(aud_clks); i++) {
> + afe_priv->clocks[i] = devm_clk_get(afe->dev, aud_clks[i]);
> + if (IS_ERR(afe_priv->clocks[i])) {
> + dev_err(afe->dev, "%s devm_clk_get %s fail\n",
> + __func__, aud_clks[i]);
> + return PTR_ERR(afe_priv->clocks[i]);
> + }
> + }
> + return 0;
> +}
> +
> +int mt8365_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk)
Do you really really really need those helper functions?
I say that you can simply call the clk API from the users instead, as you are
really just doing that in those functions...
> +{
> + int ret;
> +
> + if (clk) {
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(afe->dev, "Failed to enable clk\n");
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
> +void mt8365_afe_disable_clk(struct mtk_base_afe *afe, struct clk *clk)
> +{
> + if (clk)
> + clk_disable_unprepare(clk);
> +}
> +
> +int mt8365_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_err(afe->dev, "Failed to set rate\n");
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
> +int mt8365_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_err(afe->dev, "Failed to set parent\n");
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
.snip..
> +int mt8365_afe_enable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll)
> +{
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> +
> + mutex_lock(&afe_priv->afe_clk_mutex);
> +
> + afe_priv->apll_tuner_ref_cnt[apll]++;
> + if (afe_priv->apll_tuner_ref_cnt[apll] != 1) {
> + mutex_unlock(&afe_priv->afe_clk_mutex);
> + return 0;
> + }
> +
> + if (apll == MT8365_AFE_APLL1) {
> + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG,
> + AFE_APLL_TUNER_CFG_MASK, 0x432);
> + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG,
> + AFE_APLL_TUNER_CFG_EN_MASK, 0x1);
> + } else {
> + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG1,
> + AFE_APLL_TUNER_CFG1_MASK, 0x434);
> + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG1,
> + AFE_APLL_TUNER_CFG1_EN_MASK, 0x1);
> + }
> +
> + mutex_unlock(&afe_priv->afe_clk_mutex);
> + return 0;
> +}
> +
> +int mt8365_afe_disable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll)
There's a tabulation here, please use spaces....
> +{
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> +
> + mutex_lock(&afe_priv->afe_clk_mutex);
> +
> + afe_priv->apll_tuner_ref_cnt[apll]--;
> + if (afe_priv->apll_tuner_ref_cnt[apll] == 0) {
> + if (apll == MT8365_AFE_APLL1)
> + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG,
> + AFE_APLL_TUNER_CFG_EN_MASK, 0x0);
> + else
> + regmap_update_bits(afe->regmap, AFE_APLL_TUNER_CFG1,
> + AFE_APLL_TUNER_CFG1_EN_MASK, 0x0);
> +
> + } else if (afe_priv->apll_tuner_ref_cnt[apll] < 0) {
> + afe_priv->apll_tuner_ref_cnt[apll] = 0;
> + }
> +
> + mutex_unlock(&afe_priv->afe_clk_mutex);
> + return 0;
> +}
> +
> +int mt8365_afe_enable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll)
> +{
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> +
> + if (apll == MT8365_AFE_APLL1) {
> + if (clk_prepare_enable(afe_priv->clocks[MT8365_CLK_ENGEN1])) {
> + dev_info(afe->dev, "%s Failed to enable ENGEN1 clk\n",
> + __func__);
> + return 0;
> + }
> + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_22M);
> + mt8365_afe_hd_engen_enable(afe, true);
> +#ifdef ENABLE_AFE_APLL_TUNER
> + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_APLL_TUNER);
> + mt8365_afe_enable_apll_tuner_cfg(afe, MT8365_AFE_APLL1);
> +#endif
> + } else {
> + if (clk_prepare_enable(afe_priv->clocks[MT8365_CLK_ENGEN2])) {
> + dev_info(afe->dev, "%s Failed to enable ENGEN2 clk\n",
> + __func__);
> + return 0;
> + }
> + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_24M);
> + mt8365_afe_hd_engen_enable(afe, false);
> +#ifdef ENABLE_AFE_APLL_TUNER
Those ifdefs are ugly. If you're not using that code, please remove.
> + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_APLL2_TUNER);
> + mt8365_afe_enable_apll_tuner_cfg(afe, MT8365_AFE_APLL2);
> +#endif
> + }
> +
> + return 0;
> +}
> +
> +int mt8365_afe_disable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll)
> +{
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> +
> + if (apll == MT8365_AFE_APLL1) {
> +#ifdef ENABLE_AFE_APLL_TUNER
> + mt8365_afe_disable_apll_tuner_cfg(afe, MT8365_AFE_APLL1);
> + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_APLL_TUNER);
> +#endif
> + mt8365_afe_hd_engen_disable(afe, true);
> + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_22M);
> + clk_disable_unprepare(afe_priv->clocks[MT8365_CLK_ENGEN1]);
> + } else {
> +#ifdef ENABLE_AFE_APLL_TUNER
> + mt8365_afe_disable_apll_tuner_cfg(afe, MT8365_AFE_APLL2);
> + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_APLL2_TUNER);
> +#endif
> + mt8365_afe_hd_engen_disable(afe, false);
> + mt8365_afe_disable_top_cg(afe, MT8365_TOP_CG_24M);
> + clk_disable_unprepare(afe_priv->clocks[MT8365_CLK_ENGEN2]);
> + }
> +
> + return 0;
> +}
> diff --git a/sound/soc/mediatek/mt8365/mt8365-afe-clk.h b/sound/soc/mediatek/mt8365/mt8365-afe-clk.h
> new file mode 100644
> index 000000000000..14fca6ae2641
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8365/mt8365-afe-clk.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Mediatek 8365 AFE clock control definitions
> + *
> + * Copyright (c) 2024 MediaTek Inc.
> + * Authors: Jia Zeng <jia.zeng@...iatek.com>
> + * Alexandre Mergnat <amergnat@...libre.com>
> + */
> +
> +#ifndef _MT8365_AFE_UTILS_H_
> +#define _MT8365_AFE_UTILS_H_
> +
> +struct mtk_base_afe;
> +struct clk;
> +
> +int mt8365_afe_init_audio_clk(struct mtk_base_afe *afe);
> +
Please drop all those blank lines between function signatures, they're not needed.
Cheers,
Angelo
> +int mt8365_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk);
> +
> +void mt8365_afe_disable_clk(struct mtk_base_afe *afe, struct clk *clk);
> +
> +int mt8365_afe_set_clk_rate(struct mtk_base_afe *afe, struct clk *clk, unsigned int rate);
> +
> +int mt8365_afe_set_clk_parent(struct mtk_base_afe *afe, struct clk *clk, struct clk *parent);
> +
> +int mt8365_afe_enable_top_cg(struct mtk_base_afe *afe, unsigned int cg_type);
> +
> +int mt8365_afe_disable_top_cg(struct mtk_base_afe *afe, unsigned int cg_type);
> +
> +int mt8365_afe_enable_main_clk(struct mtk_base_afe *afe);
> +
> +int mt8365_afe_disable_main_clk(struct mtk_base_afe *afe);
> +
> +int mt8365_afe_emi_clk_on(struct mtk_base_afe *afe);
> +
> +int mt8365_afe_emi_clk_off(struct mtk_base_afe *afe);
> +
> +int mt8365_afe_enable_afe_on(struct mtk_base_afe *afe);
> +
> +int mt8365_afe_disable_afe_on(struct mtk_base_afe *afe);
> +
> +int mt8365_afe_enable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll);
> +
> +int mt8365_afe_disable_apll_tuner_cfg(struct mtk_base_afe *afe, unsigned int apll);
> +
> +int mt8365_afe_enable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll);
> +
> +int mt8365_afe_disable_apll_associated_cfg(struct mtk_base_afe *afe, unsigned int apll);
> +#endif
>
Powered by blists - more mailing lists