[PATCH v1 1/4] ASoC: Intel: bytcr_rt5640: Get platform data via dev_get_platdata()
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 */
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);
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...];
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...?
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.
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?
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.
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.
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) {
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;@@ -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);elseret = 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");@@ -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) {
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.
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; } /*
participants (4)
-
Andy Shevchenko -
Joe Perches -
Mark Brown -
Pierre-Louis Bossart