Il 12/03/22 17:18, Trevor Wu ha scritto:
On Thu, 2022-03-10 at 16:21 +0100, AngeloGioacchino Del Regno wrote:
Il 08/03/22 08:24, Trevor Wu ha scritto:
This patch adds support for mt8195 board with mt6359, max98390 and rt5682.
Signed-off-by: Trevor Wu trevor.wu@mediatek.com
Hello Trevor, thanks for the patch! However, there's something to improve...
sound/soc/mediatek/Kconfig | 16 + sound/soc/mediatek/mt8195/Makefile | 5 + .../mt8195/mt8195-mt6359-max98390-rt5682.c | 1058 +++++++++++++++++ 3 files changed, 1079 insertions(+) create mode 100644 sound/soc/mediatek/mt8195/mt8195-mt6359- max98390-rt5682.c
[...]
+static const struct snd_soc_dapm_widget
- mt8195_mt6359_max98390_rt5682_widgets[] = {
- SND_SOC_DAPM_SPK("Left Speaker", NULL),
- SND_SOC_DAPM_SPK("Right Speaker", NULL),
- SND_SOC_DAPM_HP("Headphone Jack", NULL),
We can at least partially reuse existing UCM2 configuration if you slightly change the names for these controls.
I don't know what the UCM2 configuration means. Could you give me more information?
UCM == Use Case Manager; In short, it's userspace (alsa-lib) configuration for sound cards, allowing to configure the various mixers for various usecases (speaker/headphone/HDMI playback, headset/internal microphone, etc).
Check this GitHub repository for more information: https://github.com/alsa-project/alsa-ucm-conf/tree/master/ucm2
Specifically, MAX98090 (yes I know it's a different codec) has names "Speaker Left", "Speaker Right" instead, we will be able to at least partially reuse these (or get uniform naming, which is still good). As for the "Headphone Jack", it's simply "Headphone".
Please note that the actual control names in userspace will be, exactly,
"Speaker Left Switch", "Speaker Right Switch", "Headphone Left Switch", "Headphone Right Switch"...
....where "Switch" gets automatically appended because of the control type.
- SND_SOC_DAPM_MIC("Headset Mic", NULL),
This "Headset Mic" name is fine.
- SND_SOC_DAPM_MIXER(SOF_DMA_DL2, SND_SOC_NOPM, 0, 0, NULL, 0),
- SND_SOC_DAPM_MIXER(SOF_DMA_DL3, SND_SOC_NOPM, 0, 0, NULL, 0),
- SND_SOC_DAPM_MIXER(SOF_DMA_UL4, SND_SOC_NOPM, 0, 0, NULL, 0),
- SND_SOC_DAPM_MIXER(SOF_DMA_UL5, SND_SOC_NOPM, 0, 0, NULL, 0),
+};
[...]
+static struct snd_soc_dai_link mt8195_mt6359_max98390_rt5682_dai_links[] = {
... again, different name, same contents ...
And I won't go on repeating the same thing over and over again. I think that the best idea here is to either create a mt8195-mt6359- rt5682-common.c file, or to rename the others to something else and get them all in the same file.
Regards, Angelo
Hi Angelo,
Thanks for your review. Please forgive me for deleting some comments above. I totally agree that most code can be reused. I will try revising and merging all mt8195 machine drivers in a file.
No worries. Looking forward to see the next version. Thank you!
Regards, Angelo
Thanks, Trevor