Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1646171 > unrolled thread

[PATCH] ASoC: remove NULL pointer check for clk_disable_unprepare

Started byMasahiro Yamada <yamada.masahiro@socionext.com>
First post2017-05-20 19:50 +0200
Last post2017-05-20 21:00 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] ASoC: remove NULL pointer check for clk_disable_unprepare Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-05-20 19:50 +0200
    Re: [PATCH] ASoC: remove NULL pointer check for clk_disable_unprepare Krzysztof Kozlowski <krzk@kernel.org> - 2017-05-20 20:10 +0200
      Re: [PATCH] ASoC: remove NULL pointer check for clk_disable_unprepare Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-05-20 20:50 +0200
        Re: [PATCH] ASoC: remove NULL pointer check for clk_disable_unprepare Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-05-20 21:00 +0200

#1646171 — [PATCH] ASoC: remove NULL pointer check for clk_disable_unprepare

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2017-05-20 19:50 +0200
Subject[PATCH] ASoC: remove NULL pointer check for clk_disable_unprepare
Message-ID<tJgFQ-4vo-9@gated-at.bofh.it>
After long term efforts of fixing non-common clock implementations,
clk_disable() is a no-op for a NULL pointer input, and this is now
tree-wide consistent.

All clock consumers can safely call clk_disable(_unprepare) without
NULL pointer check.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 sound/soc/codecs/adau17x1.c                | 3 +--
 sound/soc/codecs/da7213.c                  | 3 +--
 sound/soc/codecs/da7218.c                  | 3 +--
 sound/soc/codecs/da7219-aad.c              | 5 ++---
 sound/soc/codecs/da7219.c                  | 3 +--
 sound/soc/codecs/es8328.c                  | 3 +--
 sound/soc/codecs/wm8731.c                  | 3 +--
 sound/soc/davinci/davinci-evm.c            | 3 +--
 sound/soc/fsl/imx-audmux.c                 | 6 ++----
 sound/soc/intel/boards/bytcr_rt5640.c      | 2 +-
 sound/soc/intel/boards/cht_bsw_rt5645.c    | 3 +--
 sound/soc/mediatek/mt8173/mt8173-afe-pcm.c | 6 ++----
 sound/soc/samsung/i2s.c                    | 3 +--
 13 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c
index 2c1bd27..f50739b 100644
--- a/sound/soc/codecs/adau17x1.c
+++ b/sound/soc/codecs/adau17x1.c
@@ -977,8 +977,7 @@ void adau17x1_remove(struct device *dev)
 	struct adau *adau = dev_get_drvdata(dev);
 
 	snd_soc_unregister_codec(dev);
-	if (adau->mclk)
-		clk_disable_unprepare(adau->mclk);
+	clk_disable_unprepare(adau->mclk);
 }
 EXPORT_SYMBOL_GPL(adau17x1_remove);
 
diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
index 6dd7578..130e16a 100644
--- a/sound/soc/codecs/da7213.c
+++ b/sound/soc/codecs/da7213.c
@@ -1516,8 +1516,7 @@ static int da7213_set_bias_level(struct snd_soc_codec *codec,
 					    DA7213_VMID_EN | DA7213_BIAS_EN);
 		} else {
 			/* Remove MCLK */
-			if (da7213->mclk)
-				clk_disable_unprepare(da7213->mclk);
+			clk_disable_unprepare(da7213->mclk);
 		}
 		break;
 	case SND_SOC_BIAS_OFF:
diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
index d256ebf..eeb0bb6 100644
--- a/sound/soc/codecs/da7218.c
+++ b/sound/soc/codecs/da7218.c
@@ -2611,8 +2611,7 @@ static int da7218_set_bias_level(struct snd_soc_codec *codec,
 					    DA7218_LDO_EN_MASK);
 		} else {
 			/* Remove MCLK */
-			if (da7218->mclk)
-				clk_disable_unprepare(da7218->mclk);
+			clk_disable_unprepare(da7218->mclk);
 		}
 		break;
 	case SND_SOC_BIAS_OFF:
diff --git a/sound/soc/codecs/da7219-aad.c b/sound/soc/codecs/da7219-aad.c
index 6274d79..31518df 100644
--- a/sound/soc/codecs/da7219-aad.c
+++ b/sound/soc/codecs/da7219-aad.c
@@ -302,9 +302,8 @@ static void da7219_aad_hptest_work(struct work_struct *work)
 	snd_soc_update_bits(codec, DA7219_HP_R_CTRL, DA7219_HP_R_AMP_OE_MASK,
 			    DA7219_HP_R_AMP_OE_MASK);
 
