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+5EufDuxLMnwMaCqtWFZpVMNMxi-5OwCyO4a+KD2T+2NYA@mail.gmail.com>
Date: Tue, 15 Jul 2025 13:09:55 +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, 
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH v6 08/10] ASoC: dt-bindings: mediatek,mt8196-afe: add
 audio AFE

Hi,

On Tue, Jul 8, 2025 at 7:35 PM Darren.Ye <darren.ye@...iatek.com> wrote:
>
> From: Darren Ye <darren.ye@...iatek.com>
>
> Add mt8196 audio AFE.
>
> Signed-off-by: Darren Ye <darren.ye@...iatek.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> ---
>  .../bindings/sound/mediatek,mt8196-afe.yaml   | 157 ++++++++++++++++++
>  1 file changed, 157 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..fe147eddf5e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml
> @@ -0,0 +1,157 @@
> +# 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
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  memory-region:
> +    maxItems: 1
> +
> +  mediatek,vlpcksys:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: To set up the apll12 tuner

Looking at the implementation, the configuration is just a fixed value.
Can this be moved to the VLP clock driver instead?

> +
> +  power-domains:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: mux for audio intbus
> +      - description: mux for audio engen1
> +      - description: mux for audio engen2
> +      - description: mux for audio h
> +      - description: vlp 26m clock
> +      - description: audio apll1 clock
> +      - description: audio apll2 clock
> +      - description: audio apll1 divide4
> +      - description: audio apll2 divide4
> +      - description: audio apll12 divide for i2sin0
> +      - description: audio apll12 divide for i2sin1
> +      - description: audio apll12 divide for fmi2s
> +      - description: audio apll12 divide for tdmout mck
> +      - description: audio apll12 divide for tdmout bck
> +      - description: mux for audio apll1
> +      - description: mux for audio apll2
> +      - description: mux for i2sin0 mck
> +      - description: mux for i2sin1 mck
> +      - description: mux for fmi2s mck
> +      - description: mux for tdmout mck
> +      - description: mux for adsp clock
> +      - description: 26m clock
> +
> +  clock-names:
> +    items:
> +      - const: top_aud_intbus
> +      - const: top_aud_eng1
> +      - const: top_aud_eng2
> +      - const: top_aud_h
> +      - const: vlp_clk26m

> +      - const: apll1
> +      - const: apll2
> +      - const: apll1_d4
> +      - const: apll2_d4

These are parents of the top_apll[12]. They do not feed into the
hardware directly, so you should not be including them here.

> +      - const: apll12_div_i2sin0
> +      - const: apll12_div_i2sin1
> +      - const: apll12_div_fmi2s
> +      - const: apll12_div_tdmout_m
> +      - const: apll12_div_tdmout_b

In the clock bindings sent by Collabora, these dividers are no longer
separately modeled; they have been combined with their respective
top_* clocks.

> +      - const: top_apll1
> +      - const: top_apll2

These two are parents to apll12_div_*, do not feed into the hardware
directly, so you should not be including them here.

The clock tree for each audio interface clock looks like the following:

    apll1 -> apll1_d4 -> top_apll1 --
                     /               \
              clk26m                  --> top_fmi2s -> apll12_div_fmi2s
                     \               /
    apll2 -> apll2_d4 -> top_apll2 --

Only the final "apll12_div_fmi2s" should be referenced.

On the implementation side, it should simply be a matter of setting the
required rate (24.576 MHz or 22.5792 MHz, or some multiple) on this leaf
clock, and let the clock framework figure out the PLL and dividers to
use. Same thing for enabling the clock.

> +      - const: top_i2sin0
> +      - const: top_i2sin1
> +      - const: top_fmi2s
> +      - const: top_tdmout
> +      - const: top_adsp

> +      - const: clk26m

Is this one directly needed? It is similar to vlp_clk26m, and I suspect
only that one is needed.


ChenYu

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - memory-region
> +  - mediatek,vlpcksys
> +  - power-domains
> +  - 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@...10000 {
> +            compatible = "mediatek,mt8196-afe";
> +            reg = <0 0x1a110000 0 0x9000>;
> +            interrupts = <GIC_SPI 351 IRQ_TYPE_LEVEL_HIGH 0>;
> +            memory-region = <&afe_dma_mem_reserved>;
> +            mediatek,vlpcksys = <&vlp_cksys_clk>;
> +            power-domains = <&scpsys 14>; //MT8196_POWER_DOMAIN_AUDIO
> +            clocks = <&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 129>, //CLK_CK_APLL1
> +                     <&cksys_clk 132>, //CLK_CK_APLL2
> +                     <&cksys_clk 130>, //CLK_CK_APLL1_D4
> +                     <&cksys_clk 133>, //CLK_CK_APLL2_D4
> +                     <&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 43>, //CLK_CK_AUD_1_SEL
> +                     <&cksys_clk 44>, //CLK_CK_AUD_2_SEL
> +                     <&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
> +                     <&cksys_clk 45>, //CLK_CK_ADSP_SEL
> +                     <&cksys_clk 140>; //CLK_CK_TCK_26M_MX9
> +            clock-names = "top_aud_intbus",
> +                          "top_aud_eng1",
> +                          "top_aud_eng2",
> +                          "top_aud_h",
> +                          "vlp_clk26m",
> +                          "apll1",
> +                          "apll2",
> +                          "apll1_d4",
> +                          "apll2_d4",
> +                          "apll12_div_i2sin0",
> +                          "apll12_div_i2sin1",
> +                          "apll12_div_fmi2s",
> +                          "apll12_div_tdmout_m",
> +                          "apll12_div_tdmout_b",
> +                          "top_apll1",
> +                          "top_apll2",
> +                          "top_i2sin0",
> +                          "top_i2sin1",
> +                          "top_fmi2s",
> +                          "top_tdmout",
> +                          "top_adsp",
> +                          "clk26m";
> +        };
> +    };
> +
> +...
> --
> 2.45.2
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