From: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
To: Xu Wang <vulab@iscas.ac.cn>,
Support Opensource <Support.Opensource@diasemi.com>,
"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
"broonie@kernel.org" <broonie@kernel.org>,
"perex@perex.cz" <perex@perex.cz>,
"tiwai@suse.com" <tiwai@suse.com>,
"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] ASoC: codecs: da*: Remove redundant null check before clk_prepare_enable/clk_disable_unprepare
Date: Fri, 8 Jan 2021 10:10:43 +0000 [thread overview]
Message-ID: <PR3PR10MB41424697CF78E4F06FF9154480AE0@PR3PR10MB4142.EURPRD10.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20210108085319.6912-1-vulab@iscas.ac.cn>
On 08 January 2021 08:53, Xu Wang wrote:
> Because clk_prepare_enable() and clk_disable_unprepare() already checked
> NULL clock parameter, so the additional checks are unnecessary, just
> remove them.
>
> Signed-off-by: Xu Wang <vulab@iscas.ac.cn>
> ---
> sound/soc/codecs/da7218.c | 13 +++++--------
> sound/soc/codecs/da7219-aad.c | 19 ++++++++-----------
> sound/soc/codecs/da7219.c | 3 +--
> 3 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
> index 2bfafbe9e3dc..324d95e3b48a 100644
> --- a/sound/soc/codecs/da7218.c
> +++ b/sound/soc/codecs/da7218.c
> @@ -2582,12 +2582,10 @@ static int da7218_set_bias_level(struct
> snd_soc_component *component,
> case SND_SOC_BIAS_PREPARE:
> /* Enable MCLK for transition to ON state */
> if (snd_soc_component_get_bias_level(component) ==
> SND_SOC_BIAS_STANDBY) {
> - if (da7218->mclk) {
> - ret = clk_prepare_enable(da7218->mclk);
> - if (ret) {
> - dev_err(component->dev, "Failed to
> enable mclk\n");
> - return ret;
> - }
> + ret = clk_prepare_enable(da7218->mclk);
> + if (ret) {
> + dev_err(component->dev, "Failed to enable
> mclk\n");
> + return ret;
> }
> }
>
> @@ -2605,8 +2603,7 @@ static int da7218_set_bias_level(struct
> snd_soc_component *component,
> 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 48081d71c22c..fa61cfc8e50c 100644
> --- a/sound/soc/codecs/da7219-aad.c
> +++ b/sound/soc/codecs/da7219-aad.c
> @@ -123,15 +123,13 @@ static void da7219_aad_hptest_work(struct work_struct
> *work)
> mutex_lock(&da7219->pll_lock);
>
> /* Ensure MCLK is available for HP test procedure */
> - if (da7219->mclk) {
> - ret = clk_prepare_enable(da7219->mclk);
> - if (ret) {
> - dev_err(component->dev, "Failed to enable mclk -
> %d\n", ret);
> - mutex_unlock(&da7219->pll_lock);
> - mutex_unlock(&da7219->ctrl_lock);
> - snd_soc_dapm_mutex_unlock(dapm);
> - return;
> - }
> + ret = clk_prepare_enable(da7219->mclk);
> + if (ret) {
> + dev_err(component->dev, "Failed to enable mclk - %d\n", ret);
> + mutex_unlock(&da7219->pll_lock);
> + mutex_unlock(&da7219->ctrl_lock);
> + snd_soc_dapm_mutex_unlock(dapm);
> + return;
> }
>
> /*
> @@ -318,8 +316,7 @@ static void da7219_aad_hptest_work(struct work_struct
> *work)
> da7219_set_pll(component, DA7219_SYSCLK_MCLK, 0);
>
> /* Remove MCLK, if previously enabled */
> - if (da7219->mclk)
> - clk_disable_unprepare(da7219->mclk);
> + clk_disable_unprepare(da7219->mclk);
>
> mutex_unlock(&da7219->pll_lock);
> mutex_unlock(&da7219->ctrl_lock);
> diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
> index e9b45daec0ca..50307cc9d640 100644
> --- a/sound/soc/codecs/da7219.c
> +++ b/sound/soc/codecs/da7219.c
> @@ -1826,8 +1826,7 @@ static int da7219_set_bias_level(struct
> snd_soc_component *component,
>
> if (snd_soc_component_get_bias_level(component) ==
> SND_SOC_BIAS_PREPARE) {
> /* Remove MCLK */
> - if (da7219->mclk)
> - clk_disable_unprepare(da7219->mclk);
> + clk_disable_unprepare(da7219->mclk);
You seem to have missed the change for clk_prepare_enable() usage in the
same function. Also as the above is now a single line if statement, you can
remove the brackets.
In addition, the normal approach would be to split the changes into separate
patches per driver, just so it's cleaner.
> }
> break;
> case SND_SOC_BIAS_OFF:
> --
> 2.17.1
prev parent reply other threads:[~2021-01-08 10:11 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-08 8:53 [PATCH] ASoC: codecs: da*: Remove redundant null check before clk_prepare_enable/clk_disable_unprepare Xu Wang
2021-01-08 10:10 ` Adam Thomson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=PR3PR10MB41424697CF78E4F06FF9154480AE0@PR3PR10MB4142.EURPRD10.PROD.OUTLOOK.COM \
--to=adam.thomson.opensource@diasemi.com \
--cc=Support.Opensource@diasemi.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
--cc=vulab@iscas.ac.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.