[PATCH v1 1/4] ASoC: Intel: bytcr_rt5640: Get platform data via dev_get_platdata()

[PATCH v1 1/4] ASoC: Intel: bytcr_rt5640: Get platform data via dev_get_platdata()

[Andy Shevchenko]

Access to platform data via dev_get_platdata() getter to make code cleaner.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index c28abe74816f..43997048a825 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -1495,12 +1495,12 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	static const char * const map_name[] = { "dmic1", "dmic2", "in1", "in3", "none" };
+	struct snd_soc_acpi_mach *mach = dev_get_platdata(dev);
 	__maybe_unused const char *spk_type;
 	const struct dmi_system_id *dmi_id;
 	const char *headset2_string = "";
 	const char *lineout_string = "";
 	struct byt_rt5640_private *priv;
-	struct snd_soc_acpi_mach *mach;
 	const char *platform_name;
 	struct acpi_device *adev;
 	struct device *codec_dev;
@@ -1517,7 +1517,6 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 
 	/* register the soc card */
 	byt_rt5640_card.dev = &pdev->dev;
-	mach = byt_rt5640_card.dev->platform_data;
 	snd_soc_card_set_drvdata(&byt_rt5640_card, priv);
 
 	/* fix index of codec dai */
-- 
2.33.0

[PATCH v1 2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device

[Andy Shevchenko]

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

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 43997048a825..0f609a0b3527 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -1511,12 +1511,12 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	int aif;
 
 	is_bytcr = false;
-	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	/* register the soc card */
-	byt_rt5640_card.dev = &pdev->dev;
+	byt_rt5640_card.dev = dev;
 	snd_soc_card_set_drvdata(&byt_rt5640_card, priv);
 
 	/* fix index of codec dai */
@@ -1536,7 +1536,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		put_device(&adev->dev);
 		byt_rt5640_dais[dai_index].codecs->name = byt_rt5640_codec_name;
 	} else {
-		dev_err(&pdev->dev, "Error cannot find '%s' dev\n", mach->id);
+		dev_err(dev, "Error cannot find '%s' dev\n", mach->id);
 		return -ENXIO;
 	}
 
@@ -1579,13 +1579,13 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 							       &pkg_ctx);
 		if (pkg_found) {
 			if (chan_package.aif_value == 1) {
-				dev_info(&pdev->dev, "BIOS Routing: AIF1 connected\n");
+				dev_info(dev, "BIOS Routing: AIF1 connected\n");
 				byt_rt5640_quirk |= BYT_RT5640_SSP0_AIF1;
 			} else  if (chan_package.aif_value == 2) {
-				dev_info(&pdev->dev, "BIOS Routing: AIF2 connected\n");
+				dev_info(dev, "BIOS Routing: AIF2 connected\n");
 				byt_rt5640_quirk |= BYT_RT5640_SSP0_AIF2;
 			} else {
-				dev_info(&pdev->dev, "BIOS Routing isn't valid, ignored\n");
+				dev_info(dev, "BIOS Routing isn't valid, ignored\n");
 				pkg_found = false;
 			}
 		}
