2021-03-15 15:46:36

by Vijendar Mukunda

[permalink] [raw]
Subject: [PATCH v3] ASoC: amd: add support for rt5682 codec in machine driver

Add support for RT5682 codec in machine driver.

Signed-off-by: Vijendar Mukunda <[email protected]>
---
v2->v3: updated SPDX license
v1->v2: updated kconfig, spdx license, removed unnecessary
initialization and updated comment
sound/soc/amd/Kconfig | 5 +-
sound/soc/amd/acp-da7219-max98357a.c | 380 +++++++++++++++++++++++++++++++----
2 files changed, 343 insertions(+), 42 deletions(-)

diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig
index a6ce000..43f5d29 100644
--- a/sound/soc/amd/Kconfig
+++ b/sound/soc/amd/Kconfig
@@ -5,14 +5,15 @@ config SND_SOC_AMD_ACP
This option enables ACP DMA support on AMD platform.

config SND_SOC_AMD_CZ_DA7219MX98357_MACH
- tristate "AMD CZ support for DA7219 and MAX9835"
+ tristate "AMD CZ support for DA7219, RT5682 and MAX9835"
select SND_SOC_DA7219
+ select SND_SOC_RT5682_I2C
select SND_SOC_MAX98357A
select SND_SOC_ADAU7002
select REGULATOR
depends on SND_SOC_AMD_ACP && I2C && GPIOLIB
help
- This option enables machine driver for DA7219 and MAX9835.
+ This option enables machine driver for DA7219, RT5682 and MAX9835.

config SND_SOC_AMD_CZ_RT5645_MACH
tristate "AMD CZ support for RT5645"
diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index 849288d..fb7aa74 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -1,27 +1,8 @@
-/*
- * Machine driver for AMD ACP Audio engine using DA7219 & MAX98357 codec
- *
- * Copyright 2017 Advanced Micro Devices, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- *
- */
+// SPDX-License-Identifier: MIT
+//
+// Machine driver for AMD ACP Audio engine using DA7219, RT5682 & MAX98357 codec
+//
+//Copyright 2017-2021 Advanced Micro Devices, Inc.

#include <sound/core.h>
#include <sound/soc.h>
@@ -41,14 +22,19 @@
#include "acp.h"
#include "../codecs/da7219.h"
#include "../codecs/da7219-aad.h"
+#include "../codecs/rt5682.h"

#define CZ_PLAT_CLK 48000000
#define DUAL_CHANNEL 2
+#define RT5682_PLL_FREQ (48000 * 512)

static struct snd_soc_jack cz_jack;
static struct clk *da7219_dai_wclk;
static struct clk *da7219_dai_bclk;
-extern bool bt_uart_enable;
+static struct clk *rt5682_dai_wclk;
+static struct clk *rt5682_dai_bclk;
+extern int bt_uart_enable;
+void *soc_is_rltk_max(struct device *dev);

static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
{
@@ -128,6 +114,88 @@ static void da7219_clk_disable(void)
clk_disable_unprepare(da7219_dai_bclk);
}

