| Message ID | 20220308072435.22460-1-trevor.wu@mediatek.com |
|---|---|
| Headers | show |
| Series | ASoC: mediatek: Add support for MT8195 sound card with max98390 and rt5682 | expand |
On Tue, Mar 08, 2022 at 03:24:35PM +0800, Trevor Wu wrote: > This patch adds document for mt8195 board with mt6359, max98390 and > rt5682. > > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com> > --- > .../sound/mt8195-mt6359-max98390-rt5682.yaml | 61 +++++++++++++++++++ > 1 file changed, 61 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/mt8195-mt6359-max98390-rt5682.yaml > > diff --git a/Documentation/devicetree/bindings/sound/mt8195-mt6359-max98390-rt5682.yaml b/Documentation/devicetree/bindings/sound/mt8195-mt6359-max98390-rt5682.yaml > new file mode 100644 > index 000000000000..7ec14d61b109 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mt8195-mt6359-max98390-rt5682.yaml > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/mt8195-mt6359-max98390-rt5682.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT8195 with MT6359, MAX98390 and RT5682 ASoC sound card driver > + > +maintainers: > + - Trevor Wu <trevor.wu@mediatek.com> > + > +description: > + This binding describes the MT8195 sound card. > + > +properties: > + compatible: > + const: mediatek,mt8195_mt6359_max98390_rt5682 You have nodes for each of these components, why do we need new compatible string for each combination. You can figure out the combination by looking at each of those nodes. Second, why does each combination need a new schema doc? Rob
On Tue, 8 Mar 2022 15:24:30 +0800, Trevor Wu wrote: > This series of patches adds support for mt8195 board with mt6359, max98390 > and rt5682. > > Reset controller is included because mt8195 etdm is used to play sound via > max98390 before kernel boot. > > In addition, the common part of machine driver is extracted for > simplification. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/5] ASoC: mediatek: mt8195: add reset controller commit: f67084148dac015d059c64f25e57abd0ab18946c [2/5] dt-bindings: mediatek: mt8195: add reset property commit: ee7f79a81a27c47088fe0af95788621644826d91 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Thu, 2022-03-10 at 16:18 -0600, Rob Herring wrote: > On Tue, Mar 08, 2022 at 03:24:35PM +0800, Trevor Wu wrote: > > This patch adds document for mt8195 board with mt6359, max98390 and > > rt5682. > > > > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com> > > --- > > .../sound/mt8195-mt6359-max98390-rt5682.yaml | 61 > > +++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/sound/mt8195- > > mt6359-max98390-rt5682.yaml > > > > diff --git a/Documentation/devicetree/bindings/sound/mt8195-mt6359- > > max98390-rt5682.yaml > > b/Documentation/devicetree/bindings/sound/mt8195-mt6359-max98390- > > rt5682.yaml > > new file mode 100644 > > index 000000000000..7ec14d61b109 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/mt8195-mt6359- > > max98390-rt5682.yaml > > @@ -0,0 +1,61 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mt8195-mt6359-max98390-rt5682.yaml*__;Iw!!CTRNKA9wMg0ARbw!zb7eaqdAQfuyPpP5m31L3Q5pdCulclJgnygkkMgYh2M6segUZedd-cYz51-5Q2XDCA$ > > > > +$schema: > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!zb7eaqdAQfuyPpP5m31L3Q5pdCulclJgnygkkMgYh2M6segUZedd-cYz5187C1ArQA$ > > > > + > > +title: Mediatek MT8195 with MT6359, MAX98390 and RT5682 ASoC sound > > card driver > > + > > +maintainers: > > + - Trevor Wu <trevor.wu@mediatek.com> > > + > > +description: > > + This binding describes the MT8195 sound card. > > + > > +properties: > > + compatible: > > + const: mediatek,mt8195_mt6359_max98390_rt5682 > > You have nodes for each of these components, why do we need new > compatible string for each combination. You can figure out the > combination by looking at each of those nodes. > > Second, why does each combination need a new schema doc? > > Rob Hi Rob, I'm not sure whether I can reuse the old schema doc because of the doc name and compatible string seems to be specifically for the codec combination. If I want to reuse the old schema doc, should I change the doc name or compatible string? Make the naming more general. Thanks, Trevor
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? > 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. Thanks, Trevor
This series of patches adds support for mt8195 board with mt6359, max98390 and rt5682. Reset controller is included because mt8195 etdm is used to play sound via max98390 before kernel boot. In addition, the common part of machine driver is extracted for simplification. Patches are based on broonie tree "for-next" branch. Trevor Wu (5): ASoC: mediatek: mt8195: add reset controller dt-bindings: mediatek: mt8195: add reset property ASoC: mediatek: mt8195: common code for mt8195 machine driver ASoC: mediatek: mt8195: add machine driver with mt6359, max98390 and rt5682 dt-bindings: mediatek: mt8195: add mt8195-mt6359-max98390-rt5682 document .../bindings/sound/mt8195-afe-pcm.yaml | 10 + .../sound/mt8195-mt6359-max98390-rt5682.yaml | 61 + sound/soc/mediatek/Kconfig | 16 + sound/soc/mediatek/mt8195/Makefile | 17 +- sound/soc/mediatek/mt8195/mt8195-afe-pcm.c | 16 + .../mediatek/mt8195/mt8195-mt6359-common.c | 398 +++++++ .../mediatek/mt8195/mt8195-mt6359-common.h | 30 + .../mt8195/mt8195-mt6359-max98390-rt5682.c | 1058 +++++++++++++++++ .../mt8195/mt8195-mt6359-rt1011-rt5682.c | 406 +------ .../mt8195/mt8195-mt6359-rt1019-rt5682.c | 409 +------ 10 files changed, 1618 insertions(+), 803 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/mt8195-mt6359-max98390-rt5682.yaml create mode 100644 sound/soc/mediatek/mt8195/mt8195-mt6359-common.c create mode 100644 sound/soc/mediatek/mt8195/mt8195-mt6359-common.h create mode 100644 sound/soc/mediatek/mt8195/mt8195-mt6359-max98390-rt5682.c