@@ -1609,7 +1609,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	if (dmi_id)
 		byt_rt5640_quirk = (unsigned long)dmi_id->driver_data;
 	if (quirk_override != -1) {
-		dev_info(&pdev->dev, "Overriding quirk 0x%lx => 0x%x\n",
+		dev_info(dev, "Overriding quirk 0x%lx => 0x%x\n",
 			 byt_rt5640_quirk, quirk_override);
 		byt_rt5640_quirk = quirk_override;
 	}
@@ -1623,12 +1623,12 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		acpi_dev_add_driver_gpios(ACPI_COMPANION(priv->codec_dev),
 					  byt_rt5640_hp_elitepad_1000g2_gpios);
 
-		priv->hsmic_detect = devm_fwnode_gpiod_get(&pdev->dev, codec_dev->fwnode,
+		priv->hsmic_detect = devm_fwnode_gpiod_get(dev, codec_dev->fwnode,
 							   "headset-mic-detect", GPIOD_IN,
 							   "headset-mic-detect");
 		if (IS_ERR(priv->hsmic_detect)) {
 			ret_val = PTR_ERR(priv->hsmic_detect);
-			dev_err_probe(&pdev->dev, ret_val, "getting hsmic-detect GPIO\n");
+			dev_err_probe(dev, ret_val, "getting hsmic-detect GPIO\n");
 			goto err_device;
 		}
 	}
@@ -1638,7 +1638,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	if (ret_val)
 		goto err_remove_gpios;
 
-	log_quirks(&pdev->dev);
+	log_quirks(dev);
 
 	if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) ||
 	    (byt_rt5640_quirk & BYT_RT5640_SSP0_AIF2)) {
@@ -1653,11 +1653,11 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		byt_rt5640_dais[dai_index].cpus->dai_name = "ssp0-port";
 
 	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
-		priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
+		priv->mclk = devm_clk_get(dev, "pmc_plt_clk_3");
 		if (IS_ERR(priv->mclk)) {
 			ret_val = PTR_ERR(priv->mclk);
 
-			dev_err(&pdev->dev,
+			dev_err(dev,
 				"Failed to get MCLK from pmc_plt_clk_3: %d\n",
 				ret_val);
 
@@ -1713,7 +1713,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	if (ret_val)
 		goto err;
 
-	sof_parent = snd_soc_acpi_sof_parent(&pdev->dev);
+	sof_parent = snd_soc_acpi_sof_parent(dev);
 
 	/* set card and driver name */
 	if (sof_parent) {
@@ -1728,11 +1728,9 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	if (sof_parent)
 		dev->driver->pm = &snd_soc_pm_ops;
 
-	ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5640_card);
-
+	ret_val = devm_snd_soc_register_card(dev, &byt_rt5640_card);
 	if (ret_val) {
-		dev_err(&pdev->dev, "devm_snd_soc_register_card failed %d\n",
-			ret_val);
+		dev_err(dev, "devm_snd_soc_register_card failed %d\n", ret_val);
 		goto err;
 	}
 	platform_set_drvdata(pdev, &byt_rt5640_card);
-- 
2.33.0

Re: [PATCH v1 2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device

[Joe Perches]

On Wed, 2021-10-06 at 18:04 +0300, Andy Shevchenko wrote:
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

trivia:

Some will complain about a lack of commit message.

> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
[]
> @@ -1536,7 +1536,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
>  		put_device(&adev->dev);
>  		byt_rt5640_dais[dai_index].codecs->name = byt_rt5640_codec_name;
>  	} else {
> -		dev_err(&pdev->dev, "Error cannot find '%s' dev\n", mach->id);
> +		dev_err(dev, "Error cannot find '%s' dev\n", mach->id);
>  		return -ENXIO;
>  	}

And code that does

	if (foo) {
		[code...]
	} else {
		log_msg();
		return -ERR;
	}

should generally have its test reversed and use an unindented block.

	if (!foo) {
		log_msg();
		return -ERR;
	}
	[code...];

Re: [PATCH v1 2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device

[Andy Shevchenko]

On Wed, Oct 06, 2021 at 08:21:01AM -0700, Joe Perches wrote:
> On Wed, 2021-10-06 at 18:04 +0300, Andy Shevchenko wrote:
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> trivia:
> 
> Some will complain about a lack of commit message.

Yeah, sorry for that, it wasn't deliberate. I forgot to run `git msg-filter`
on these three patches to add it.

Mark, do you want me resend entire bunch(es) or just starting from these
patches? Or...?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device

[Mark Brown]

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

On Wed, Oct 06, 2021 at 06:44:07PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 06, 2021 at 08:21:01AM -0700, Joe Perches wrote:

> > Some will complain about a lack of commit message.

> Yeah, sorry for that, it wasn't deliberate. I forgot to run `git msg-filter`
> on these three patches to add it.

> Mark, do you want me resend entire bunch(es) or just starting from these
> patches? Or...?

If you're adding a commit message with automation it's probably not
adding any value.

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

Re: [PATCH v1 2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device

[Andy Shevchenko]

On Wed, Oct 06, 2021 at 04:48:57PM +0100, Mark Brown wrote:
> On Wed, Oct 06, 2021 at 06:44:07PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 06, 2021 at 08:21:01AM -0700, Joe Perches wrote:
> 
> > > Some will complain about a lack of commit message.
> 
> > Yeah, sorry for that, it wasn't deliberate. I forgot to run `git msg-filter`
> > on these three patches to add it.
> 
> > Mark, do you want me resend entire bunch(es) or just starting from these
> > patches? Or...?
> 
> If you're adding a commit message with automation it's probably not
> adding any value.

What do you mean? I add it exceptionally for the same (by nature) patches.
What do you expect to be altered in these three, if the idea behind the change
is very well the same?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device

[Mark Brown]

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

On Wed, Oct 06, 2021 at 07:05:47PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 06, 2021 at 04:48:57PM +0100, Mark Brown wrote:
> > On Wed, Oct 06, 2021 at 06:44:07PM +0300, Andy Shevchenko wrote:
> > > On Wed, Oct 06, 2021 at 08:21:01AM -0700, Joe Perches wrote:

> > > > Some will complain about a lack of commit message.

> > > Yeah, sorry for that, it wasn't deliberate. I forgot to run `git msg-filter`
> > > on these three patches to add it.

> > > Mark, do you want me resend entire bunch(es) or just starting from these
> > > patches? Or...?

> > If you're adding a commit message with automation it's probably not
> > adding any value.

> What do you mean? I add it exceptionally for the same (by nature) patches.
> What do you expect to be altered in these three, if the idea behind the change
> is very well the same?

I really don't care if there's a separate changelog for trivial patches
like this, it adds nothing of value.

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

Re: [PATCH v1 2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device

[Andy Shevchenko]

On Wed, Oct 06, 2021 at 05:20:04PM +0100, Mark Brown wrote:
> On Wed, Oct 06, 2021 at 07:05:47PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 06, 2021 at 04:48:57PM +0100, Mark Brown wrote:
> > > On Wed, Oct 06, 2021 at 06:44:07PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Oct 06, 2021 at 08:21:01AM -0700, Joe Perches wrote:
> 
> > > > > Some will complain about a lack of commit message.
> 
> > > > Yeah, sorry for that, it wasn't deliberate. I forgot to run `git msg-filter`
> > > > on these three patches to add it.
> 
> > > > Mark, do you want me resend entire bunch(es) or just starting from these
> > > > patches? Or...?
> 
> > > If you're adding a commit message with automation it's probably not
> > > adding any value.
> 
> > What do you mean? I add it exceptionally for the same (by nature) patches.
> > What do you expect to be altered in these three, if the idea behind the change
> > is very well the same?
> 
> I really don't care if there's a separate changelog for trivial patches
> like this, it adds nothing of value.

I see. In any case I'll add something meaningful here.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 3/4] ASoC: Intel: bytcr_rt5640: use devm_clk_get_optional() for mclk

[Andy Shevchenko]

The devm_clk_get_optional() helper returns NULL when devm_clk_get()
returns -ENOENT. This makes things slightly cleaner. The added benefit
is mostly cosmetic.

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

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 0f609a0b3527..2e7d45f0f05d 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -269,13 +269,10 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 		return -EIO;
 
 	if (SND_SOC_DAPM_EVENT_ON(event)) {
-		if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
-			ret = clk_prepare_enable(priv->mclk);
-			if (ret < 0) {
-				dev_err(card->dev,
-					"could not configure MCLK state\n");
-				return ret;
-			}
+		ret = clk_prepare_enable(priv->mclk);
+		if (ret < 0) {
+			dev_err(card->dev, "could not configure MCLK state\n");
+			return ret;
 		}
 		ret = byt_rt5640_prepare_and_enable_pll1(codec_dai, 48000);
 	} else {
@@ -287,10 +284,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
 		ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_RCCLK,
 					     48000 * 512,
 					     SND_SOC_CLOCK_IN);
-		if (!ret) {
-			if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN)
-				clk_disable_unprepare(priv->mclk);
-		}
+		if (!ret)
+			clk_disable_unprepare(priv->mclk);
 	}
 
 	if (ret < 0) {
@@ -1217,30 +1212,25 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 			return ret;
 	}
 
-	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
-		/*
-		 * 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(priv->mclk);
-		if (!ret)
-			clk_disable_unprepare(priv->mclk);
-
-		if (byt_rt5640_quirk & BYT_RT5640_MCLK_25MHZ)
-			ret = clk_set_rate(priv->mclk, 25000000);
-		else
-			ret = clk_set_rate(priv->mclk, 19200000);
+	/*
+	 * 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(priv->mclk);
+	if (!ret)
+		clk_disable_unprepare(priv->mclk);
 
-		if (ret) {
-			dev_err(card->dev, "unable to set MCLK rate\n");
-			return ret;
-		}
+	if (byt_rt5640_quirk & BYT_RT5640_MCLK_25MHZ)
+		ret = clk_set_rate(priv->mclk, 25000000);
+	else
+		ret = clk_set_rate(priv->mclk, 19200000);
+	if (ret) {
+		dev_err(card->dev, "unable to set MCLK rate\n");
+		return ret;
 	}
 
 	if (BYT_RT5640_JDSRC(byt_rt5640_quirk)) {
@@ -1653,7 +1643,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		byt_rt5640_dais[dai_index].cpus->dai_name = "ssp0-port";
 
 	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
-		priv->mclk = devm_clk_get(dev, "pmc_plt_clk_3");
+		priv->mclk = devm_clk_get_optional(dev, "pmc_plt_clk_3");
 		if (IS_ERR(priv->mclk)) {
 			ret_val = PTR_ERR(priv->mclk);
 
@@ -1661,15 +1651,14 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 				"Failed to get MCLK from pmc_plt_clk_3: %d\n",
 				ret_val);
 
-			/*
-			 * Fall back to bit clock usage for -ENOENT (clock not
-			 * available likely due to missing dependencies), bail
-			 * for all other errors, including -EPROBE_DEFER
-			 */
-			if (ret_val != -ENOENT)
-				goto err;
-			byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN;
+			goto err;
 		}
+		/*
+		 * Fall back to bit clock usage when clock is not
+		 * available likely due to missing dependencies.
+		 */
+		if (!priv->mclk)
+			byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN;
 	}
 
 	if (byt_rt5640_quirk & BYT_RT5640_NO_SPEAKERS) {
-- 
2.33.0

Re: [PATCH v1 3/4] ASoC: Intel: bytcr_rt5640: use devm_clk_get_optional() for mclk

[Pierre-Louis Bossart]

On 10/6/21 10:04 AM, Andy Shevchenko wrote:
> The devm_clk_get_optional() helper returns NULL when devm_clk_get()
> returns -ENOENT. This makes things slightly cleaner. The added benefit
> is mostly cosmetic.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  sound/soc/intel/boards/bytcr_rt5640.c | 75 ++++++++++++---------------
>  1 file changed, 32 insertions(+), 43 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
> index 0f609a0b3527..2e7d45f0f05d 100644
> --- a/sound/soc/intel/boards/bytcr_rt5640.c
> +++ b/sound/soc/intel/boards/bytcr_rt5640.c
> @@ -269,13 +269,10 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
>  		return -EIO;
>  
>  	if (SND_SOC_DAPM_EVENT_ON(event)) {
> -		if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {

same comment as for rt5651, I don't see the point of removing the test
on this quirk?

> -			ret = clk_prepare_enable(priv->mclk);
> -			if (ret < 0) {
> -				dev_err(card->dev,
> -					"could not configure MCLK state\n");
> -				return ret;
> -			}
> +		ret = clk_prepare_enable(priv->mclk);
> +		if (ret < 0) {
> +			dev_err(card->dev, "could not configure MCLK state\n");
> +			return ret;
>  		}
>  		ret = byt_rt5640_prepare_and_enable_pll1(codec_dai, 48000);
>  	} else {
> @@ -287,10 +284,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
>  		ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_RCCLK,
>  					     48000 * 512,
>  					     SND_SOC_CLOCK_IN);
> -		if (!ret) {
> -			if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN)
> -				clk_disable_unprepare(priv->mclk);
> -		}
> +		if (!ret)
> +			clk_disable_unprepare(priv->mclk);
>  	}
>  
>  	if (ret < 0) {
> @@ -1217,30 +1212,25 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
>  			return ret;
>  	}
>  
> -	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
> -		/*
> -		 * 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(priv->mclk);
> -		if (!ret)
> -			clk_disable_unprepare(priv->mclk);
> -
> -		if (byt_rt5640_quirk & BYT_RT5640_MCLK_25MHZ)
> -			ret = clk_set_rate(priv->mclk, 25000000);
> -		else
> -			ret = clk_set_rate(priv->mclk, 19200000);
> +	/*
> +	 * 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(priv->mclk);
> +	if (!ret)
> +		clk_disable_unprepare(priv->mclk);
>  
> -		if (ret) {
> -			dev_err(card->dev, "unable to set MCLK rate\n");
> -			return ret;
> -		}
> +	if (byt_rt5640_quirk & BYT_RT5640_MCLK_25MHZ)
> +		ret = clk_set_rate(priv->mclk, 25000000);
> +	else
> +		ret = clk_set_rate(priv->mclk, 19200000);
> +	if (ret) {
> +		dev_err(card->dev, "unable to set MCLK rate\n");
> +		return ret;
>  	}
>  
>  	if (BYT_RT5640_JDSRC(byt_rt5640_quirk)) {
> @@ -1653,7 +1643,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
>  		byt_rt5640_dais[dai_index].cpus->dai_name = "ssp0-port";
>  
>  	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
> -		priv->mclk = devm_clk_get(dev, "pmc_plt_clk_3");
> +		priv->mclk = devm_clk_get_optional(dev, "pmc_plt_clk_3");
>  		if (IS_ERR(priv->mclk)) {
>  			ret_val = PTR_ERR(priv->mclk);
>  
> @@ -1661,15 +1651,14 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
>  				"Failed to get MCLK from pmc_plt_clk_3: %d\n",
>  				ret_val);
>  
> -			/*
> -			 * Fall back to bit clock usage for -ENOENT (clock not
> -			 * available likely due to missing dependencies), bail
> -			 * for all other errors, including -EPROBE_DEFER
> -			 */
> -			if (ret_val != -ENOENT)
> -				goto err;
> -			byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN;
> +			goto err;
>  		}
> +		/*
> +		 * Fall back to bit clock usage when clock is not
> +		 * available likely due to missing dependencies.
> +		 */
> +		if (!priv->mclk)
> +			byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN;
>  	}
>  
>  	if (byt_rt5640_quirk & BYT_RT5640_NO_SPEAKERS) {
> 

Re: [PATCH v1 3/4] ASoC: Intel: bytcr_rt5640: use devm_clk_get_optional() for mclk

[Andy Shevchenko]

On Wed, Oct 06, 2021 at 10:54:12AM -0500, Pierre-Louis Bossart wrote:
> On 10/6/21 10:04 AM, Andy Shevchenko wrote:
> > The devm_clk_get_optional() helper returns NULL when devm_clk_get()
> > returns -ENOENT. This makes things slightly cleaner. The added benefit
> > is mostly cosmetic.

...

> >  	if (SND_SOC_DAPM_EVENT_ON(event)) {
> > -		if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
> 
> same comment as for rt5651, I don't see the point of removing the test
> on this quirk?

Same answers.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 4/4] ASoC: Intel: bytcr_rt5640: Utilize dev_err_probe() to avoid log saturation

[Andy Shevchenko]

dev_err_probe() avoids printing into log when the deferred probe is invoked.
This is possible when clock provider is pending to appear.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 2e7d45f0f05d..a0c5f0e9c22a 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -1617,8 +1617,8 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 							   "headset-mic-detect", GPIOD_IN,
 							   "headset-mic-detect");
 		if (IS_ERR(priv->hsmic_detect)) {
-			ret_val = PTR_ERR(priv->hsmic_detect);
-			dev_err_probe(dev, ret_val, "getting hsmic-detect GPIO\n");
+			ret_val = dev_err_probe(dev, PTR_ERR(priv->hsmic_detect),
+						"getting hsmic-detect GPIO\n");
 			goto err_device;
 		}
 	}
@@ -1645,12 +1645,8 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) {
 		priv->mclk = devm_clk_get_optional(dev, "pmc_plt_clk_3");
 		if (IS_ERR(priv->mclk)) {
-			ret_val = PTR_ERR(priv->mclk);
-
-			dev_err(dev,
-				"Failed to get MCLK from pmc_plt_clk_3: %d\n",
-				ret_val);
-
+			ret_val = dev_err_probe(dev, PTR_ERR(priv->mclk),
+						"Failed to get MCLK from pmc_plt_clk_3\n");
 			goto err;
 		}
 		/*
-- 
2.33.0