| Message ID | 20250514081125.24475-9-darren.ye@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: mediatek: Add support for MT8196 SoC | expand |
On 14/05/2025 12:54, Krzysztof Kozlowski wrote: > On 14/05/2025 10:11, Darren.Ye wrote: >> From: Darren Ye <darren.ye@mediatek.com> >> >> Add mt8196 audio AFE document. > > A nit, subject: drop second/last, redundant "document". > See also: > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 Since you asked what did you miss from previous review (and I replied that EVERYTHING) then maybe this will help: How did you implement this? > >> >> Signed-off-by: Darren Ye <darren.ye@mediatek.com> > > > ... > >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - mediatek,vlpcksys >> + - power-domains >> + - memory-region >> + - clocks >> + - clock-names > > > Keep the same order as in properties:. How did you implement this? > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> How did you implement this? > > <form letter> > This is an automated instruction, just in case, because many review tags > are being ignored. If you know the process, you can skip it (please do > not feel offended by me posting it here - no bad intentions intended). > If you do not know the process, here is a short explanation: > > Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions > of patchset, under or above your Signed-off-by tag, unless patch changed > significantly (e.g. new properties added to the DT bindings). Tag is > "received", when provided in a message replied to you on the mailing > list. Tools like b4 can help here. However, there's no need to repost > patches *only* to add the tags. The upstream maintainer will do that for > tags received on the version they apply. > > Full context and explanation: > https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577 How did you implement this? Does such questions help you to find what was missing? Best regards, Krzysztof
On Wed, 2025-06-11 at 11:32 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 14/05/2025 12:54, Krzysztof Kozlowski wrote: > > On 14/05/2025 10:11, Darren.Ye wrote: > > > From: Darren Ye <darren.ye@mediatek.com> > > > > > > Add mt8196 audio AFE document. > > > > A nit, subject: drop second/last, redundant "document". > > See also: > > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst*L18__;Iw!!CTRNKA9wMg0ARbw!l3YASE64DTMTp8bOMry9UyBMf-IcYFTvXOlP2DXfpeUEFdQK79xGYEnsxnJuMihVX7_WhEe3q1evSg$ > > Since you asked what did you miss from previous review (and I replied > that EVERYTHING) then maybe this will help: > > How did you implement this? > My mistake, please forgive my carelessness. I will change it to ASoC: dt-bindings: mediatek,mt8196-afe: Add suport for MT8196 audio AFE controller THis patch adds initial supoprt for the audio AFE(Audio Front End) controller on the Mediatek MT8196 platform. > > > > > > > > Signed-off-by: Darren Ye <darren.ye@mediatek.com> > > > > > > ... > > > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - interrupts > > > + - mediatek,vlpcksys > > > + - power-domains > > > + - memory-region > > > + - clocks > > > + - clock-names > > > > > > Keep the same order as in properties:. > > How did you implement this? > I have already made the change here. The "required" fields have been sorted according to the order of the properties. > > > > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > How did you implement this? Sorry, I got it now. I will add it to the commit. > > > > > > <form letter> > > This is an automated instruction, just in case, because many review > > tags > > are being ignored. If you know the process, you can skip it (please > > do > > not feel offended by me posting it here - no bad intentions > > intended). > > If you do not know the process, here is a short explanation: > > > > Please add Acked-by/Reviewed-by/Tested-by tags when posting new > > versions > > of patchset, under or above your Signed-off-by tag, unless patch > > changed > > significantly (e.g. new properties added to the DT bindings). Tag > > is > > "received", when provided in a message replied to you on the > > mailing > > list. Tools like b4 can help here. However, there's no need to > > repost > > patches *only* to add the tags. The upstream maintainer will do > > that for > > tags received on the version they apply. > > > > Full context and explanation: > > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst*L577__;Iw!!CTRNKA9wMg0ARbw!l3YASE64DTMTp8bOMry9UyBMf-IcYFTvXOlP2DXfpeUEFdQK79xGYEnsxnJuMihVX7_WhEcicLV6xg$ > > How did you implement this? Got it. > > Does such questions help you to find what was missing? > > Best regards, > Krzysztof I'm very sorry, this is my first time upstreaming. Thank you for your patient guidance. Best Regards, Darren
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..384a97ab5f4e --- /dev/null +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml @@ -0,0 +1,155 @@ +# 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@mediatek.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 + + 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 + - const: apll12_div_i2sin0 + - const: apll12_div_i2sin1 + - const: apll12_div_fmi2s + - const: apll12_div_tdmout_m + - const: apll12_div_tdmout_b + - const: top_apll1 + - const: top_apll2 + - const: top_i2sin0 + - const: top_i2sin1 + - const: top_fmi2s + - const: top_tdmout + - const: top_adsp + - const: clk26m + +required: + - compatible + - reg + - interrupts + - 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@1a110000 { + 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"; + }; + };