-	/* Remove MCLK, if previously enabled */
-	if (da7219->mclk)
-		clk_disable_unprepare(da7219->mclk);
+	/* Remove MCLK */
+	clk_disable_unprepare(da7219->mclk);
 
 	mutex_unlock(&da7219->lock);
 	snd_soc_dapm_mutex_unlock(dapm);
diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
index 9960162..307039e 100644
--- a/sound/soc/codecs/da7219.c
+++ b/sound/soc/codecs/da7219.c
@@ -1630,8 +1630,7 @@ static int da7219_set_bias_level(struct snd_soc_codec *codec,
 
 		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_PREPARE) {
 			/* Remove MCLK */
-			if (da7219->mclk)
-				clk_disable_unprepare(da7219->mclk);
+			clk_disable_unprepare(da7219->mclk);
 		}
 		break;
 	case SND_SOC_BIAS_OFF:
diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
index ed7cc42..848e0b3 100644
--- a/sound/soc/codecs/es8328.c
+++ b/sound/soc/codecs/es8328.c
@@ -812,8 +812,7 @@ static int es8328_remove(struct snd_soc_codec *codec)
 
 	es8328 = snd_soc_codec_get_drvdata(codec);
 
-	if (es8328->clk)
-		clk_disable_unprepare(es8328->clk);
+	clk_disable_unprepare(es8328->clk);
 
 	regulator_bulk_disable(ARRAY_SIZE(es8328->supplies),
 			       es8328->supplies);
diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c
index 4f9a1eb..302ed88 100644
--- a/sound/soc/codecs/wm8731.c
+++ b/sound/soc/codecs/wm8731.c
@@ -517,8 +517,7 @@ static int wm8731_set_bias_level(struct snd_soc_codec *codec,
 		snd_soc_write(codec, WM8731_PWR, reg | 0x0040);
 		break;
 	case SND_SOC_BIAS_OFF:
-		if (wm8731->mclk)
-			clk_disable_unprepare(wm8731->mclk);
+		clk_disable_unprepare(wm8731->mclk);
 		snd_soc_write(codec, WM8731_PWR, 0xffff);
 		regulator_bulk_disable(ARRAY_SIZE(wm8731->supplies),
 				       wm8731->supplies);
diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c
index 7a369e0..07a91c7 100644
--- a/sound/soc/davinci/davinci-evm.c
+++ b/sound/soc/davinci/davinci-evm.c
@@ -49,8 +49,7 @@ static void evm_shutdown(struct snd_pcm_substream *substream)
 	struct snd_soc_card_drvdata_davinci *drvdata =
 		snd_soc_card_get_drvdata(soc_card);
 
-	if (drvdata->mclk)
-		clk_disable_unprepare(drvdata->mclk);
+	clk_disable_unprepare(drvdata->mclk);
 }
 
 static int evm_hw_params(struct snd_pcm_substream *substream,
diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c
index fc57da3..a7561e3 100644
--- a/sound/soc/fsl/imx-audmux.c
+++ b/sound/soc/fsl/imx-audmux.c
@@ -79,8 +79,7 @@ static ssize_t audmux_read_file(struct file *file, char __user *user_buf,
 	ptcr = readl(audmux_base + IMX_AUDMUX_V2_PTCR(port));
 	pdcr = readl(audmux_base + IMX_AUDMUX_V2_PDCR(port));
 
-	if (audmux_clk)
-		clk_disable_unprepare(audmux_clk);
+	clk_disable_unprepare(audmux_clk);
 
 	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!buf)
@@ -245,8 +244,7 @@ int imx_audmux_v2_configure_port(unsigned int port, unsigned int ptcr,
 	writel(ptcr, audmux_base + IMX_AUDMUX_V2_PTCR(port));
 	writel(pdcr, audmux_base + IMX_AUDMUX_V2_PDCR(port));
 
-	if (audmux_clk)
-		clk_disable_unprepare(audmux_clk);
+	clk_disable_unprepare(audmux_clk);
 
 	return 0;
 }
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 4a76b09..8c2a7a8 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -199,7 +199,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 					     48000 * 512,
 					     SND_SOC_CLOCK_IN);
 		if (!ret) {
-			if ((byt_rt5640_quirk & BYT_RT5640_MCLK_EN) && priv->mclk)
+			if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN)
 				clk_disable_unprepare(priv->mclk);
 		}
 	}
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index 5bcde01..cb6f587 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -122,8 +122,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;
diff --git a/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c b/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
index 8a643a3..8857c08 100644
--- a/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
+++ b/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
@@ -294,10 +294,8 @@ static int mt8173_afe_dais_set_clks(struct mtk_base_afe *afe,
 static void mt8173_afe_dais_disable_clks(struct mtk_base_afe *afe,
 					 struct clk *m_ck, struct clk *b_ck)
 {
-	if (m_ck)
-		clk_disable_unprepare(m_ck);
-	if (b_ck)
-		clk_disable_unprepare(b_ck);
+	clk_disable_unprepare(m_ck);
+	clk_disable_unprepare(b_ck);
 }
 
 static int mt8173_afe_i2s_startup(struct snd_pcm_substream *substream,
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index af3ba4d..3a06acd 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -1122,8 +1122,7 @@ static int i2s_runtime_suspend(struct device *dev)
 	i2s->suspend_i2scon = readl(i2s->addr + I2SCON);
 	i2s->suspend_i2spsr = readl(i2s->addr + I2SPSR);
 
-	if (i2s->op_clk)
-		clk_disable_unprepare(i2s->op_clk);
+	clk_disable_unprepare(i2s->op_clk);
 	clk_disable_unprepare(i2s->clk);
 
 	return 0;
-- 
2.7.4

[toc] | [next] | [standalone]

#1646177

FromKrzysztof Kozlowski <krzk@kernel.org>
Date2017-05-20 20:10 +0200
Message-ID<tJgZb-4S9-13@gated-at.bofh.it>
In reply to#1646171
On Sun, May 21, 2017 at 02:42:38AM +0900, Masahiro Yamada wrote:
> After long term efforts of fixing non-common clock implementations,
> clk_disable() is a no-op for a NULL pointer input, and this is now
> tree-wide consistent.
> 
> All clock consumers can safely call clk_disable(_unprepare) without
> NULL pointer check.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  sound/soc/codecs/adau17x1.c                | 3 +--
>  sound/soc/codecs/da7213.c                  | 3 +--
>  sound/soc/codecs/da7218.c                  | 3 +--
>  sound/soc/codecs/da7219-aad.c              | 5 ++---
>  sound/soc/codecs/da7219.c                  | 3 +--
>  sound/soc/codecs/es8328.c                  | 3 +--
>  sound/soc/codecs/wm8731.c                  | 3 +--
>  sound/soc/davinci/davinci-evm.c            | 3 +--
>  sound/soc/fsl/imx-audmux.c                 | 6 ++----
>  sound/soc/intel/boards/bytcr_rt5640.c      | 2 +-
>  sound/soc/intel/boards/cht_bsw_rt5645.c    | 3 +--
>  sound/soc/mediatek/mt8173/mt8173-afe-pcm.c | 6 ++----
>  sound/soc/samsung/i2s.c                    | 3 +--
>  13 files changed, 16 insertions(+), 30 deletions(-)
> 

(...)

> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
> index af3ba4d..3a06acd 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -1122,8 +1122,7 @@ static int i2s_runtime_suspend(struct device *dev)
>  	i2s->suspend_i2scon = readl(i2s->addr + I2SCON);
>  	i2s->suspend_i2spsr = readl(i2s->addr + I2SPSR);
>  
> -	if (i2s->op_clk)
> -		clk_disable_unprepare(i2s->op_clk);
> +	clk_disable_unprepare(i2s->op_clk);
>  	clk_disable_unprepare(i2s->clk);

I think the check in i2s_runtime_resume() should be removed then as well
to keep it symmetrical. Otherwise it looks suspicious.

Best regards,
Krzysztof

[toc] | [prev] | [next] | [standalone]

#1646183

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2017-05-20 20:50 +0200
Message-ID<tJhBT-55Q-7@gated-at.bofh.it>
In reply to#1646177
Hi Krzysztof,


2017-05-21 3:04 GMT+09:00 Krzysztof Kozlowski <krzk@kernel.org>:
> On Sun, May 21, 2017 at 02:42:38AM +0900, Masahiro Yamada wrote:
>> After long term efforts of fixing non-common clock implementations,
>> clk_disable() is a no-op for a NULL pointer input, and this is now
>> tree-wide consistent.
>>
>> All clock consumers can safely call clk_disable(_unprepare) without
>> NULL pointer check.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  sound/soc/codecs/adau17x1.c                | 3 +--
>>  sound/soc/codecs/da7213.c                  | 3 +--
>>  sound/soc/codecs/da7218.c                  | 3 +--
>>  sound/soc/codecs/da7219-aad.c              | 5 ++---
>>  sound/soc/codecs/da7219.c                  | 3 +--
>>  sound/soc/codecs/es8328.c                  | 3 +--
>>  sound/soc/codecs/wm8731.c                  | 3 +--
>>  sound/soc/davinci/davinci-evm.c            | 3 +--
>>  sound/soc/fsl/imx-audmux.c                 | 6 ++----
>>  sound/soc/intel/boards/bytcr_rt5640.c      | 2 +-
>>  sound/soc/intel/boards/cht_bsw_rt5645.c    | 3 +--
>>  sound/soc/mediatek/mt8173/mt8173-afe-pcm.c | 6 ++----
>>  sound/soc/samsung/i2s.c                    | 3 +--
>>  13 files changed, 16 insertions(+), 30 deletions(-)
>>
>
> (...)
>
>> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
>> index af3ba4d..3a06acd 100644
>> --- a/sound/soc/samsung/i2s.c
>> +++ b/sound/soc/samsung/i2s.c
>> @@ -1122,8 +1122,7 @@ static int i2s_runtime_suspend(struct device *dev)
>>       i2s->suspend_i2scon = readl(i2s->addr + I2SCON);
>>       i2s->suspend_i2spsr = readl(i2s->addr + I2SPSR);
>>
>> -     if (i2s->op_clk)
>> -             clk_disable_unprepare(i2s->op_clk);
>> +     clk_disable_unprepare(i2s->op_clk);
>>       clk_disable_unprepare(i2s->clk);
>
> I think the check in i2s_runtime_resume() should be removed then as well
> to keep it symmetrical. Otherwise it looks suspicious.


Hmm, you have a point.  We will lose the symmetry.

If samsung/i2s.c is only used with the common clock framework,
we can omit the NULL pointer check for clk_prepare_enable()
because it is also a no-op for NULL clk input
(it returns 0).

At this moment, this is not consistent for non-common clock implementations,
though.



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1646184

FromMasahiro Yamada <yamada.masahiro@socionext.com>
Date2017-05-20 21:00 +0200
Message-ID<tJhLz-59g-3@gated-at.bofh.it>
In reply to#1646183
2017-05-21 3:42 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> Hi Krzysztof,
>
>
> 2017-05-21 3:04 GMT+09:00 Krzysztof Kozlowski <krzk@kernel.org>:
>> On Sun, May 21, 2017 at 02:42:38AM +0900, Masahiro Yamada wrote:
>>> After long term efforts of fixing non-common clock implementations,
>>> clk_disable() is a no-op for a NULL pointer input, and this is now
>>> tree-wide consistent.
>>>
>>> All clock consumers can safely call clk_disable(_unprepare) without
>>> NULL pointer check.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  sound/soc/codecs/adau17x1.c                | 3 +--
>>>  sound/soc/codecs/da7213.c                  | 3 +--
>>>  sound/soc/codecs/da7218.c                  | 3 +--
>>>  sound/soc/codecs/da7219-aad.c              | 5 ++---
>>>  sound/soc/codecs/da7219.c                  | 3 +--
>>>  sound/soc/codecs/es8328.c                  | 3 +--
>>>  sound/soc/codecs/wm8731.c                  | 3 +--
>>>  sound/soc/davinci/davinci-evm.c            | 3 +--
>>>  sound/soc/fsl/imx-audmux.c                 | 6 ++----
>>>  sound/soc/intel/boards/bytcr_rt5640.c      | 2 +-
>>>  sound/soc/intel/boards/cht_bsw_rt5645.c    | 3 +--
>>>  sound/soc/mediatek/mt8173/mt8173-afe-pcm.c | 6 ++----
>>>  sound/soc/samsung/i2s.c                    | 3 +--
>>>  13 files changed, 16 insertions(+), 30 deletions(-)
>>>
>>
>> (...)
>>
>>> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
>>> index af3ba4d..3a06acd 100644
>>> --- a/sound/soc/samsung/i2s.c
>>> +++ b/sound/soc/samsung/i2s.c
>>> @@ -1122,8 +1122,7 @@ static int i2s_runtime_suspend(struct device *dev)
>>>       i2s->suspend_i2scon = readl(i2s->addr + I2SCON);
>>>       i2s->suspend_i2spsr = readl(i2s->addr + I2SPSR);
>>>
>>> -     if (i2s->op_clk)
>>> -             clk_disable_unprepare(i2s->op_clk);
>>> +     clk_disable_unprepare(i2s->op_clk);
>>>       clk_disable_unprepare(i2s->clk);
>>
>> I think the check in i2s_runtime_resume() should be removed then as well
>> to keep it symmetrical. Otherwise it looks suspicious.
>
>
> Hmm, you have a point.  We will lose the symmetry.
>
> If samsung/i2s.c is only used with the common clock framework,
> we can omit the NULL pointer check for clk_prepare_enable()
> because it is also a no-op for NULL clk input
> (it returns 0).
>
> At this moment, this is not consistent for non-common clock implementations,
> though.


I retract this patch.



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web