Re: [PATCH 12/14] dt-bindings: mediatek: mt8196: add audio AFE document
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-03-07 15:15:43
Also in:
linux-devicetree, linux-gpio, linux-mediatek, linux-sound, lkml
On 07/03/2025 13:47, Darren.Ye wrote:
From: Darren Ye <redacted> Add mt8196 audio AFE document. Signed-off-by: Darren Ye <redacted>
Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
quoted hunk ↗ jump to hunk
--- .../bindings/sound/mediatek,mt8196-afe.yaml | 259 ++++++++++++++++++ 1 file changed, 259 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yamldiff --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..59f8fdf3167c --- /dev/null +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml@@ -0,0 +1,259 @@ +# 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-pcm + + reg: + maxItems: 1 + + "#sound-dai-cells": + const: 0 + + clocks: + items: + - description: audio hopping clock + - description: audio f26m clock + - description: audio ul0 adc clock + - description: audio ul0 adc hires clock + - description: audio ul1 adc clock + - description: audio ul1 adc hires clock + - description: audio apll1 clock + - description: audio apll2 clock + - description: audio apll tuner1 clock + - description: audio apll tuner2 clock + - description: vlp mux audio int + - description: vlp mux aud engen1 + - description: vlp mux aud engen2 + - description: vlp mux audio h + - description: vlp clock 26m + - description: ck mainpll d4 d4 + - description: ck mux aud 1 + - description: ck apll1 + - description: ck mux aud 2 + - description: ck apll2 + - description: ck apll1 d4 + - description: ck apll2 d4 + - description: ck i2sin0 m sel + - description: ck i2sin1 m sel + - description: ck fmi2s m sel + - description: ck tdmout m sel + - description: ck apll12 div i2sin0 + - description: ck apll12 div i2sin1 + - description: ck apll12 div fmi2s + - description: ck apll12 div tdmout m + - description: ck apll12 div tdmout b + - description: ck adsp sel + - description: ck clock 26m + + clock-names: + items: + - const: aud_hopping_clk
Look how other bindings call it. s/_clk//
+ - const: aud_f26m_clk + - const: aud_ul0_adc_clk + - const: aud_ul0_adc_hires_clk + - const: aud_ul1_adc_clk + - const: aud_ul1_adc_hires_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
What does ck stand for? You should name and explain the clocks based on this block, not the source.
+ - 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
s/ck// s/clk// and this goes probably first. Look at other bindings.
+ + interrupts: + maxItems: 1 + + power-domains: + maxItems: 1 + + cksys:
Again, open existing bindings.
+ $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to the cksys clock controller.
This tell me not much. Why do you need it? Drop redundant 'Phandle to' and explain how it is used.
+ + vlpcksys: + $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to the vlpcksys clock controller.
No, because you keep encoding clock information via non-clock API.
+ + memory-region: + $ref: /schemas/types.yaml#/definitions/phandle
Drop, see other bindings.
+ description: Phandle to the reserved memory region for AFE DMA. + + pinctrl-names:
Drop
+ items: + - const: default + + pinctrl-0:
Drop
+ $ref: /schemas/types.yaml#/definitions/phandle + description: Phandle to the pin control group for default state. + + mediatek,etdm-out-ch: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Number of ETDM output channels. + enum: [2]
That's pointless.
+ + mediatek,etdm-in-ch: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Number of ETDM input channels. + enum: [2] + + mediatek,etdm-out-sync: + $ref: /schemas/types.yaml#/definitions/uint32 + description: ETDM output synchronization. + enum: [0, 1] + + mediatek,etdm-in-sync: + $ref: /schemas/types.yaml#/definitions/uint32 + description: ETDM input synchronization. + enum: [0, 1] + + mediatek,etdm-ip-mode: + $ref: /schemas/types.yaml#/definitions/uint32 + description: ETDM IP mode. + enum: [0, 1]
Drop all above properties or explain why they make any sense in the terms of board configuration.
+ +required: + - compatible + - reg + - clocks + - clock-names + - interrupts + - power-domains + - cksys + - vlpcksys + - memory-region
+ - pinctrl-names + - pinctrl-0
Why?
+ - mediatek,etdm-out-ch
+ - mediatek,etdm-in-ch
+ - mediatek,etdm-out-sync
+ - mediatek,etdm-in-sync
+ - mediatek,etdm-ip-mode
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mt8196-clk.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/power/mt8196-power.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ afe: mt8196-afe-pcm@1a110000 {Again... look at other bindings. Best regards, Krzysztof