linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Trevor Wu <trevor.wu@mediatek.com>
To: Chen-Yu Tsai <wenst@chromium.org>
Cc: <broonie@kernel.org>, <tiwai@suse.com>,
	Rob Herring <robh+dt@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	<alsa-devel@alsa-project.org>,
	<linux-mediatek@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	LKML <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<bicycle.tsai@mediatek.com>, Jiaxin Yu <jiaxin.yu@mediatek.com>,
	<cychiang@google.com>, <aaronyu@google.com>
Subject: Re: [PATCH v2 5/8] ASoC: mediatek: mt8195: add platform driver
Date: Mon, 12 Jul 2021 23:09:58 +0800	[thread overview]
Message-ID: <9929a3a20df1a89fc94baf7c75c0c65d9a61de0f.camel@mediatek.com> (raw)
In-Reply-To: <CAGXv+5FzOs+=9PbYAEahVXvdJG1FnRkSUo_r3AVYZzNKGqg0oA@mail.gmail.com>

On Mon, 2021-07-12 at 14:57 +0800, Chen-Yu Tsai wrote:
>  are all internal Hi,
> 
> On Tue, Jun 29, 2021 at 9:49 AM Trevor Wu <trevor.wu@mediatek.com>
> wrote:
> > 
> > This patch adds mt8195 platform and affiliated driver.
> > 
> > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> > ---
> >  sound/soc/mediatek/Kconfig                     |    9 +
> >  sound/soc/mediatek/Makefile                   |    1 +
> >  sound/soc/mediatek/mt8195/Makefile            |   11 +
> >  sound/soc/mediatek/mt8195/mt8195-afe-clk.c    |  899 +++++
> >  sound/soc/mediatek/mt8195/mt8195-afe-clk.h    |  201 +
> >  sound/soc/mediatek/mt8195/mt8195-afe-common.h |  200 +
> >  sound/soc/mediatek/mt8195/mt8195-afe-pcm.c    | 3264
> > +++++++++++++++++
> >  sound/soc/mediatek/mt8195/mt8195-reg.h        | 2793
> > ++++++++++++++
> >  8 files changed, 7378 insertions(+)
> >  create mode 100644 sound/soc/mediatek/mt8195/Makefile
> >  create mode 100644 sound/soc/mediatek/mt8195/mt8195-afe-clk.c
> >  create mode 100644 sound/soc/mediatek/mt8195/mt8195-afe-clk.h
> >  create mode 100644 sound/soc/mediatek/mt8195/mt8195-afe-common.h
> >  create mode 100644 sound/soc/mediatek/mt8195/mt8195-afe-pcm.c
> >  create mode 100644 sound/soc/mediatek/mt8195/mt8195-reg.h
> > 
> > diff --git a/sound/soc/mediatek/Kconfig
> > b/sound/soc/mediatek/Kconfig
> > index 74dae4332d17..3389f382be06 100644
> > --- a/sound/soc/mediatek/Kconfig
> > +++ b/sound/soc/mediatek/Kconfig
> > @@ -184,3 +184,12 @@ config SND_SOC_MT8192_MT6359_RT1015_RT5682
> >           with the MT6359 RT1015 RT5682 audio codec.
> >           Select Y if you have such device.
> >           If unsure select "N".
> > +
> > +config SND_SOC_MT8195
> > +       tristate "ASoC support for Mediatek MT8195 chip"
> > +       select SND_SOC_MEDIATEK
> > +       help
> > +         This adds ASoC platform driver support for Mediatek
> > MT8195 chip
> > +         that can be used with other codecs.
> > +         Select Y if you have such device.
> > +         If unsure select "N".
> > diff --git a/sound/soc/mediatek/Makefile
> > b/sound/soc/mediatek/Makefile
> > index f6cb6b8508e3..34778ca12106 100644
> > --- a/sound/soc/mediatek/Makefile
> > +++ b/sound/soc/mediatek/Makefile
> > @@ -5,3 +5,4 @@ obj-$(CONFIG_SND_SOC_MT6797) += mt6797/
> >  obj-$(CONFIG_SND_SOC_MT8173) += mt8173/
> >  obj-$(CONFIG_SND_SOC_MT8183) += mt8183/
> >  obj-$(CONFIG_SND_SOC_MT8192) += mt8192/
> > +obj-$(CONFIG_SND_SOC_MT8195) += mt8195/
> > diff --git a/sound/soc/mediatek/mt8195/Makefile
> > b/sound/soc/mediatek/mt8195/Makefile
> > new file mode 100644
> > index 000000000000..b2c9fd88f39e
> > --- /dev/null
> > +++ b/sound/soc/mediatek/mt8195/Makefile
> > @@ -0,0 +1,11 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# platform driver
> > +snd-soc-mt8195-afe-objs := \
> > +       mt8195-afe-clk.o \
> > +       mt8195-afe-pcm.o \
> > +       mt8195-dai-adda.o \
> > +       mt8195-dai-etdm.o \
> > +       mt8195-dai-pcm.o
> > +
> > +obj-$(CONFIG_SND_SOC_MT8195) += snd-soc-mt8195-afe.o
> > diff --git a/sound/soc/mediatek/mt8195/mt8195-afe-clk.c
> > b/sound/soc/mediatek/mt8195/mt8195-afe-clk.c
> > new file mode 100644
> > index 000000000000..57aa799b4f41
> > --- /dev/null
> > +++ b/sound/soc/mediatek/mt8195/mt8195-afe-clk.c
> > @@ -0,0 +1,899 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * mt8195-afe-clk.c  --  Mediatek 8195 afe clock ctrl
> > + *
> > + * Copyright (c) 2021 MediaTek Inc.
> > + * Author: Bicycle Tsai <bicycle.tsai@mediatek.com>
> > + *         Trevor Wu <trevor.wu@mediatek.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +
> > +#include "mt8195-afe-common.h"
> > +#include "mt8195-afe-clk.h"
> > +#include "mt8195-reg.h"
> > +
> > +static const char *aud_clks[MT8195_CLK_NUM] = {
> 
> Most of these clocks are not described in the device tree binding. If
> the driver needs to reference them, they should be described. We
> should
> not be hard-coding clock names across different drivers.
> 
Sorry, I didn't know I have to list all clocks in the dt-binding.
Originally, I thought these clocks will be described in the clock
binding, so I didn't add them to the binding of afe driver.
I will add these clocks to mt8195-afe-pcm.yaml.

> The more important question is, why does the driver need to reference
> all of them? Maybe we should take a step back and draw out a clock
> tree
> diagram for the hardware?
> 
The clock structure is PLL -> MUX -> GATE.
xtal, pll and divider are the possible clock inputs for MUX.
Because we select the clock input of audio module based on the use
case, we use clk_get to retrive all clocks which are possible to be
used.
Some of them are not used in this series, because some modules are
still developing. Should I only keep the clocks that have been used in
the series?

> > +       /* xtal */
> > +       [MT8195_CLK_XTAL_26M] = "clk26m",
> > +       /* pll */
> > +       [MT8195_CLK_APMIXED_APLL1] = "apll1",
> > +       [MT8195_CLK_APMIXED_APLL2] = "apll2",
> > +       [MT8195_CLK_APMIXED_APLL3] = "apll3",
> > +       [MT8195_CLK_APMIXED_APLL4] = "apll4",
> > +       [MT8195_CLK_APMIXED_APLL5] = "apll5",
> > +       [MT8195_CLK_APMIXED_HDMIRX_APLL] = "hdmirx_apll",
> > +       /* divider */
> > +       [MT8195_CLK_TOP_APLL1] = "apll1_ck",
> > +       [MT8195_CLK_TOP_APLL1_D4] = "apll1_d4",
> > +       [MT8195_CLK_TOP_APLL2] = "apll2_ck",
> > +       [MT8195_CLK_TOP_APLL2_D4] = "apll2_d4",
> > +       [MT8195_CLK_TOP_APLL3] = "apll3_ck",
> > +       [MT8195_CLK_TOP_APLL3_D4] = "apll3_d4",
> > +       [MT8195_CLK_TOP_APLL4] = "apll4_ck",
> > +       [MT8195_CLK_TOP_APLL4_D4] = "apll4_d4",
> > +       [MT8195_CLK_TOP_APLL5] = "apll5_ck",
> > +       [MT8195_CLK_TOP_APLL5_D4] = "apll5_d4",
> > +       [MT8195_CLK_TOP_APLL12_DIV0] = "apll12_div0",
> > +       [MT8195_CLK_TOP_APLL12_DIV1] = "apll12_div1",
> > +       [MT8195_CLK_TOP_APLL12_DIV2] = "apll12_div2",
> > +       [MT8195_CLK_TOP_APLL12_DIV3] = "apll12_div3",
> > +       [MT8195_CLK_TOP_APLL12_DIV4] = "apll12_div4",
> > +       [MT8195_CLK_TOP_APLL12_DIV9] = "apll12_div9",
> > +       [MT8195_CLK_TOP_HDMIRX_APLL] = "hdmirx_apll_ck",
> > +       [MT8195_CLK_TOP_MAINPLL_D4_D4] = "mainpll_d4_d4",
> > +       [MT8195_CLK_TOP_MAINPLL_D5_D2] = "mainpll_d5_d2",
> > +       [MT8195_CLK_TOP_MAINPLL_D7_D2] = "mainpll_d7_d2",
> > +       [MT8195_CLK_TOP_UNIVPLL_D4] = "univpll_d4",
> > +       /* mux */
> > +       [MT8195_CLK_TOP_APLL1_SEL] = "apll1_sel",
> > +       [MT8195_CLK_TOP_APLL2_SEL] = "apll2_sel",
> > +       [MT8195_CLK_TOP_APLL3_SEL] = "apll3_sel",
> > +       [MT8195_CLK_TOP_APLL4_SEL] = "apll4_sel",
> > +       [MT8195_CLK_TOP_APLL5_SEL] = "apll5_sel",
> > +       [MT8195_CLK_TOP_A1SYS_HP_SEL] = "a1sys_hp_sel",
> > +       [MT8195_CLK_TOP_A2SYS_SEL] = "a2sys_sel",
> > +       [MT8195_CLK_TOP_A3SYS_SEL] = "a3sys_sel",
> > +       [MT8195_CLK_TOP_A4SYS_SEL] = "a4sys_sel",
> > +       [MT8195_CLK_TOP_ASM_H_SEL] = "asm_h_sel",
> > +       [MT8195_CLK_TOP_ASM_M_SEL] = "asm_m_sel",
> > +       [MT8195_CLK_TOP_ASM_L_SEL] = "asm_l_sel",
> > +       [MT8195_CLK_TOP_AUD_IEC_SEL] = "aud_iec_sel",
> > +       [MT8195_CLK_TOP_AUD_INTBUS_SEL] = "aud_intbus_sel",
> > +       [MT8195_CLK_TOP_AUDIO_H_SEL] = "audio_h_sel",
> > +       [MT8195_CLK_TOP_AUDIO_LOCAL_BUS_SEL] =
> > "audio_local_bus_sel",
> > +       [MT8195_CLK_TOP_DPTX_M_SEL] = "dptx_m_sel",
> > +       [MT8195_CLK_TOP_INTDIR_SEL] = "intdir_sel",
> > +       [MT8195_CLK_TOP_I2SO1_M_SEL] = "i2so1_m_sel",
> > +       [MT8195_CLK_TOP_I2SO2_M_SEL] = "i2so2_m_sel",
> > +       [MT8195_CLK_TOP_I2SI1_M_SEL] = "i2si1_m_sel",
> > +       [MT8195_CLK_TOP_I2SI2_M_SEL] = "i2si2_m_sel",
> > +       /* clock gate */
> > +       [MT8195_CLK_TOP_MPHONE_SLAVE_B] = "mphone_slave_b",
> > +       [MT8195_CLK_TOP_CFG_26M_AUD] = "cfg_26m_aud",
> > +       [MT8195_CLK_INFRA_AO_AUDIO] = "infra_ao_audio",
> > +       [MT8195_CLK_INFRA_AO_AUDIO_26M_B] = "infra_ao_audio_26m_b",
> > +       [MT8195_CLK_SCP_ADSP_AUDIODSP] = "scp_adsp_audiodsp",
> 
> 
> > +       [MT8195_CLK_AUD_AFE] = "aud_afe",
> > +       [MT8195_CLK_AUD_LRCK_CNT] = "aud_lrck_cnt",
> > +       [MT8195_CLK_AUD_SPDIFIN_TUNER_APLL] =
> > "aud_spdifin_tuner_apll",
> > +       [MT8195_CLK_AUD_SPDIFIN_TUNER_DBG] =
> > "aud_spdifin_tuner_dbg",
> > +       [MT8195_CLK_AUD_UL_TML] = "aud_ul_tml",
> > +       [MT8195_CLK_AUD_APLL1_TUNER] = "aud_apll1_tuner",
> > +       [MT8195_CLK_AUD_APLL2_TUNER] = "aud_apll2_tuner",
> > +       [MT8195_CLK_AUD_TOP0_SPDF] = "aud_top0_spdf",
> > +       [MT8195_CLK_AUD_APLL] = "aud_apll",
> > +       [MT8195_CLK_AUD_APLL2] = "aud_apll2",
> > +       [MT8195_CLK_AUD_DAC] = "aud_dac",
> > +       [MT8195_CLK_AUD_DAC_PREDIS] = "aud_dac_predis",
> > +       [MT8195_CLK_AUD_TML] = "aud_tml",
> > +       [MT8195_CLK_AUD_ADC] = "aud_adc",
> > +       [MT8195_CLK_AUD_DAC_HIRES] = "aud_dac_hires",
> > +       [MT8195_CLK_AUD_A1SYS_HP] = "aud_a1sys_hp",
> > +       [MT8195_CLK_AUD_AFE_DMIC1] = "aud_afe_dmic1",
> > +       [MT8195_CLK_AUD_AFE_DMIC2] = "aud_afe_dmic2",
> > +       [MT8195_CLK_AUD_AFE_DMIC3] = "aud_afe_dmic3",
> > +       [MT8195_CLK_AUD_AFE_DMIC4] = "aud_afe_dmic4",
> > +       [MT8195_CLK_AUD_AFE_26M_DMIC_TM] = "aud_afe_26m_dmic_tm",
> > +       [MT8195_CLK_AUD_UL_TML_HIRES] = "aud_ul_tml_hires",
> > +       [MT8195_CLK_AUD_ADC_HIRES] = "aud_adc_hires",
> > +       [MT8195_CLK_AUD_ADDA6_ADC] = "aud_adda6_adc",
> > +       [MT8195_CLK_AUD_ADDA6_ADC_HIRES] = "aud_adda6_adc_hires",
> > +       [MT8195_CLK_AUD_LINEIN_TUNER] = "aud_linein_tuner",
> > +       [MT8195_CLK_AUD_EARC_TUNER] = "aud_earc_tuner",
> > +       [MT8195_CLK_AUD_I2SIN] = "aud_i2sin",
> > +       [MT8195_CLK_AUD_TDM_IN] = "aud_tdm_in",
> > +       [MT8195_CLK_AUD_I2S_OUT] = "aud_i2s_out",
> > +       [MT8195_CLK_AUD_TDM_OUT] = "aud_tdm_out",
> > +       [MT8195_CLK_AUD_HDMI_OUT] = "aud_hdmi_out",
> > +       [MT8195_CLK_AUD_ASRC11] = "aud_asrc11",
> > +       [MT8195_CLK_AUD_ASRC12] = "aud_asrc12",
> > +       [MT8195_CLK_AUD_MULTI_IN] = "aud_multi_in",
> > +       [MT8195_CLK_AUD_INTDIR] = "aud_intdir",
> > +       [MT8195_CLK_AUD_A1SYS] = "aud_a1sys",
> > +       [MT8195_CLK_AUD_A2SYS] = "aud_a2sys",
> > +       [MT8195_CLK_AUD_PCMIF] = "aud_pcmif",
> > +       [MT8195_CLK_AUD_A3SYS] = "aud_a3sys",
> > +       [MT8195_CLK_AUD_A4SYS] = "aud_a4sys",
> > +       [MT8195_CLK_AUD_MEMIF_UL1] = "aud_memif_ul1",
> > +       [MT8195_CLK_AUD_MEMIF_UL2] = "aud_memif_ul2",
> > +       [MT8195_CLK_AUD_MEMIF_UL3] = "aud_memif_ul3",
> > +       [MT8195_CLK_AUD_MEMIF_UL4] = "aud_memif_ul4",
> > +       [MT8195_CLK_AUD_MEMIF_UL5] = "aud_memif_ul5",
> > +       [MT8195_CLK_AUD_MEMIF_UL6] = "aud_memif_ul6",
> > +       [MT8195_CLK_AUD_MEMIF_UL8] = "aud_memif_ul8",
> > +       [MT8195_CLK_AUD_MEMIF_UL9] = "aud_memif_ul9",
> > +       [MT8195_CLK_AUD_MEMIF_UL10] = "aud_memif_ul10",
> > +       [MT8195_CLK_AUD_MEMIF_DL2] = "aud_memif_dl2",
> > +       [MT8195_CLK_AUD_MEMIF_DL3] = "aud_memif_dl3",
> > +       [MT8195_CLK_AUD_MEMIF_DL6] = "aud_memif_dl6",
> > +       [MT8195_CLK_AUD_MEMIF_DL7] = "aud_memif_dl7",
> > +       [MT8195_CLK_AUD_MEMIF_DL8] = "aud_memif_dl8",
> > +       [MT8195_CLK_AUD_MEMIF_DL10] = "aud_memif_dl10",
> > +       [MT8195_CLK_AUD_MEMIF_DL11] = "aud_memif_dl11",
> > +       [MT8195_CLK_AUD_GASRC0] = "aud_gasrc0",
> > +       [MT8195_CLK_AUD_GASRC1] = "aud_gasrc1",
> > +       [MT8195_CLK_AUD_GASRC2] = "aud_gasrc2",
> > +       [MT8195_CLK_AUD_GASRC3] = "aud_gasrc3",
> > +       [MT8195_CLK_AUD_GASRC4] = "aud_gasrc4",
> > +       [MT8195_CLK_AUD_GASRC5] = "aud_gasrc5",
> > +       [MT8195_CLK_AUD_GASRC6] = "aud_gasrc6",
> > +       [MT8195_CLK_AUD_GASRC7] = "aud_gasrc7",
> > +       [MT8195_CLK_AUD_GASRC8] = "aud_gasrc8",
> > +       [MT8195_CLK_AUD_GASRC9] = "aud_gasrc9",
> > +       [MT8195_CLK_AUD_GASRC10] = "aud_gasrc10",
> > +       [MT8195_CLK_AUD_GASRC11] = "aud_gasrc11",
> > +       [MT8195_CLK_AUD_GASRC12] = "aud_gasrc12",
> > +       [MT8195_CLK_AUD_GASRC13] = "aud_gasrc13",
> > +       [MT8195_CLK_AUD_GASRC14] = "aud_gasrc14",
> > +       [MT8195_CLK_AUD_GASRC15] = "aud_gasrc15",
> > +       [MT8195_CLK_AUD_GASRC16] = "aud_gasrc16",
> > +       [MT8195_CLK_AUD_GASRC17] = "aud_gasrc17",
> > +       [MT8195_CLK_AUD_GASRC18] = "aud_gasrc18",
> > +       [MT8195_CLK_AUD_GASRC19] = "aud_gasrc19",
> 
> The MT8195_CLK_AUD_* clocks are all internal to the audio subsystem:
> the bits that control these clock gates are in the same address space
> as the audio parts. Would it be possible to model them as internal
> ASoC SUPPLY widgets? The external ones could be modeled using ASoC
> CLK_SUPPLY widgets, and the dependencies could be modeled with ASoC
> routes. The ASoC core could then handle power sequencing, which the
> driver currently does manually.
> 
> IMO this is better than having two drivers handling two aspects of
> the same piece of hardware, while the two aspects are intertwined.
> 

Yes, it's ok to use the CLK_SUPPLY and SUPPLY to model such clocks.
But those clocks are managed by CCF in the preceding SOCs like mt2701,
mt6779 and mt8183. Additionally, in some audio modules, clocks should
be enabled before configuring parameters(hw_params). As far as I know,
if we use CLK_SUPPLY or SUPPLY to model clocks, the power sequence is
controlled by DAPM. It seems to be impossible to fulfill all use cases.
That's why we just keep the manual control sequence and CCF seems to be
the best choice to model such clock gatess.

Thanks,
Trevor
> 
> Regards
> ChenYu

  reply	other threads:[~2021-07-12 15:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-29  1:47 [PATCH v2 0/8] ASoC: mediatek: Add support for MT8195 SoC Trevor Wu
2021-06-29  1:47 ` [PATCH v2 1/8] ASoC: mediatek: mt8195: update mediatek common driver Trevor Wu
2021-06-29  1:47 ` [PATCH v2 2/8] ASoC: mediatek: mt8195: support etdm in platform driver Trevor Wu
2021-06-29  1:47 ` [PATCH v2 3/8] ASoC: mediatek: mt8195: support adda " Trevor Wu
2021-06-29  1:47 ` [PATCH v2 4/8] ASoC: mediatek: mt8195: support pcm " Trevor Wu
2021-06-29  1:47 ` [PATCH v2 5/8] ASoC: mediatek: mt8195: add " Trevor Wu
2021-07-12  6:57   ` Chen-Yu Tsai
2021-07-12 15:09     ` Trevor Wu [this message]
2021-07-13  6:00       ` Chen-Yu Tsai
2021-07-15 11:05         ` Trevor Wu
2021-07-19 10:05           ` Chen-Yu Tsai
2021-07-22  8:56             ` Trevor Wu
2021-07-23  6:27               ` Chen-Yu Tsai
2021-07-26 14:31                 ` Trevor Wu
2021-08-02 10:21                   ` Chen-Yu Tsai
2021-08-03 10:12                     ` Trevor Wu
2021-06-29  1:47 ` [PATCH v2 6/8] dt-bindings: mediatek: mt8195: add audio afe document Trevor Wu
2021-07-01 20:18   ` Rob Herring
2021-07-05  7:01     ` Trevor Wu
2021-06-29  1:47 ` [PATCH v2 7/8] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1019 and rt5682 Trevor Wu
2021-06-29  1:47 ` [PATCH v2 8/8] dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1019-rt5682 document Trevor Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9929a3a20df1a89fc94baf7c75c0c65d9a61de0f.camel@mediatek.com \
    --to=trevor.wu@mediatek.com \
    --cc=aaronyu@google.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=bicycle.tsai@mediatek.com \
    --cc=broonie@kernel.org \
    --cc=cychiang@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jiaxin.yu@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=tiwai@suse.com \
    --cc=wenst@chromium.org \
    --subject='Re: [PATCH v2 5/8] ASoC: mediatek: mt8195: add platform driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).