+static int cz_rt5682_init(struct snd_soc_pcm_runtime *rtd)
+{
+ int ret;
+ struct snd_soc_card *card = rtd->card;
+ struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+ struct snd_soc_component *component = codec_dai->component;
+
+ dev_info(codec_dai->dev, "codec dai name = %s\n", codec_dai->name);
+
+ /* Set codec sysclk */
+ ret = snd_soc_dai_set_sysclk(codec_dai, RT5682_SCLK_S_PLL2,
+ RT5682_PLL_FREQ, SND_SOC_CLOCK_IN);
+ if (ret < 0) {
+ dev_err(codec_dai->dev,
+ "Failed to set rt5682 SYSCLK: %d\n", ret);
+ return ret;
+ }
+ /* set codec PLL */
+ ret = snd_soc_dai_set_pll(codec_dai, RT5682_PLL2, RT5682_PLL2_S_MCLK,
+ CZ_PLAT_CLK, RT5682_PLL_FREQ);
+ if (ret < 0) {
+ dev_err(codec_dai->dev, "can't set rt5682 PLL: %d\n", ret);
+ return ret;
+ }
+
+ rt5682_dai_wclk = devm_clk_get(component->dev, "rt5682-dai-wclk");
+ if (IS_ERR(rt5682_dai_wclk))
+ return PTR_ERR(rt5682_dai_wclk);
+
+ rt5682_dai_bclk = devm_clk_get(component->dev, "rt5682-dai-bclk");
+ if (IS_ERR(rt5682_dai_bclk))
+ return PTR_ERR(rt5682_dai_bclk);
+
+ ret = snd_soc_card_jack_new(card, "Headset Jack",
+ SND_JACK_HEADSET | SND_JACK_LINEOUT |
+ SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+ SND_JACK_BTN_2 | SND_JACK_BTN_3,
+ &cz_jack, NULL, 0);
+ if (ret) {
+ dev_err(card->dev, "HP jack creation failed %d\n", ret);
+ return ret;
+ }
+
+ snd_jack_set_key(cz_jack.jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
+ snd_jack_set_key(cz_jack.jack, SND_JACK_BTN_1, KEY_VOLUMEUP);
+ snd_jack_set_key(cz_jack.jack, SND_JACK_BTN_2, KEY_VOLUMEDOWN);
+ snd_jack_set_key(cz_jack.jack, SND_JACK_BTN_3, KEY_VOICECOMMAND);
+
+ ret = snd_soc_component_set_jack(component, &cz_jack, NULL);
+ if (ret) {
+ dev_err(rtd->dev, "Headset Jack call-back failed: %d\n", ret);
+ return ret;
+ }
+ return 0;
+}
+
+static int rt5682_clk_enable(struct snd_pcm_substream *substream)
+{
+ int ret;
+ struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+
+ /*
+ * Set wclk to 48000 because the rate constraint of this driver is
+ * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate that is
+ * minimum of 64x the LRCLK sample rate." RT5682 is the only clk
+ * source so for all codecs we have to limit bclk to 64X lrclk.
+ */
+ clk_set_rate(rt5682_dai_wclk, 48000);
+ clk_set_rate(rt5682_dai_bclk, 48000 * 64);
+ ret = clk_prepare_enable(rt5682_dai_bclk);
+ if (ret < 0) {
+ dev_err(rtd->dev, "can't enable master clock %d\n", ret);
+ return ret;
+ }
+ return ret;
+}
+
+static void rt5682_clk_disable(void)
+{
+ clk_disable_unprepare(rt5682_dai_bclk);
+}
+
static const unsigned int channels[] = {
DUAL_CHANNEL,
};
@@ -260,6 +328,118 @@ static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
da7219_clk_disable();
}

