Re: [PATCH v2 2/4] ASoC: mediatek: mt8183: Fix APLL enable error handling
From: Cezary Rojewski <cezary.rojewski@intel.com>
Date: 2026-08-26 18:31:29
Also in:
linux-mediatek, linux-sound, lkml
On 8/21/2026 2:29 PM, phucduc.bui@gmail.com wrote:
From: bui duc phuc <redacted> Currently, the mt8183_apll*_enable() functions call mux_setting(afe, true) but do not check its return value to handle failures. In addition, the cleanup paths of mt8183_apll*_enable() do not call mux_setting(afe, false) when the enable operation fails, while the mt8183_apll*_disable() functions do. Add error handling for apll*_mux_setting() and call mux_setting(afe, false) in the cleanup paths when mt8183_apll*_enable() fails.
I have mixed feelings about appl*_mux_setting(). Take a look at its disable-path: if clk_set_parent() fails the follow up clk_disable_unprepare() is skipped possibly leaving one of the clks hanging. I'd expect error paths of callers (of said mux_setting() function) to ensure all the clks are disabled and unprepared unconditionally.
quoted hunk ↗ jump to hunk
Fixes: a94aec035a12 ("ASoC: mediatek: mt8183: add platform driver") Signed-off-by: bui duc phuc <redacted> --- sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)diff --git a/sound/soc/mediatek/mt8183/mt8183-afe-clk.c b/sound/soc/mediatek/mt8183/mt8183-afe-clk.c index 6ab0734ad136..0790d8123179 100644 --- a/sound/soc/mediatek/mt8183/mt8183-afe-clk.c +++ b/sound/soc/mediatek/mt8183/mt8183-afe-clk.c@@ -382,7 +382,9 @@ int mt8183_apll1_enable(struct mtk_base_afe *afe) int ret; /* setting for APLL */ - apll1_mux_setting(afe, true); + ret = apll1_mux_setting(afe, true); + if (ret) + goto ERR_APLL1_MUX_SETTING;
Why goto? The check is valid but the label in my opinion is unnecessary.
quoted hunk ↗ jump to hunk
ret = clk_prepare_enable(afe_priv->clk[CLK_APLL22M]); if (ret) {@@ -411,6 +413,8 @@ int mt8183_apll1_enable(struct mtk_base_afe *afe) ERR_CLK_APLL1_TUNER: clk_disable_unprepare(afe_priv->clk[CLK_APLL22M]); ERR_CLK_APLL22M: + apll1_mux_setting(afe, false); +ERR_APLL1_MUX_SETTING:
While UPPER case for goto-labels is not part of the coding style I see why you did it - to be cohesive with the rest of the file.