Skip to contentSkip to conversation

ASoC: mediatek: mt2701: Use devm_clk_get_optional() for audio_mrgif_pd

PATCH In review [email protected] 3 participants 9 messages Aug 17 – Sep 1

Subsystems (3)
Sound ASoC Arm MediaTek
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/3 ASoC: mediatek: mt2701: Use devm_clk_get_optional() for audio_mrgif_pd

From: bui duc phuc <[email protected]>

The audio_mrgif_pd clock is optional as some platforms may support the BT path while others do not. Use devm_clk_get_optional() to reflect this.

Propagate other errors to the caller.

Signed-off-by bui duc phuc <[email protected]>
---
 sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c +3 -7
index d217f9320ad2..18f0f27bd8c0 100644
@@ -89,13 +89,9 @@ int mt2701_init_clock(struct mtk_base_afe *afe)
 	}
 
 	/* Some platforms may support BT path */
-	afe_priv->mrgif_ck = devm_clk_get(afe->dev, "audio_mrgif_pd");
-	if (IS_ERR(afe_priv->mrgif_ck)) {
-		if (PTR_ERR(afe_priv->mrgif_ck) == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
-
-		afe_priv->mrgif_ck = NULL;
-	}
+	afe_priv->mrgif_ck = devm_clk_get_optional(afe->dev, "audio_mrgif_pd");
+	if (IS_ERR(afe_priv->mrgif_ck))
+		return PTR_ERR(afe_priv->mrgif_ck);
 
 	/*
 	 * Optional HDMI audio clocks. Platforms that do not wire up the
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH 2/3 ASoC: mediatek: mt2701: Use dev_err_probe() for error handling

From: bui duc phuc <[email protected]>

Replace dev_err() with dev_err_probe() to prevent log spam when probe returns -EPROBE_DEFER.

Signed-off-by bui duc phuc <[email protected]>
---
 .../mediatek/mt2701/mt2701-afe-clock-ctrl.c   | 51 +++++++++----------
 1 file changed, 23 insertions(+), 28 deletions(-)
sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c +23 -28
index 18f0f27bd8c0..1cf66ca44a33 100644
@@ -30,10 +30,9 @@ int mt2701_init_clock(struct mtk_base_afe *afe)
 
 	for (i = 0; i < MT2701_BASE_CLK_NUM; i++) {
 		afe_priv->base_ck[i] = devm_clk_get(afe->dev, base_clks[i]);
-		if (IS_ERR(afe_priv->base_ck[i])) {
-			dev_err(afe->dev, "failed to get %s\n", base_clks[i]);
-			return PTR_ERR(afe_priv->base_ck[i]);
-		}
+		if (IS_ERR(afe_priv->base_ck[i]))
+			return dev_err_probe(afe->dev, PTR_ERR(afe_priv->base_ck[i]),
+					     "failed to get %s\n", base_clks[i]);
 	}
 
 	i2s_num = min(afe_priv->soc->i2s_num, MT2701_BASE_CLK_NUM);

@@ -45,47 +44,43 @@ int mt2701_init_clock(struct mtk_base_afe *afe)
 
 		snprintf(name, sizeof(name), "i2s%d_src_sel", i);
 		i2s_path->sel_ck = devm_clk_get(afe->dev, name);
-		if (IS_ERR(i2s_path->sel_ck)) {
-			dev_err(afe->dev, "failed to get %s\n", name);
-			return PTR_ERR(i2s_path->sel_ck);
-		}
+		if (IS_ERR(i2s_path->sel_ck))
+			return dev_err_probe(afe->dev, PTR_ERR(i2s_path->sel_ck),
+					     "failed to get %s\n", name);
 
 		snprintf(name, sizeof(name), "i2s%d_src_div", i);
 		i2s_path->div_ck = devm_clk_get(afe->dev, name);
-		if (IS_ERR(i2s_path->div_ck)) {
-			dev_err(afe->dev, "failed to get %s\n", name);
-			return PTR_ERR(i2s_path->div_ck);
-		}
+		if (IS_ERR(i2s_path->div_ck))
+			return dev_err_probe(afe->dev, PTR_ERR(i2s_path->div_ck),
+					     "failed to get %s\n", name);
 
 		snprintf(name, sizeof(name), "i2s%d_mclk_en", i);
 		i2s_path->mclk_ck = devm_clk_get(afe->dev, name);
-		if (IS_ERR(i2s_path->mclk_ck)) {
-			dev_err(afe->dev, "failed to get %s\n", name);
-			return PTR_ERR(i2s_path->mclk_ck);
-		}
+		if (IS_ERR(i2s_path->mclk_ck))
+			return dev_err_probe(afe->dev, PTR_ERR(i2s_path->mclk_ck),
+				       "failed to get %s\n", name);
 
 		snprintf(name, sizeof(name), "i2so%d_hop_ck", i);
 		i2s_ck = devm_clk_get(afe->dev, name);
-		if (IS_ERR(i2s_ck)) {
-			dev_err(afe->dev, "failed to get %s\n", name);
-			return PTR_ERR(i2s_ck);
-		}
+		if (IS_ERR(i2s_ck))
+			return dev_err_probe(afe->dev, PTR_ERR(i2s_ck),
+					     "failed to get %s\n", name);
+
 		i2s_path->hop_ck[SNDRV_PCM_STREAM_PLAYBACK] = i2s_ck;
 
 		snprintf(name, sizeof(name), "i2si%d_hop_ck", i);
 		i2s_ck = devm_clk_get(afe->dev, name);
-		if (IS_ERR(i2s_ck)) {
-			dev_err(afe->dev, "failed to get %s\n", name);
-			return PTR_ERR(i2s_ck);
-		}
+		if (IS_ERR(i2s_ck))
+			return dev_err_probe(afe->dev, PTR_ERR(i2s_ck),
+					     "failed to get %s\n", name);
+
 		i2s_path->hop_ck[SNDRV_PCM_STREAM_CAPTURE] = i2s_ck;
 
 		snprintf(name, sizeof(name), "asrc%d_out_ck", i);
 		i2s_path->asrco_ck = devm_clk_get(afe->dev, name);
-		if (IS_ERR(i2s_path->asrco_ck)) {
-			dev_err(afe->dev, "failed to get %s\n", name);
-			return PTR_ERR(i2s_path->asrco_ck);
-		}
+		if (IS_ERR(i2s_path->asrco_ck))
+			return dev_err_probe(afe->dev, PTR_ERR(i2s_path->asrco_ck),
+					     "failed to get %s\n", name);
 	}
 
 	/* Some platforms may support BT path */
Signature · 1 line
2.43.0
author
#3 lore raw

PATCH 3/3 ASoC: mediatek: mt2701: Drop redundant probe error messages

In reply to [email protected]

From: bui duc phuc <[email protected]>

The errors handled here are already reported by the called functions, either directly or deeper in the call chain. Therefore, the additional dev_err() calls are redundant and can be removed.

Signed-off-by bui duc phuc <[email protected]>
---
 sound/soc/mediatek/mt2701/mt2701-afe-pcm.c | 4 +---
 sound/soc/mediatek/mt2701/mt2701-cs42448.c | 4 +---
 sound/soc/mediatek/mt2701/mt2701-wm8960.c  | 4 +---
 3 files changed, 3 insertions(+), 9 deletions(-)
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c +1 -3
index d56b498e8c0c..3c5a76591ea7 100644
@@ -1622,10 +1622,8 @@ static int mt2701_afe_pcm_dev_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, irq_id, mt2701_asys_isr,
 			       IRQF_TRIGGER_NONE, "asys-isr", (void *)afe);
