lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXv+5F0Ys63hcO8u7p3zSnnOT4gYc2Z0BhQW=dOXAvBc_nmvg@mail.gmail.com>
Date: Wed, 16 Apr 2025 16:34:11 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: "Darren.Ye" <darren.ye@...iatek.com>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Jaroslav Kysela <perex@...ex.cz>, 
	Takashi Iwai <tiwai@...e.com>, Linus Walleij <linus.walleij@...aro.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, linux-sound@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-mediatek@...ts.infradead.org, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v2 09/11] ASoC: dt-bindings: mediatek,mt8196-afe: add
 audio AFE document

On Mon, Apr 7, 2025 at 8:39 PM Darren.Ye <darren.ye@...iatek.com> wrote:
>
> From: Darren Ye <darren.ye@...iatek.com>
>
> Add mt8196 audio AFE document.
>
> Signed-off-by: Darren Ye <darren.ye@...iatek.com>
> ---
>  .../bindings/sound/mediatek,mt8196-afe.yaml   | 233 ++++++++++++++++++
>  1 file changed, 233 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml
> new file mode 100644
> index 000000000000..44f8847b13a8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml
> @@ -0,0 +1,233 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mediatek,mt8196-afe.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Audio Front End PCM controller for MT8196
> +
> +maintainers:
> +  - Darren Ye <darren.ye@...iatek.com>
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8196-afe-pcm
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +  memory-region:
> +    maxItems: 1
> +    description: |
> +      Shared memory region for AFE memif.  A "shared-dma-pool".
> +      See dtschema reserved-memory/shared-dma-pool.yaml for details.
> +  mediatek,cksys:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: The phandle of the mediatek clk systemd controller
> +
> +  mediatek,vlpcksys:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: The phandle of the mediatek vlpcksys controller
> +  power-domains:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: audio hopping clock gate
> +      - description: audio f26m clock gate
> +      - description: audio apll1 clock gate
> +      - description: audio apll2 clock gate
> +      - description: audio apll1 tuner gate
> +      - description: audio apll2 tuner gate
> +      - description: mux for audio vlp int
> +      - description: mux for audio vlp engen1
> +      - description: mux for audio vlp engen2
> +      - description: mux for audio h
> +      - description: vlp clock 26m
> +      - description: audio mainpll divide 4
> +      - description: mux for audio apll1
> +      - description: audio apll1
> +      - description: mux for audio apll2
> +      - description: audio apll2
> +      - description: audio apll1 divide 4
> +      - description: audio apll2 divide 4
> +      - description: mux for i2sin0 mck
> +      - description: mux for i2sin1 mck
> +      - description: mux for fmi2s mck
> +      - description: mux for tdmout mck
> +      - description: auido apll12 divide for i2sin0
> +      - description: auido apll12 divide for i2sin1
> +      - description: auido apll12 divide for fmi2s
> +      - description: auido apll12 divide for tdmout mck
> +      - description: auido apll12 divide for tdmout bck

There's a bunch of typos for "audio".

> +      - description: audio adsp clk
> +      - description: 26m clock

Can we look into trimming down the list of clocks? Ideally this should
only list the actual clock inputs of the hardware, which are normally
the leaf clocks from the clock controller. You should not have to list
all the intermediate dividers and muxes.

On the Linux implementation side, it should be a matter of calling
clk_set_rate() on the clock input corresponding to the interface in
use. If the clock is not resolving the correct clock rate / parenting,
the clock driver should be fixed.

