* [alsa-devel] [PATCH] ASoC: Intel: cht_bsw_rt5645: Add quirk for boards using pmc_plt_clk_0
@ 2019-09-16 7:15 Sam McNally
2019-09-16 15:02 ` Pierre-Louis Bossart
0 siblings, 1 reply; 3+ messages in thread
From: Sam McNally @ 2019-09-16 7:15 UTC (permalink / raw)
To: alsa-devel; +Cc: Sam McNally, Pierre-Louis Bossart
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.
Signed-off-by: Sam McNally <sammc@chromium.org>
---
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";
+
+ 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);
}
--
2.23.0.237.gc6a4ce50a0-goog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: Intel: cht_bsw_rt5645: Add quirk for boards using pmc_plt_clk_0
2019-09-16 7:15 [alsa-devel] [PATCH] ASoC: Intel: cht_bsw_rt5645: Add quirk for boards using pmc_plt_clk_0 Sam McNally
@ 2019-09-16 15:02 ` Pierre-Louis Bossart
2019-09-17 5:46 ` Sam McNally
0 siblings, 1 reply; 3+ messages in thread
From: Pierre-Louis Bossart @ 2019-09-16 15:02 UTC (permalink / raw)
To: Sam McNally, alsa-devel; +Cc: Takashi Iwai, Mark Brown
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@chromium.org>
> ---
>
> 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@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: Intel: cht_bsw_rt5645: Add quirk for boards using pmc_plt_clk_0
2019-09-16 15:02 ` Pierre-Louis Bossart
@ 2019-09-17 5:46 ` Sam McNally
0 siblings, 0 replies; 3+ messages in thread
From: Sam McNally @ 2019-09-17 5:46 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: Takashi Iwai, alsa-devel, Mark Brown
On Tue, 17 Sep 2019 at 01:02, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> 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?
>
Will do.
> Also see more important comment below
>
> >
> > Signed-off-by: Sam McNally <sammc@chromium.org>
> > ---
> >
> > 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.
>
The existing dmi_check_system() call at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/intel/boards/cht_bsw_rt5645.c?h=for-5.4&id=fca11622d600228bec405456f41590155b3a3eca#n630
uses the quirks table, calling the previously-commented-out callback
to assign to the quirks global. I'll add a mention in the description.
> > + 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@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-17 5:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-16 7:15 [alsa-devel] [PATCH] ASoC: Intel: cht_bsw_rt5645: Add quirk for boards using pmc_plt_clk_0 Sam McNally
2019-09-16 15:02 ` Pierre-Louis Bossart
2019-09-17 5:46 ` Sam McNally
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.