[PATCH v1 1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

[PATCH v1 1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

[Andy Shevchenko]

After the commit 7735bce05a9c ("ASoC: Intel: boards: use devm_clk_get()
unconditionally") the driver assumes that mclk is always provided, hence
no need to check for it being NULL anymore.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/intel/boards/cht_bsw_rt5672.c | 56 +++++++++++--------------
 1 file changed, 24 insertions(+), 32 deletions(-)

diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index fd4cefd298d2..9f59bee88e6f 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -63,13 +63,11 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 	}
 
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
-		if (ctx->mclk) {
-			ret = clk_prepare_enable(ctx->mclk);
-			if (ret < 0) {
-				dev_err(card->dev,
-					"could not configure MCLK state: %d\n", ret);
-				return ret;
-			}
+		ret = clk_prepare_enable(ctx->mclk);
+		if (ret < 0) {
+			dev_err(card->dev,
+				"could not configure MCLK state: %d\n", ret);
+			return ret;
 		}
 
 		/* set codec PLL source to the 19.2MHz platform clock (MCLK) */
@@ -77,8 +75,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 				CHT_PLAT_CLK_3_HZ, 48000 * 512);
 		if (ret < 0) {
 			dev_err(card->dev, "can't set codec pll: %d\n", ret);
-			if (ctx->mclk)
-				clk_disable_unprepare(ctx->mclk);
+			clk_disable_unprepare(ctx->mclk);
 			return ret;
 		}
 
@@ -87,8 +84,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 			48000 * 512, SND_SOC_CLOCK_IN);
 		if (ret < 0) {
 			dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
-			if (ctx->mclk)
-				clk_disable_unprepare(ctx->mclk);
+			clk_disable_unprepare(ctx->mclk);
 			return ret;
 		}
 	} else {
@@ -104,8 +100,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 			return ret;
 		}
 
-		if (ctx->mclk)
-			clk_disable_unprepare(ctx->mclk);
+		clk_disable_unprepare(ctx->mclk);
 	}
 	return 0;
 }
@@ -244,28 +239,25 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 	snd_jack_set_key(ctx->headset.jack, SND_JACK_BTN_2, KEY_VOLUMEDOWN);
 
 	rt5670_set_jack_detect(component, &ctx->headset);
-	if (ctx->mclk) {
-		/*
-		 * The firmware might enable the clock at
-		 * boot (this information may or may not
-		 * be reflected in the enable clock register).
-		 * To change the rate we must disable the clock
-		 * first to cover these cases. Due to common
-		 * clock framework restrictions that do not allow
-		 * to disable a clock that has not been enabled,
-		 * we need to enable the clock first.
-		 */
-		ret = clk_prepare_enable(ctx->mclk);
-		if (!ret)
-			clk_disable_unprepare(ctx->mclk);
 
-		ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
+	/*
+	 * The firmware might enable the clock at boot (this information
+	 * may or may not be reflected in the enable clock register).
+	 * To change the rate we must disable the clock first to cover
+	 * these cases. Due to Common Clock Framework restrictions that
+	 * do not allow to disable a clock that has not been enabled, we
+	 * need to enable the clock first.
+	 */
+	ret = clk_prepare_enable(ctx->mclk);
+	if (!ret)
+		clk_disable_unprepare(ctx->mclk);
 
-		if (ret) {
-			dev_err(runtime->dev, "unable to set MCLK rate\n");
-			return ret;
-		}
+	ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ);
+	if (ret) {
+		dev_err(runtime->dev, "unable to set MCLK rate\n");
+		return ret;
 	}
+
 	return 0;
 }
 
-- 
2.50.1

Re: [PATCH v1 1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

On Tue, Apr 28, 2026 at 09:47:58AM +0200, Andy Shevchenko wrote:
> After the commit 7735bce05a9c ("ASoC: Intel: boards: use devm_clk_get()
> unconditionally") the driver assumes that mclk is always provided, hence
> no need to check for it being NULL anymore.

There's also the fact that as part of support for optional clocks the
clock API will quite happily consume NULL clocks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 05:07:39PM +0900, Mark Brown wrote:
> On Tue, Apr 28, 2026 at 09:47:58AM +0200, Andy Shevchenko wrote:
> > After the commit 7735bce05a9c ("ASoC: Intel: boards: use devm_clk_get()
> > unconditionally") the driver assumes that mclk is always provided, hence
> > no need to check for it being NULL anymore.
> 
> There's also the fact that as part of support for optional clocks the
> clock API will quite happily consume NULL clocks.

This clock is not optional, it's always retrieved with devm_clk_get().
When the flow runs into these code pieces the mclk != NULL, and always
valid (excluding some unrelated to this driver strange data corruption cases,
of course).

But yes, the CCF APIs are NULL-aware for a long time.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

On Tue, Apr 28, 2026 at 11:16:56AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 28, 2026 at 05:07:39PM +0900, Mark Brown wrote:

> > There's also the fact that as part of support for optional clocks the
> > clock API will quite happily consume NULL clocks.

> This clock is not optional, it's always retrieved with devm_clk_get().
> When the flow runs into these code pieces the mclk != NULL, and always
> valid (excluding some unrelated to this driver strange data corruption cases,
> of course).

> But yes, the CCF APIs are NULL-aware for a long time.

My point is that it makes no difference if the caller does the NULL
check.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 05:37:44PM +0900, Mark Brown wrote:
> On Tue, Apr 28, 2026 at 11:16:56AM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 28, 2026 at 05:07:39PM +0900, Mark Brown wrote:
> 
> > > There's also the fact that as part of support for optional clocks the
> > > clock API will quite happily consume NULL clocks.
> 
> > This clock is not optional, it's always retrieved with devm_clk_get().
> > When the flow runs into these code pieces the mclk != NULL, and always
> > valid (excluding some unrelated to this driver strange data corruption cases,
> > of course).
> 
> > But yes, the CCF APIs are NULL-aware for a long time.
> 
> My point is that it makes no difference if the caller does the NULL
> check.

The dead code or always-true one is not welcome in the kernel, it increases
unneeded burden on maintaining and understanding. While your point is correct
the lesser the code base the better. I don't believe we ever get this clock
become conditional (optional) again.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

[Mark Brown]

On Tue, 28 Apr 2026 09:47:58 +0200, Andy Shevchenko wrote:
> ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks

Applied to

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

Thanks!

[1/1] ASoC: Intel: cht_bsw_rt5672: Drop unneeded NULL checks
      https://git.kernel.org/broonie/sound/c/ee26bad1b52c

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