Re: [PATCH] ASoC: Intel: cht_bsw_rt5645: Add quirk for boards using pmc_plt_clk_0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/16/19 2:15 AM, Sam McNally wrote:
As of commit 648e921888ad ("clk: x86: Stop marking clocks as
CLK_IS_CRITICAL"), the cht_bsw_rt5645 driver needs to enable the clock
it's using for the codec's mclk. It does this from commit 7735bce05a9c
("ASoC: Intel: boards: use devm_clk_get() unconditionally"), enabling
pmc_plt_clk_3. However, Strago family Chromebooks use pmc_plt_clk_0 for
the codec mclk, resulting in white noise with some digital microphones.
Add a DMI-based quirk for Strago family Chromebooks to use pmc_plt_clk_0
instead.

Sounds good, thanks for the patch. You will need to Cc: maintainers (Takashi and Mark) if you want them to see your patches.

Maybe you should mention in the commit message that this mirrors the changes made in cht_bsw_max98090_ti?

Also see more important comment below


Signed-off-by: Sam McNally <sammc@xxxxxxxxxxxx>
---

  sound/soc/intel/boards/cht_bsw_rt5645.c | 26 +++++++++++++++++++------
  1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index 8879c3be29d5..c68a5b85a4a0 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -48,6 +48,7 @@ struct cht_mc_private {
  #define CHT_RT5645_SSP2_AIF2     BIT(16) /* default is using AIF1  */
  #define CHT_RT5645_SSP0_AIF1     BIT(17)
  #define CHT_RT5645_SSP0_AIF2     BIT(18)
+#define CHT_RT5645_PMC_PLT_CLK_0 BIT(19)
static unsigned long cht_rt5645_quirk = 0; @@ -59,6 +60,8 @@ static void log_quirks(struct device *dev)
  		dev_info(dev, "quirk SSP0_AIF1 enabled");
  	if (cht_rt5645_quirk & CHT_RT5645_SSP0_AIF2)
  		dev_info(dev, "quirk SSP0_AIF2 enabled");
+	if (cht_rt5645_quirk & CHT_RT5645_PMC_PLT_CLK_0)
+		dev_info(dev, "quirk PMC_PLT_CLK_0 enabled");
  }
static int platform_clock_control(struct snd_soc_dapm_widget *w,
@@ -226,15 +229,21 @@ static int cht_aif1_hw_params(struct snd_pcm_substream *substream,
  	return 0;
  }
-/* uncomment when we have a real quirk
  static int cht_rt5645_quirk_cb(const struct dmi_system_id *id)
  {
  	cht_rt5645_quirk = (unsigned long)id->driver_data;
  	return 1;
  }
-*/
static const struct dmi_system_id cht_rt5645_quirk_table[] = {
+	{
+		/* Strago family Chromebooks */
+		.callback = cht_rt5645_quirk_cb,
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_FAMILY, "Intel_Strago"),
+		},
+		.driver_data = (void *)CHT_RT5645_PMC_PLT_CLK_0,
+	},
  	{
  	},
  };
@@ -526,6 +535,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
  	int dai_index = 0;
  	int ret_val = 0;
  	int i;
+	const char *mclk_name;
drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
  	if (!drv)
@@ -662,11 +672,15 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
  	if (ret_val)
  		return ret_val;
- drv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
+	if (cht_rt5645_quirk & CHT_RT5645_PMC_PLT_CLK_0)
+		mclk_name = "pmc_plt_clk_0";
+	else
+		mclk_name = "pmc_plt_clk_3";

Aren't you missing a call to dmi_first_match() to change the default value of this cht_rt5645_quirk variable?

The rest of the patch looks good but I don't see how the DMI info is actually used.

+	drv->mclk = devm_clk_get(&pdev->dev, mclk_name);
  	if (IS_ERR(drv->mclk)) {
-		dev_err(&pdev->dev,
-			"Failed to get MCLK from pmc_plt_clk_3: %ld\n",
-			PTR_ERR(drv->mclk));
+		dev_err(&pdev->dev, "Failed to get MCLK from %s: %ld\n",
+			mclk_name, PTR_ERR(drv->mclk)); >   		return PTR_ERR(drv->mclk);
  	}

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux