[PATCH v2 0/4] ASoC: Intel: bytcr_rt5640: few cleanups
The small set of cleanups against bytcr_rt5640 board file.
In v2: - added commit message to patch 2 (Joe, Pierre) - added cover letter (Pierre) - added Hans to Cc list (Hans)
Andy Shevchenko (4): ASoC: Intel: bytcr_rt5640: Get platform data via dev_get_platdata() ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device ASoC: Intel: bytcr_rt5640: use devm_clk_get_optional() for mclk ASoC: Intel: bytcr_rt5640: Utilize dev_err_probe() to avoid log saturation
sound/soc/intel/boards/bytcr_rt5640.c | 116 +++++++++++--------------- 1 file changed, 49 insertions(+), 67 deletions(-)
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 */
Use temporary variable for struct device to make code neater.
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);
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) {
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; } /*
Hi,
On 10/7/21 6:57 PM, Andy Shevchenko wrote:
The small set of cleanups against bytcr_rt5640 board file.
In v2:
- added commit message to patch 2 (Joe, Pierre)
- added cover letter (Pierre)
- added Hans to Cc list (Hans)
Andy Shevchenko (4): ASoC: Intel: bytcr_rt5640: Get platform data via dev_get_platdata() ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device ASoC: Intel: bytcr_rt5640: use devm_clk_get_optional() for mclk ASoC: Intel: bytcr_rt5640: Utilize dev_err_probe() to avoid log saturation
Thanks, the entire series good to me, this one I've also given a quick test-spin, so:
Reviewed-by: Hans de Goede hdegoede@redhat.com Tested-by: Hans de Goede hdegoede@redhat.com
For the series.
Regards,
Hans
Andy Shevchenko (4): ASoC: Intel: bytcr_rt5640: Get platform data via dev_get_platdata() ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device ASoC: Intel: bytcr_rt5640: use devm_clk_get_optional() for mclk ASoC: Intel: bytcr_rt5640: Utilize dev_err_probe() to avoid log saturation
Thanks, the entire series good to me, this one I've also given a quick test-spin, so:
Reviewed-by: Hans de Goede hdegoede@redhat.com Tested-by: Hans de Goede hdegoede@redhat.com
For the series.
Acked-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
On Thu, 7 Oct 2021 19:57:11 +0300, Andy Shevchenko wrote:
The small set of cleanups against bytcr_rt5640 board file.
In v2:
- added commit message to patch 2 (Joe, Pierre)
- added cover letter (Pierre)
- added Hans to Cc list (Hans)
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: Intel: bytcr_rt5640: Get platform data via dev_get_platdata() commit: e86c1893d6785a0f5e5d82cd161b991564720eaa [2/4] ASoC: Intel: bytcr_rt5640: Use temporary variable for struct device commit: 81d43ca17506ba32c6ead7fe4cf3b7f37368cc83 [3/4] ASoC: Intel: bytcr_rt5640: use devm_clk_get_optional() for mclk commit: a15ca6e3b8a21ff335a2eedbc5ba4708967be2be [4/4] ASoC: Intel: bytcr_rt5640: Utilize dev_err_probe() to avoid log saturation commit: ee233500eea421118cd9d53c82fd5e612f6d7bd5
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
participants (4)
-
Andy Shevchenko -
Hans de Goede -
Mark Brown -
Pierre-Louis Bossart