+static int cz_rt5682_play_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+ struct snd_soc_card *card = rtd->card;
+ struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+ /*
+ * On this platform for PCM device we support stereo
+ */
+
+ runtime->hw.channels_max = DUAL_CHANNEL;
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+ &constraints_channels);
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+ &constraints_rates);
+
+ machine->play_i2s_instance = I2S_SP_INSTANCE;
+ return rt5682_clk_enable(substream);
+}
+
+static int cz_rt5682_cap_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+ struct snd_soc_card *card = rtd->card;
+ struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+ /*
+ * On this platform for PCM device we support stereo
+ */
+
+ runtime->hw.channels_max = DUAL_CHANNEL;
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+ &constraints_channels);
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+ &constraints_rates);
+
+ machine->cap_i2s_instance = I2S_SP_INSTANCE;
+ machine->capture_channel = CAP_CHANNEL1;
+ return rt5682_clk_enable(substream);
+}
+
+static int cz_rt5682_max_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+ struct snd_soc_card *card = rtd->card;
+ struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+ /*
+ * On this platform for PCM device we support stereo
+ */
+
+ runtime->hw.channels_max = DUAL_CHANNEL;
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+ &constraints_channels);
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+ &constraints_rates);
+
+ machine->play_i2s_instance = I2S_BT_INSTANCE;
+ return rt5682_clk_enable(substream);
+}
+
+static int cz_rt5682_dmic0_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+ struct snd_soc_card *card = rtd->card;
+ struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+ /*
+ * On this platform for PCM device we support stereo
+ */
+
+ runtime->hw.channels_max = DUAL_CHANNEL;
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+ &constraints_channels);
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+ &constraints_rates);
+
+ machine->cap_i2s_instance = I2S_BT_INSTANCE;
+ return rt5682_clk_enable(substream);
+}
+
+static int cz_rt5682_dmic1_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+ struct snd_soc_card *card = rtd->card;
+ struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+ /*
+ * On this platform for PCM device we support stereo
+ */
+
+ runtime->hw.channels_max = DUAL_CHANNEL;
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+ &constraints_channels);
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+ &constraints_rates);
+
+ machine->cap_i2s_instance = I2S_SP_INSTANCE;
+ machine->capture_channel = CAP_CHANNEL0;
+ return rt5682_clk_enable(substream);
+}
+
+static void cz_rt5682_shutdown(struct snd_pcm_substream *substream)
+{
+ rt5682_clk_disable();
+}
+
static const struct snd_soc_ops cz_da7219_play_ops = {
.startup = cz_da7219_play_startup,
.shutdown = cz_da7219_shutdown,
@@ -285,6 +465,31 @@ static const struct snd_soc_ops cz_dmic1_cap_ops = {
.shutdown = cz_da7219_shutdown,
};

+static const struct snd_soc_ops cz_rt5682_play_ops = {
+ .startup = cz_rt5682_play_startup,
+ .shutdown = cz_rt5682_shutdown,
+};
+
+static const struct snd_soc_ops cz_rt5682_cap_ops = {
+ .startup = cz_rt5682_cap_startup,
+ .shutdown = cz_rt5682_shutdown,
+};
+
+static const struct snd_soc_ops cz_rt5682_max_play_ops = {
+ .startup = cz_rt5682_max_startup,
+ .shutdown = cz_rt5682_shutdown,
+};
+
+static const struct snd_soc_ops cz_rt5682_dmic0_cap_ops = {
+ .startup = cz_rt5682_dmic0_startup,
+ .shutdown = cz_rt5682_shutdown,
+};
+
+static const struct snd_soc_ops cz_rt5682_dmic1_cap_ops = {
+ .startup = cz_rt5682_dmic1_startup,
+ .shutdown = cz_rt5682_shutdown,
+};
+
SND_SOC_DAILINK_DEF(designware1,
DAILINK_COMP_ARRAY(COMP_CPU("designware-i2s.1.auto")));
SND_SOC_DAILINK_DEF(designware2,
@@ -294,6 +499,8 @@ SND_SOC_DAILINK_DEF(designware3,

SND_SOC_DAILINK_DEF(dlgs,
DAILINK_COMP_ARRAY(COMP_CODEC("i2c-DLGS7219:00", "da7219-hifi")));
+SND_SOC_DAILINK_DEF(rt5682,
+ DAILINK_COMP_ARRAY(COMP_CODEC("i2c-10EC5682:00", "rt5682-aif1")));
SND_SOC_DAILINK_DEF(mx,
DAILINK_COMP_ARRAY(COMP_CODEC("MX98357A:00", "HiFi")));
SND_SOC_DAILINK_DEF(adau,
@@ -353,6 +560,57 @@ static struct snd_soc_dai_link cz_dai_7219_98357[] = {
},
};

+static struct snd_soc_dai_link cz_dai_5682_98357[] = {
+ {
+ .name = "amd-rt5682-play",
+ .stream_name = "Playback",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
+ | SND_SOC_DAIFMT_CBM_CFM,
+ .init = cz_rt5682_init,
+ .dpcm_playback = 1,
+ .ops = &cz_rt5682_play_ops,
+ SND_SOC_DAILINK_REG(designware1, rt5682, platform),
+ },
+ {
+ .name = "amd-rt5682-cap",
+ .stream_name = "Capture",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
+ | SND_SOC_DAIFMT_CBM_CFM,
+ .dpcm_capture = 1,
+ .ops = &cz_rt5682_cap_ops,
+ SND_SOC_DAILINK_REG(designware2, rt5682, platform),
+ },
+ {
+ .name = "amd-max98357-play",
+ .stream_name = "HiFi Playback",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
+ | SND_SOC_DAIFMT_CBM_CFM,
+ .dpcm_playback = 1,
+ .ops = &cz_rt5682_max_play_ops,
+ SND_SOC_DAILINK_REG(designware3, mx, platform),
+ },
+ {
+ /* C panel DMIC */
+ .name = "dmic0",
+ .stream_name = "DMIC0 Capture",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
+ | SND_SOC_DAIFMT_CBM_CFM,
+ .dpcm_capture = 1,
+ .ops = &cz_rt5682_dmic0_cap_ops,
+ SND_SOC_DAILINK_REG(designware3, adau, platform),
+ },
+ {
+ /* A/B panel DMIC */
+ .name = "dmic1",
+ .stream_name = "DMIC1 Capture",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
+ | SND_SOC_DAIFMT_CBM_CFM,
+ .dpcm_capture = 1,
+ .ops = &cz_rt5682_dmic1_cap_ops,
+ SND_SOC_DAILINK_REG(designware2, adau, platform),
+ },
+};
+
static const struct snd_soc_dapm_widget cz_widgets[] = {
SND_SOC_DAPM_HP("Headphones", NULL),
SND_SOC_DAPM_SPK("Speakers", NULL),
@@ -368,6 +626,14 @@ static const struct snd_soc_dapm_route cz_audio_route[] = {
{"PDM_DAT", NULL, "Int Mic"},
};

+static const struct snd_soc_dapm_route cz_rt5682_audio_route[] = {
+ {"Headphones", NULL, "HPOL"},
+ {"Headphones", NULL, "HPOR"},
+ {"IN1P", NULL, "Headset Mic"},
+ {"Speakers", NULL, "Speaker"},
+ {"PDM_DAT", NULL, "Int Mic"},
+};
+
static const struct snd_kcontrol_new cz_mc_controls[] = {
SOC_DAPM_PIN_SWITCH("Headphones"),
SOC_DAPM_PIN_SWITCH("Speakers"),
@@ -388,6 +654,28 @@ static struct snd_soc_card cz_card = {
.num_controls = ARRAY_SIZE(cz_mc_controls),
};

+static struct snd_soc_card cz_rt5682_card = {
+ .name = "acpr5682m98357",
+ .owner = THIS_MODULE,
+ .dai_link = cz_dai_5682_98357,
+ .num_links = ARRAY_SIZE(cz_dai_5682_98357),
+ .dapm_widgets = cz_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(cz_widgets),
+ .dapm_routes = cz_rt5682_audio_route,
+ .controls = cz_mc_controls,
+ .num_controls = ARRAY_SIZE(cz_mc_controls),
+};
+
+void *soc_is_rltk_max(struct device *dev)
+{
+ const struct acpi_device_id *match;
+
+ match = acpi_match_device(dev->driver->acpi_match_table, dev);
+ if (!match)
+ return NULL;
+ return (void *)match->driver_data;
+}
+
static struct regulator_consumer_supply acp_da7219_supplies[] = {
REGULATOR_SUPPLY("VDD", "i2c-DLGS7219:00"),
REGULATOR_SUPPLY("VDDMIC", "i2c-DLGS7219:00"),
@@ -425,29 +713,39 @@ static int cz_probe(struct platform_device *pdev)
struct snd_soc_card *card;
struct acp_platform_info *machine;
struct regulator_dev *rdev;
-
- acp_da7219_cfg.dev = &pdev->dev;
- rdev = devm_regulator_register(&pdev->dev, &acp_da7219_desc,
- &acp_da7219_cfg);
- if (IS_ERR(rdev)) {
- dev_err(&pdev->dev, "Failed to register regulator: %d\n",
- (int)PTR_ERR(rdev));
- return -EINVAL;
+ struct device *dev = &pdev->dev;
+
+ card = (struct snd_soc_card *)soc_is_rltk_max(dev);
+ if (!card)
+ return -ENODEV;
+ if (!strcmp(card->name, "acpd7219m98357")) {
+ acp_da7219_cfg.dev = &pdev->dev;
+ rdev = devm_regulator_register(&pdev->dev, &acp_da7219_desc,
+ &acp_da7219_cfg);
+ if (IS_ERR(rdev)) {
+ dev_err(&pdev->dev, "Failed to register regulator: %d\n",
+ (int)PTR_ERR(rdev));
+ return -EINVAL;
+ }
}

machine = devm_kzalloc(&pdev->dev, sizeof(struct acp_platform_info),
GFP_KERNEL);
if (!machine)
return -ENOMEM;
- card = &cz_card;
- cz_card.dev = &pdev->dev;
+ card->dev = &pdev->dev;
platform_set_drvdata(pdev, card);
snd_soc_card_set_drvdata(card, machine);
- ret = devm_snd_soc_register_card(&pdev->dev, &cz_card);
+ ret = devm_snd_soc_register_card(&pdev->dev, card);
if (ret) {
- dev_err(&pdev->dev,
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev,
"devm_snd_soc_register_card(%s) failed: %d\n",
- cz_card.name, ret);
+ card->name, ret);
+ else
+ dev_dbg(&pdev->dev,
+ "devm_snd_soc_register_card(%s) probe deferred: %d\n",
+ card->name, ret);
return ret;
}
bt_uart_enable = !device_property_read_bool(&pdev->dev,
@@ -457,7 +755,8 @@ static int cz_probe(struct platform_device *pdev)

#ifdef CONFIG_ACPI
static const struct acpi_device_id cz_audio_acpi_match[] = {
- { "AMD7219", 0 },
+ { "AMD7219", (unsigned long)&cz_card },
+ { "AMDI5682", (unsigned long)&cz_rt5682_card},
{},
};
MODULE_DEVICE_TABLE(acpi, cz_audio_acpi_match);
@@ -475,5 +774,6 @@ static struct platform_driver cz_pcm_driver = {
module_platform_driver(cz_pcm_driver);

MODULE_AUTHOR("[email protected]");
-MODULE_DESCRIPTION("DA7219 & MAX98357A audio support");
+MODULE_AUTHOR("[email protected]");
+MODULE_DESCRIPTION("DA7219, RT5682 & MAX98357A audio support");
MODULE_LICENSE("GPL v2");
--
2.7.4


2021-03-15 18:36:34

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH v3] ASoC: amd: add support for rt5682 codec in machine driver


> +static int rt5682_clk_enable(struct snd_pcm_substream *substream)
> +{
> + int ret;
> + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> +
> + /*
> + * Set wclk to 48000 because the rate constraint of this driver is
> + * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate that is
> + * minimum of 64x the LRCLK sample rate." RT5682 is the only clk
> + * source so for all codecs we have to limit bclk to 64X lrclk.
> + */
> + clk_set_rate(rt5682_dai_wclk, 48000);
> + clk_set_rate(rt5682_dai_bclk, 48000 * 64);
> + ret = clk_prepare_enable(rt5682_dai_bclk);
> + if (ret < 0) {
> + dev_err(rtd->dev, "can't enable master clock %d\n", ret);
> + return ret;
> + }
> + return ret;
> +}

Out of curiosity, is there a reason why you use clk_prepare_enable() for
the bclk but not for the wclk?

2021-03-16 16:03:35

by Vijendar Mukunda

[permalink] [raw]
Subject: Re: [PATCH v3] ASoC: amd: add support for rt5682 codec in machine driver



On 15/03/21 9:30 pm, Pierre-Louis Bossart wrote:
>
>> +static int rt5682_clk_enable(struct snd_pcm_substream *substream)
>> +{
>> +    int ret;
>> +    struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
>> +
>> +    /*
>> +     * Set wclk to 48000 because the rate constraint of this driver is
>> +     * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate that is
>> +     * minimum of 64x the LRCLK sample rate." RT5682 is the only clk
>> +     * source so for all codecs we have to limit bclk to 64X lrclk.
>> +     */
>> +    clk_set_rate(rt5682_dai_wclk, 48000);
>> +    clk_set_rate(rt5682_dai_bclk, 48000 * 64);
>> +    ret = clk_prepare_enable(rt5682_dai_bclk);
>> +    if (ret < 0) {
>> +        dev_err(rtd->dev, "can't enable master clock %d\n", ret);
>> +        return ret;
>> +    }
>> +    return ret;
>> +}
>
> Out of curiosity, is there a reason why you use clk_prepare_enable() for
> the bclk but not for the wclk?These changes were shared by codec vendor as an initial patch.
We should use clk_prepare_enable() for wclk not for bclk.
We will update and share the new patch.



2021-03-16 16:31:05

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH v3] ASoC: amd: add support for rt5682 codec in machine driver



On 3/16/21 8:37 AM, Mukunda,Vijendar wrote:
>
>
> On 15/03/21 9:30 pm, Pierre-Louis Bossart wrote:
>>
>>> +static int rt5682_clk_enable(struct snd_pcm_substream *substream)
>>> +{
>>> +    int ret;
>>> +    struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
>>> +
>>> +    /*
>>> +     * Set wclk to 48000 because the rate constraint of this driver is
>>> +     * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate that is
>>> +     * minimum of 64x the LRCLK sample rate." RT5682 is the only clk
>>> +     * source so for all codecs we have to limit bclk to 64X lrclk.
>>> +     */
>>> +    clk_set_rate(rt5682_dai_wclk, 48000);
>>> +    clk_set_rate(rt5682_dai_bclk, 48000 * 64);
>>> +    ret = clk_prepare_enable(rt5682_dai_bclk);
>>> +    if (ret < 0) {
>>> +        dev_err(rtd->dev, "can't enable master clock %d\n", ret);
>>> +        return ret;
>>> +    }
>>> +    return ret;
>>> +}
>>
>> Out of curiosity, is there a reason why you use clk_prepare_enable()
>> for the bclk but not for the wclk?These changes were shared by codec
>> vendor as an initial patch.
> We should use clk_prepare_enable() for wclk not for bclk.
> We will update and share the new patch.

Well the question remains: if you have two clocks and only enable one,
why do you need to get two clocks.

Also this patch was modeled after the da7219 case, where the same open
applies.

2021-03-17 05:59:55

by Shuming [范書銘]

[permalink] [raw]
Subject: RE: [PATCH v3] ASoC: amd: add support for rt5682 codec in machine driver

> On 3/16/21 8:37 AM, Mukunda,Vijendar wrote:
> >
> >
> > On 15/03/21 9:30 pm, Pierre-Louis Bossart wrote:
> >>
> >>> +static int rt5682_clk_enable(struct snd_pcm_substream *substream) {
> >>> +    int ret;
> >>> +    struct snd_soc_pcm_runtime *rtd =
> >>> +asoc_substream_to_rtd(substream);
> >>> +
> >>> +    /*
> >>> +     * Set wclk to 48000 because the rate constraint of this driver
> >>> +is
> >>> +     * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate
> >>> +that is
> >>> +     * minimum of 64x the LRCLK sample rate." RT5682 is the only
> >>> +clk
> >>> +     * source so for all codecs we have to limit bclk to 64X lrclk.
> >>> +     */
> >>> +    clk_set_rate(rt5682_dai_wclk, 48000);
> >>> +    clk_set_rate(rt5682_dai_bclk, 48000 * 64);
> >>> +    ret = clk_prepare_enable(rt5682_dai_bclk);
> >>> +    if (ret < 0) {
> >>> +        dev_err(rtd->dev, "can't enable master clock %d\n", ret);
> >>> +        return ret;
> >>> +    }
> >>> +    return ret;
> >>> +}
> >>
> >> Out of curiosity, is there a reason why you use clk_prepare_enable()
> >> for the bclk but not for the wclk?These changes were shared by codec
> >> vendor as an initial patch.
> > We should use clk_prepare_enable() for wclk not for bclk.
> > We will update and share the new patch.
>
> Well the question remains: if you have two clocks and only enable one, why do
> you need to get two clocks.
>
> Also this patch was modeled after the da7219 case, where the same open
> applies.

The RT5682 could set the wclk and bclk separately.
The bclk could set the different ratios (32/64/128/256).
However, only the wclk of DAI clock has the .prepare callback function to enable the related power.
The next patch should update the clk_prepare_enable() for wclk.

> ------Please consider the environment before printing this e-mail.