[PATCH] [v2] ASoC: Intel: sof_rt5682: Add support for max98390 speaker amp
Configure adl_max98390_rt5682 to support the rt5682 headset codec and max98390 speaker
BUG=b:191811888 TEST=emerge-brya chromeos-kernel-5_10
Signed-off-by: Mark Hsieh mark_hsieh@wistron.corp-partner.google.com --- sound/soc/intel/boards/Kconfig | 1 + sound/soc/intel/boards/sof_maxim_common.c | 59 +++++++++++++++++++ sound/soc/intel/boards/sof_maxim_common.h | 12 ++++ sound/soc/intel/boards/sof_rt5682.c | 20 ++++++- .../intel/common/soc-acpi-intel-adl-match.c | 13 ++++ 5 files changed, 104 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 7e29b0d911e2..0017c08c5a62 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -470,6 +470,7 @@ config SND_SOC_INTEL_SOF_RT5682_MACH select SND_SOC_RT1015 select SND_SOC_RT1015P select SND_SOC_RT5682_I2C + select SND_SOC_MAX98390 select SND_SOC_DMIC select SND_SOC_HDAC_HDMI select SND_SOC_INTEL_HDA_DSP_COMMON diff --git a/sound/soc/intel/boards/sof_maxim_common.c b/sound/soc/intel/boards/sof_maxim_common.c index e9c52f8b6428..477883d571ab 100644 --- a/sound/soc/intel/boards/sof_maxim_common.c +++ b/sound/soc/intel/boards/sof_maxim_common.c @@ -133,6 +133,65 @@ void max_98373_set_codec_conf(struct snd_soc_card *card) } EXPORT_SYMBOL_NS(max_98373_set_codec_conf, SND_SOC_INTEL_SOF_MAXIM_COMMON);
+/* + * Maxim MAX98390 + */ + +static struct snd_soc_codec_conf max_98390_codec_conf[] = { + { + .dlc = COMP_CODEC_CONF(MAX_98390_DEV0_NAME), + .name_prefix = "Right", + }, + { + .dlc = COMP_CODEC_CONF(MAX_98390_DEV1_NAME), + .name_prefix = "Left", + }, +}; + +struct snd_soc_dai_link_component max_98390_components[] = { + { /* For Right */ + .name = MAX_98390_DEV0_NAME, + .dai_name = MAX_98390_CODEC_DAI, + }, + { /* For Left */ + .name = MAX_98390_DEV1_NAME, + .dai_name = MAX_98390_CODEC_DAI, + }, +}; +EXPORT_SYMBOL_NS(max_98390_components, SND_SOC_INTEL_SOF_MAXIM_COMMON); + +static int max_98390_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_dai *codec_dai; + int j; + + for_each_rtd_codec_dais(rtd, j, codec_dai) { + if (!strcmp(codec_dai->component->name, MAX_98390_DEV0_NAME)) { + /* DEV0 tdm slot configuration */ + snd_soc_dai_set_tdm_slot(codec_dai, 0x03, 3, 2, 16); + } + if (!strcmp(codec_dai->component->name, MAX_98390_DEV1_NAME)) { + /* DEV1 tdm slot configuration */ + snd_soc_dai_set_tdm_slot(codec_dai, 0x0C, 3, 2, 16); + } + } + return 0; +} + +struct snd_soc_ops max_98390_ops = { + .hw_params = max_98390_hw_params, +}; +EXPORT_SYMBOL_NS(max_98390_ops, SND_SOC_INTEL_SOF_MAXIM_COMMON); + +void max_98390_set_codec_conf(struct snd_soc_card *card) +{ + card->codec_conf = max_98390_codec_conf; + card->num_configs = ARRAY_SIZE(max_98390_codec_conf); +} +EXPORT_SYMBOL_NS(max_98390_set_codec_conf, SND_SOC_INTEL_SOF_MAXIM_COMMON); + /* * Maxim MAX98357A */ diff --git a/sound/soc/intel/boards/sof_maxim_common.h b/sound/soc/intel/boards/sof_maxim_common.h index 2674f1e373ef..2458758866f3 100644 --- a/sound/soc/intel/boards/sof_maxim_common.h +++ b/sound/soc/intel/boards/sof_maxim_common.h @@ -24,6 +24,18 @@ int max_98373_spk_codec_init(struct snd_soc_pcm_runtime *rtd); void max_98373_set_codec_conf(struct snd_soc_card *card); int max_98373_trigger(struct snd_pcm_substream *substream, int cmd);
+/* + * Maxim MAX98390 + */ +#define MAX_98390_CODEC_DAI "max98390-aif1" +#define MAX_98390_DEV0_NAME "i2c-MX98390:00" +#define MAX_98390_DEV1_NAME "i2c-MX98390:01" + +extern struct snd_soc_dai_link_component max_98390_components[2]; +extern struct snd_soc_ops max_98390_ops; + +void max_98390_set_codec_conf(struct snd_soc_card *card); + /* * Maxim MAX98357A */ diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c index 39217223d50c..dc4966056b7d 100644 --- a/sound/soc/intel/boards/sof_rt5682.c +++ b/sound/soc/intel/boards/sof_rt5682.c @@ -49,6 +49,7 @@ #define SOF_RT1015P_SPEAKER_AMP_PRESENT BIT(16) #define SOF_MAX98373_SPEAKER_AMP_PRESENT BIT(17) #define SOF_MAX98360A_SPEAKER_AMP_PRESENT BIT(18) +#define SOF_MAX98390_SPEAKER_AMP_PRESENT BIT(23)
/* BT audio offload: reserve 3 bits for future */ #define SOF_BT_OFFLOAD_SSP_SHIFT 19 @@ -781,6 +782,13 @@ static struct snd_soc_dai_link *sof_card_dai_links_create(struct device *dev, } else if (sof_rt5682_quirk & SOF_RT1011_SPEAKER_AMP_PRESENT) { sof_rt1011_dai_link(&links[id]); + } else if (sof_rt5682_quirk & + SOF_MAX98390_SPEAKER_AMP_PRESENT) { + links[id].codecs = max_98390_components; + links[id].num_codecs = ARRAY_SIZE(max_98390_components); + links[id].init = max_98373_spk_codec_init; + links[id].ops = &max_98390_ops; + links[id].dpcm_capture = 1; } else { max_98357a_dai_link(&links[id]); } @@ -917,7 +925,8 @@ static int sof_audio_probe(struct platform_device *pdev) sof_rt1011_codec_conf(&sof_audio_card_rt5682); else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) sof_rt1015p_codec_conf(&sof_audio_card_rt5682); - + else if (sof_rt5682_quirk & SOF_MAX98390_SPEAKER_AMP_PRESENT) + max_98390_set_codec_conf(&sof_audio_card_rt5682); if (sof_rt5682_quirk & SOF_SSP_BT_OFFLOAD_PRESENT) sof_audio_card_rt5682.num_links++;
@@ -1043,6 +1052,15 @@ static const struct platform_device_id board_ids[] = { SOF_RT5682_SSP_AMP(2) | SOF_RT5682_NUM_HDMIDEV(4)), }, + { + .name = "adl_max98390_rt5682", + .driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN | + SOF_RT5682_SSP_CODEC(0) | + SOF_SPEAKER_AMP_PRESENT | + SOF_MAX98390_SPEAKER_AMP_PRESENT | + SOF_RT5682_SSP_AMP(2) | + SOF_RT5682_NUM_HDMIDEV(4)), + }, { } }; MODULE_DEVICE_TABLE(platform, board_ids); diff --git a/sound/soc/intel/common/soc-acpi-intel-adl-match.c b/sound/soc/intel/common/soc-acpi-intel-adl-match.c index a0f6a69c7038..2db152998e4a 100644 --- a/sound/soc/intel/common/soc-acpi-intel-adl-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-adl-match.c @@ -280,6 +280,11 @@ static const struct snd_soc_acpi_codecs adl_max98357a_amp = { .codecs = {"MX98357A"} };
+static const struct snd_soc_acpi_codecs adl_max98390_amp = { + .num_codecs = 1, + .codecs = {"MX98390"} +}; + struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = { { .id = "10EC5682", @@ -297,6 +302,14 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = { .sof_fw_filename = "sof-adl.ri", .sof_tplg_filename = "sof-adl-max98357a-rt5682.tplg", }, + { + .id = "10EC5682", + .drv_name = "adl_max98390_rt5682", + .machine_quirk = snd_soc_acpi_codec_list, + .quirk_data = &adl_max98390_amp, + .sof_fw_filename = "sof-adl.ri", + .sof_tplg_filename = "sof-adl-max98390-rt5682.tplg", + }, {}, }; EXPORT_SYMBOL_GPL(snd_soc_acpi_intel_adl_machines);
On 2021-08-24 3:21 PM, Mark Hsieh wrote:
Configure adl_max98390_rt5682 to support the rt5682 headset codec and max98390 speaker
Unsure if line-length for commit messages has been extended to 100 as it was the case for code parts but this line certainly exceeds default.
BUG=b:191811888 TEST=emerge-brya chromeos-kernel-5_10
Are these two tags meaningful for upstream kernel?
Signed-off-by: Mark Hsieh mark_hsieh@wistron.corp-partner.google.com
sound/soc/intel/boards/Kconfig | 1 + sound/soc/intel/boards/sof_maxim_common.c | 59 +++++++++++++++++++ sound/soc/intel/boards/sof_maxim_common.h | 12 ++++ sound/soc/intel/boards/sof_rt5682.c | 20 ++++++- .../intel/common/soc-acpi-intel-adl-match.c | 13 ++++ 5 files changed, 104 insertions(+), 1 deletion(-)
...
+static int max_98390_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
- struct snd_soc_dai *codec_dai;
- int j;
We usually start from 'i' when it comes to choosing iteration-variable names.
- for_each_rtd_codec_dais(rtd, j, codec_dai) {
if (!strcmp(codec_dai->component->name, MAX_98390_DEV0_NAME)) {/* DEV0 tdm slot configuration */snd_soc_dai_set_tdm_slot(codec_dai, 0x03, 3, 2, 16);}
Suggesting to move comment one line up as you ideally want to explain the reasoning behind if-statement, not the actual execution statement.
Once that is done, both parenthesis can be dropped.
if (!strcmp(codec_dai->component->name, MAX_98390_DEV1_NAME)) {/* DEV1 tdm slot configuration */snd_soc_dai_set_tdm_slot(codec_dai, 0x0C, 3, 2, 16);}
Same applies here.
- }
- return 0;
+}
...
diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c index 39217223d50c..dc4966056b7d 100644 --- a/sound/soc/intel/boards/sof_rt5682.c +++ b/sound/soc/intel/boards/sof_rt5682.c @@ -49,6 +49,7 @@ #define SOF_RT1015P_SPEAKER_AMP_PRESENT BIT(16) #define SOF_MAX98373_SPEAKER_AMP_PRESENT BIT(17) #define SOF_MAX98360A_SPEAKER_AMP_PRESENT BIT(18) +#define SOF_MAX98390_SPEAKER_AMP_PRESENT BIT(23)
/* BT audio offload: reserve 3 bits for future */ #define SOF_BT_OFFLOAD_SSP_SHIFT 19 @@ -781,6 +782,13 @@ static struct snd_soc_dai_link *sof_card_dai_links_create(struct device *dev, } else if (sof_rt5682_quirk & SOF_RT1011_SPEAKER_AMP_PRESENT) { sof_rt1011_dai_link(&links[id]);
} else if (sof_rt5682_quirk &SOF_MAX98390_SPEAKER_AMP_PRESENT) {
Pretty sure this two lines could be combined - does not exceed 100character limit.
links[id].codecs = max_98390_components;links[id].num_codecs = ARRAY_SIZE(max_98390_components);links[id].init = max_98373_spk_codec_init;links[id].ops = &max_98390_ops;links[id].dpcm_capture = 1;@@ -917,7 +925,8 @@ static int sof_audio_probe(struct platform_device *pdev) sof_rt1011_codec_conf(&sof_audio_card_rt5682); else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) sof_rt1015p_codec_conf(&sof_audio_card_rt5682);
- else if (sof_rt5682_quirk & SOF_MAX98390_SPEAKER_AMP_PRESENT)
max_98390_set_codec_conf(&sof_audio_card_rt5682);
Please keep the newline between these two conditional blocks. A new if-statement usually translates to new thought and it is good to separate those.
@@ -1043,6 +1052,15 @@ static const struct platform_device_id board_ids[] = { SOF_RT5682_SSP_AMP(2) | SOF_RT5682_NUM_HDMIDEV(4)), },
- {
.name = "adl_max98390_rt5682",.driver_data = (kernel_ulong_t)(SOF_RT5682_MCLK_EN |SOF_RT5682_SSP_CODEC(0) |SOF_SPEAKER_AMP_PRESENT |SOF_MAX98390_SPEAKER_AMP_PRESENT |SOF_RT5682_SSP_AMP(2) |SOF_RT5682_NUM_HDMIDEV(4)),- },
This indentation seems off. Though it seems the same applies for the rest of the array..
{ } }; MODULE_DEVICE_TABLE(platform, board_ids); diff --git a/sound/soc/intel/common/soc-acpi-intel-adl-match.c b/sound/soc/intel/common/soc-acpi-intel-adl-match.c index a0f6a69c7038..2db152998e4a 100644 --- a/sound/soc/intel/common/soc-acpi-intel-adl-match.c +++ b/sound/soc/intel/common/soc-acpi-intel-adl-match.c @@ -280,6 +280,11 @@ static const struct snd_soc_acpi_codecs adl_max98357a_amp = { .codecs = {"MX98357A"} };
+static const struct snd_soc_acpi_codecs adl_max98390_amp = {
- .num_codecs = 1,
- .codecs = {"MX98390"}
For 'forward compatibility' with possible changes to struct snd_soc_acpi_codecs, I'd advise appending comma at the end of the line.
+};
Regards, Czarek
On Tue, Aug 24, 2021 at 03:59:44PM +0200, Cezary Rojewski wrote:
On 2021-08-24 3:21 PM, Mark Hsieh wrote:
Configure adl_max98390_rt5682 to support the rt5682 headset codec and max98390 speaker
Unsure if line-length for commit messages has been extended to 100 as it was the case for code parts but this line certainly exceeds default.
There's certainly no reason not to wrap this one. Even for code it's not something to actively aim for.
BUG=b:191811888 TEST=emerge-brya chromeos-kernel-5_10
Are these two tags meaningful for upstream kernel?
No.
Looks mostly good, minor comments below
On 8/24/21 8:21 AM, Mark Hsieh wrote:
Configure adl_max98390_rt5682 to support the rt5682 headset codec and max98390 speaker
BUG=b:191811888 TEST=emerge-brya chromeos-kernel-5_10
I don't think we need this for upstream?
+struct snd_soc_ops max_98390_ops = {
- .hw_params = max_98390_hw_params,
+};
this could be const. We didn't do it for previous codecs but that was a miss. I'll send a patch for the max98373
+extern struct snd_soc_ops max_98390_ops;
const
participants (4)
-
Cezary Rojewski -
Mark Brown -
Mark Hsieh -
Pierre-Louis Bossart