-	if (ret) {
-		dev_err(dev, "could not request_irq for asys-isr\n");
+	if (ret)
 		return ret;
-	}
 
 	afe->regmap = syscon_node_to_regmap(dev->parent->of_node);
 	if (IS_ERR(afe->regmap)) {
sound/soc/mediatek/mt2701/mt2701-cs42448.c +1 -3
index 778a9dccfcaa..af833a8daa2a 100644
@@ -366,10 +366,8 @@ static int mt2701_cs42448_machine_probe(struct platform_device *pdev)
 							= codec_node_bt_mrg;
 
 	ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
-	if (ret) {
-		dev_err(dev, "failed to parse audio-routing: %d\n", ret);
+	if (ret)
 		return ret;
-	}
 
 	priv->i2s1_in_mux_sel_1 = devm_gpiod_get_optional(dev, "i2s1-in-sel-gpio1",
 							  GPIOD_OUT_LOW);
sound/soc/mediatek/mt2701/mt2701-wm8960.c +1 -3
index 84b3d6cd77a5..25ea3e274aa8 100644
@@ -137,10 +137,8 @@ static int mt2701_wm8960_machine_probe(struct platform_device *pdev)
 	}
 
 	ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
-	if (ret) {
-		dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
+	if (ret)
 		goto put_codec_node;
-	}
 
 	ret = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret)