> +
> +  clock-names:
> +    items:
> +      - const: aud_hopping_clk
> +      - const: aud_f26m_clk
> +      - const: aud_apll1_clk
> +      - const: aud_apll2_clk
> +      - const: aud_apll_tuner1_clk
> +      - const: aud_apll_tuner2_clk
> +      - const: vlp_mux_audio_int
> +      - const: vlp_mux_aud_eng1
> +      - const: vlp_mux_aud_eng2
> +      - const: vlp_mux_audio_h
> +      - const: vlp_clk26m_clk
> +      - const: ck_mainpll_d4_d4
> +      - const: ck_mux_aud_1
> +      - const: ck_apll1_ck
> +      - const: ck_mux_aud_2
> +      - const: ck_apll2_ck
> +      - const: ck_apll1_d4
> +      - const: ck_apll2_d4
> +      - const: ck_i2sin0_m_sel
> +      - const: ck_i2sin1_m_sel
> +      - const: ck_fmi2s_m_sel
> +      - const: ck_tdmout_m_sel
> +      - const: ck_apll12_div_i2sin0
> +      - const: ck_apll12_div_i2sin1
> +      - const: ck_apll12_div_fmi2s
> +      - const: ck_apll12_div_tdmout_m
> +      - const: ck_apll12_div_tdmout_b
> +      - const: ck_adsp_sel
> +      - const: ck_clk26m_clk
> +
> +  mediatek,etdm4-out-ch:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Number of ETDM4 output channels.
> +    minimum: 1
> +    maximum: 8
> +
> +  mediatek,etdm4-in-ch:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Number of ETDM4 input channels.
> +    minimum: 1
> +    maximum: 8
> +
> +  mediatek,etdm4-out-sync:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      ETDM4 output and input enable synchronization.
> +    enum:
> +      - 0 # Enable controlled by itself
> +      - 1 # Enable synchronization with ETDM4 input.
> +
> +  mediatek,etdm4-in-sync:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      ETDM4 input and outpuot enable synchronization.
> +    enum:
> +      - 0 # Enable controlled by itself
> +      - 1 # Enable synchronization with ETDM4 output.
> +
> +
> +
> +  mediatek,etdm4-ip-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: ETDM IP mode.
> +    enum:
> +      - 0 # One ip multi-ch mode
> +      - 1 # Multi-ip 2ch mode
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - mediatek,cksys
> +  - mediatek,vlpcksys
> +  - power-domains
> +  - memory-region
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        afe: mt8196-afe-pcm@...10000 {
> +            compatible = "mediatek,mt8196-afe-pcm";
> +            reg = <0 0x1a110000 0 0x9000>;
> +            interrupts = <GIC_SPI 351 IRQ_TYPE_LEVEL_HIGH 0>;
> +            memory-region = <&afe_dma_mem_reserved>;
> +            mediatek,cksys = <&cksys_clk>;
> +            mediatek,vlpcksys = <&vlp_cksys_clk>;
> +            power-domains = <&scpsys 14>; //MT8196_POWER_DOMAIN_AUDIO
> +            mediatek,etdm4-out-ch = <2>;
> +            mediatek,etdm4-in-ch = <2>;
> +            mediatek,etdm4-out-sync = <0>;
> +            mediatek,etdm4-in-sync = <1>;
> +            mediatek,etdm4-ip-mode = <0>;
> +            clocks = <&afe_clk 109>, //CLK_AFE_AUDIO_HOPPING_AFE
> +                     <&afe_clk 111>, //CLK_AFE_AUDIO_F26M_AFE
> +                     <&afe_clk 113>, //CLK_AFE_APLL1_AFE
> +                     <&afe_clk 115>, //CLK_AFE_APLL2_AFE
> +                     <&afe_clk 121>, //CLK_AFE_APLL_TUNER1_AFE
> +                     <&afe_clk 119>, //CLK_AFE_APLL_TUNER2_AFE

<&afe_clk> has the same register range as <&afe>, i.e they are the
same hardware block. Please find a way to internalize them in the
driver implementation and drop them from the bindings / DT.

> +                     <&vlp_cksys_clk 40>, //CLK_VLP_CK_AUD_INTBUS_SEL
> +                     <&vlp_cksys_clk 38>, //CLK_VLP_CK_AUD_ENGEN1_SEL
> +                     <&vlp_cksys_clk 39>, //CLK_VLP_CK_AUD_ENGEN2_SEL
> +                     <&vlp_cksys_clk 37>, //CLK_VLP_CK_AUDIO_H_SEL
> +                     <&vlp_cksys_clk 45>, //CLK_VLP_CK_CLKSQ
> +                     <&cksys_clk 98>, //CLK_CK_MAINPLL_D4_D4

This is not actually used in the implementation.

> +                     <&cksys_clk 43>, //CLK_CK_AUD_1_SEL

Intermediate clock feeding into the I2S / TDM clocks. Does this
really feed into the audio block?

> +                     <&cksys_clk 129>, //CLK_CK_APLL1

PLL source clock.

> +                     <&cksys_clk 44>, //CLK_CK_AUD_2_SEL

Intermediate clock feeding into the I2S / TDM clocks. Does this
really feed into the audio block?

> +                     <&cksys_clk 132>, //CLK_CK_APLL2

PLL source clock.

> +                     <&cksys_clk 130>, //CLK_CK_APLL1_D4

Divider after PLL.

> +                     <&cksys_clk 133>, //CLK_CK_APLL2_D4

Divider after PLL.

> +                     <&cksys_clk 66>, //CLK_CK_APLL_I2SIN0_MCK_SEL
> +                     <&cksys_clk 67>, //CLK_CK_APLL_I2SIN1_MCK_SEL
> +                     <&cksys_clk 78>, //CLK_CK_APLL_FMI2S_MCK_SEL
> +                     <&cksys_clk 79>, //CLK_CK_APLL_TDMOUT_MCK_SEL

These four feed into the next four.

Please take a good look at the hardware and determine which ones are
actually directly used by the hardware.


ChenYu

> +                     <&cksys_clk 80>, //CLK_CK_APLL12_CK_DIV_I2SIN0
> +                     <&cksys_clk 81>, //CLK_CK_APLL12_CK_DIV_I2SIN1
> +                     <&cksys_clk 92>, //CLK_CK_APLL12_CK_DIV_FMI2S
> +                     <&cksys_clk 93>, //CLK_CK_APLL12_CK_DIV_TDMOUT_M
> +                     <&cksys_clk 94>, //CLK_CK_APLL12_CK_DIV_TDMOUT_B
> +                     <&cksys_clk 45>, //CLK_CK_ADSP_SEL
> +                     <&cksys_clk 140>; //CLK_CK_TCK_26M_MX9
> +            clock-names = "aud_hopping_clk",
> +                          "aud_f26m_clk",
> +                          "aud_apll1_clk",
> +                          "aud_apll2_clk",
> +                          "aud_apll_tuner1_clk",
> +                          "aud_apll_tuner2_clk",
> +                          "vlp_mux_audio_int",
> +                          "vlp_mux_aud_eng1",
> +                          "vlp_mux_aud_eng2",
> +                          "vlp_mux_audio_h",
> +                          "vlp_clk26m_clk",
> +                          "ck_mainpll_d4_d4",
> +                          "ck_mux_aud_1",
> +                          "ck_apll1_ck",
> +                          "ck_mux_aud_2",
> +                          "ck_apll2_ck",
> +                          "ck_apll1_d4",
> +                          "ck_apll2_d4",
> +                          "ck_i2sin0_m_sel",
> +                          "ck_i2sin1_m_sel",
> +                          "ck_fmi2s_m_sel",
> +                          "ck_tdmout_m_sel",
> +                          "ck_apll12_div_i2sin0",
> +                          "ck_apll12_div_i2sin1",
> +                          "ck_apll12_div_fmi2s",
> +                          "ck_apll12_div_tdmout_m",
> +                          "ck_apll12_div_tdmout_b",
> +                          "ck_adsp_sel",
> +                          "ck_clk26m_clk";
> +        };
> +    };
> --
> 2.45.2
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