| Message ID | 20240226-audio-i350-v3-1-16bb2c974c55@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add audio support for the MediaTek Genio 350-evk board | expand |
On 09/04/2024 15:41, Alexandre Mergnat wrote: > Add MT8365 audio front-end bindings > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- > +properties: > + compatible: > + const: mediatek,mt8365-afe-pcm > + > + reg: > + maxItems: 1 > + > + "#sound-dai-cells": > + const: 0 > + > + clocks: > + items: > + - description: 26M clock > + - description: mux for audio clock > + - description: audio i2s0 mck > + - description: audio i2s1 mck > + - description: audio i2s2 mck > + - description: audio i2s3 mck > + - description: engen 1 clock > + - description: engen 2 clock > + - description: audio 1 clock > + - description: audio 2 clock > + - description: mux for i2s0 > + - description: mux for i2s1 > + - description: mux for i2s2 > + - description: mux for i2s3 > + > + clock-names: > + items: > + - const: top_clk26m_clk > + - const: top_audio_sel > + - const: audio_i2s0_m > + - const: audio_i2s1_m > + - const: audio_i2s2_m > + - const: audio_i2s3_m > + - const: engen1 > + - const: engen2 > + - const: aud1 > + - const: aud2 > + - const: i2s0_m_sel > + - const: i2s1_m_sel > + - const: i2s2_m_sel > + - const: i2s3_m_sel > + > + interrupts: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + mediatek,dmic-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Indicates how many data pins are used to transmit two channels of PDM > + signal. 1 means two wires, 0 means one wire. Default value is 0. > + enum: > + - 0 # one wire > + - 1 # two wires > + > + mediatek,topckgen: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: The phandle of the mediatek topckgen controller Nothing improved, so again, so something which is not obvious. What is it used for? Why AFE needs topckgen for example? > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - interrupts > + - power-domains > + - mediatek,topckgen > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/mediatek,mt8365-clk.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/power/mediatek,mt8365-power.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + afe@11220000 { Did you implement the comment or decided to keep afe? BTW, whatever "consistency" you have in mind, it does not really matter that much for that example. And for sure do not add incorrect code intentionally just to fix it in next patch. Best regards, Krzysztof
On 09/04/2024 17:46, Krzysztof Kozlowski wrote: >> + soc { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + afe@11220000 { > Did you implement the comment or decided to keep afe? > Though it was clear according to [1]: " Audio Front End, this is the same name used for other MTK SoC, to be consistent. Cook a new patch serie to change "afe" by "audio-controller" for all MTK SoC would be great. " I want to keep it and fix it later with ALL other MTK SoC. You didn't answer after that, I though it was ok for you... So if you don't agree with that, just tell me to change it instead of let me think I've the choice. > BTW, whatever "consistency" you have in mind, it does not really matter > that much for that example. And for sure do not add incorrect code > intentionally just to fix it in next patch. I don't get your point. What do you mean by "do not add incorrect code intentionally" please ??? My PoV: I implement my bindings aligned to ALL other MTK SoC, which have been validated by you in the past. I can understand the copied code is wrong, but I've suggested a solution and you didn't NACK it. I fell like you bully me. Are you ok to stop insinuating bad things please and tell me directly if a fix is mandatory and if you are waiting for a specific one, tell me directly to avoid this kind of unpleasant talk and waste of time, I know maintainers are busy persons. Beside that, thanks for your reviews. [1]: https://lore.kernel.org/all/eeb3329b-0558-4237-8343-4e11f65a6a35@baylibre.com/
On 10/04/2024 11:29, Alexandre Mergnat wrote: > > > On 09/04/2024 17:46, Krzysztof Kozlowski wrote: >>> + soc { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + afe@11220000 { >> Did you implement the comment or decided to keep afe? >> > > Though it was clear according to [1]: > " > Audio Front End, this is the same name used for other MTK SoC, to be > consistent. > > Cook a new patch serie to change "afe" by "audio-controller" for all MTK > SoC would be great. > " > > I want to keep it and fix it later with ALL other MTK SoC. > You didn't answer after that, I though it was ok for you... Then no, I don't agree. If you add code, which you already plan to fix, it means the code is not correct somehow. Then just add correct code in the beginning. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml new file mode 100644 index 000000000000..d0759898b9c5 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml @@ -0,0 +1,136 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-afe.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek AFE PCM controller for MT8365 + +maintainers: + - Alexandre Mergnat <amergnat@baylibre.com> + +properties: + compatible: + const: mediatek,mt8365-afe-pcm + + reg: + maxItems: 1 + + "#sound-dai-cells": + const: 0 + + clocks: + items: + - description: 26M clock + - description: mux for audio clock + - description: audio i2s0 mck + - description: audio i2s1 mck + - description: audio i2s2 mck + - description: audio i2s3 mck + - description: engen 1 clock + - description: engen 2 clock + - description: audio 1 clock + - description: audio 2 clock + - description: mux for i2s0 + - description: mux for i2s1 + - description: mux for i2s2 + - description: mux for i2s3 + + clock-names: + items: + - const: top_clk26m_clk + - const: top_audio_sel + - const: audio_i2s0_m + - const: audio_i2s1_m + - const: audio_i2s2_m + - const: audio_i2s3_m + - const: engen1 + - const: engen2 + - const: aud1 + - const: aud2 + - const: i2s0_m_sel + - const: i2s1_m_sel + - const: i2s2_m_sel + - const: i2s3_m_sel + + interrupts: + maxItems: 1 + + power-domains: + maxItems: 1 + + mediatek,dmic-mode: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Indicates how many data pins are used to transmit two channels of PDM + signal. 1 means two wires, 0 means one wire. Default value is 0. + enum: + - 0 # one wire + - 1 # two wires + + mediatek,topckgen: + $ref: /schemas/types.yaml#/definitions/phandle + description: The phandle of the mediatek topckgen controller + +required: + - compatible + - reg + - clocks + - clock-names + - interrupts + - power-domains + - mediatek,topckgen + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/mediatek,mt8365-clk.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/power/mediatek,mt8365-power.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + afe@11220000 { + compatible = "mediatek,mt8365-afe-pcm"; + reg = <0 0x11220000 0 0x1000>; + #sound-dai-cells = <0>; + clocks = <&clk26m>, + <&topckgen CLK_TOP_AUDIO_SEL>, + <&topckgen CLK_TOP_AUD_I2S0_M>, + <&topckgen CLK_TOP_AUD_I2S1_M>, + <&topckgen CLK_TOP_AUD_I2S2_M>, + <&topckgen CLK_TOP_AUD_I2S3_M>, + <&topckgen CLK_TOP_AUD_ENGEN1_SEL>, + <&topckgen CLK_TOP_AUD_ENGEN2_SEL>, + <&topckgen CLK_TOP_AUD_1_SEL>, + <&topckgen CLK_TOP_AUD_2_SEL>, + <&topckgen CLK_TOP_APLL_I2S0_SEL>, + <&topckgen CLK_TOP_APLL_I2S1_SEL>, + <&topckgen CLK_TOP_APLL_I2S2_SEL>, + <&topckgen CLK_TOP_APLL_I2S3_SEL>; + clock-names = "top_clk26m_clk", + "top_audio_sel", + "audio_i2s0_m", + "audio_i2s1_m", + "audio_i2s2_m", + "audio_i2s3_m", + "engen1", + "engen2", + "aud1", + "aud2", + "i2s0_m_sel", + "i2s1_m_sel", + "i2s2_m_sel", + "i2s3_m_sel"; + interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_LOW>; + power-domains = <&spm MT8365_POWER_DOMAIN_AUDIO>; + mediatek,dmic-mode = <1>; + mediatek,topckgen = <&topckgen>; + }; + }; + +...
Add MT8365 audio front-end bindings Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> --- .../bindings/sound/mediatek,mt8365-afe.yaml | 136 +++++++++++++++++++++ 1 file changed, 136 insertions(+)