Signature · 1 line
2.43.0
Reviewed-by on v1 3/3
#4 lore raw

Re: [PATCH 3/3] ASoC: mediatek: mt2701: Drop redundant probe error messages

Quoted 7 lines · On 8/17/2026 12:49 PM, [email protected] wrote:

From: bui duc phuc <[email protected]>

The errors handled here are already reported by the called functions, either directly or deeper in the call chain. Therefore, the additional dev_err() calls are redundant and can be removed.

Signed-off-by bui duc phuc <[email protected]>
Reviewed-by Cezary Rojewski <[email protected]>
#5 lore raw
In reply to [email protected]
Quoted 7 lines · On 8/17/2026 12:49 PM, [email protected] wrote:

From: bui duc phuc <[email protected]>

The audio_mrgif_pd clock is optional as some platforms may support the BT path while others do not. Use devm_clk_get_optional() to reflect this.

Propagate other errors to the caller.

What does that mean? Did the functional path change?

Quoted 1 line
Signed-off-by bui duc phuc <[email protected]>

This should be part of a mediatek-patch-series with a cover letter. Yeah, the mt6797 changes should be here too.

author
#6 lore raw

Hi Cezary,

Thank you for your review.

Quoted 8 lines
Quoted 5 lines

The audio_mrgif_pd clock is optional as some platforms may support the BT path while others do not. Use devm_clk_get_optional() to reflect this.

Propagate other errors to the caller.

What does that mean? Did the functional path change?

Previously, the code only returned an error for -EPROBE_DEFER and ignored other errors.
After switching to devm_clk_get_optional(), all other errors are propagated to the caller as well.

Regarding the functional path, I don't think it changes on platforms where the clock is present.
The change only allows platforms without this optional clock to continue probing.

Quoted 3 lines
Quoted 1 line
Signed-off-by bui duc phuc <[email protected]>

This should be part of a mediatek-patch-series with a cover letter. Yeah, the mt6797 changes should be here too.

Currently, the MediaTek ASoC drivers are split into multiple groups by SoC. I am handling them one group at a time to make the review easier.

I will send each group once it is completed, rather than waiting for the entire MediaTek cleanup, which would take quite a long time. A single series covering all
MediaTek SoCs would also be very long and harder to follow, especially if changes are requested during review.

I hope this approach works for you.

Best regards,
Phuc

Reviewed-by
#7 lore raw
Quoted 21 lines · Bui Duc Phuc
Quoted 4 lines
Quoted 1 line

Propagate other errors to the caller.

What does that mean? Did the functional path change?

Previously, the code only returned an error for -EPROBE_DEFER and ignored other errors.
After switching to devm_clk_get_optional(), all other errors are propagated to the caller as well.

Regarding the functional path, I don't think it changes on platforms where the clock is present.
The change only allows platforms without this optional clock to continue probing.

Quoted 2 lines

This should be part of a mediatek-patch-series with a cover letter. Yeah, the mt6797 changes should be here too.

Currently, the MediaTek ASoC drivers are split into multiple groups by SoC. I am handling them one group at a time to make the review easier.

Quoted 7 lines

I will send each group once it is completed, rather than waiting for the entire MediaTek cleanup, which would take quite a long time. A single series covering all
MediaTek SoCs would also be very long and harder to follow, especially if changes are requested during review.

I hope this approach works for you.

Thanks for the explanations. I see only benefits from these changes.

Reviewed-by Cezary Rojewski <[email protected]>
maintainer
#8 lore raw
In reply to [email protected]
Quoted 1 line · On Mon, 17 Aug 2026 17:49:27 +0700, [email protected] wrote:

ASoC: mediatek: mt2701: Use devm_clk_get_optional() for audio_mrgif_pd

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.4

Thanks!

[1/3] ASoC: mediatek: mt2701: Use devm_clk_get_optional() for audio_mrgif_pd

      https://git.kernel.org/broonie/sound/c/e1c41b5395a1
[2/3] ASoC: mediatek: mt2701: Use dev_err_probe() for error handling
      https://git.kernel.org/broonie/sound/c/d468c1c89e7d
[3/3] ASoC: mediatek: mt2701: Drop redundant probe error messages
      https://git.kernel.org/broonie/sound/c/fd97db580cea

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark