| Message ID | 20240226-audio-i350-v1-0-4fa1cea1667f@baylibre.com |
|---|---|
| Headers | show |
| Series | Add audio support for the MediaTek Genio 350-evk board | expand |
On 26/02/2024 15:01, Alexandre Mergnat wrote: > Add MT8365 audio front-end bindings > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- > .../bindings/sound/mediatek,mt8365-afe.yaml | 164 +++++++++++++++++++++ > 1 file changed, 164 insertions(+) > > 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..1d7eb2532ad2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml > @@ -0,0 +1,164 @@ > +# 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: 2 You must describe the items. > + > + interrupts: > + maxItems: 1 > + > + mediatek,topckgen: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: The phandle of the mediatek topckgen controller What for? Don't repeat the property name. Say something useful. > + > + power-domains: > + 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 > + > + mediatek,i2s-shared-clock: Why do you need this property? Linux (and other systems) handle sharing clocks properly. > + description: > + i2s modules can share the same external clock pin. > + If this property is not present the clock mode is separrate. Typo > + type: boolean > + > + mediatek,dmic-iir-on: > + description: > + Boolean which specifies whether the DMIC IIR is enabled. > + If this property is not present the IIR is disabled. "is enabled" or "enable it"? You described the desired Linux feature or behavior, not the actual hardware. The bindings are about the latter, so instead you need to rephrase the property and its description to match actual hardware capabilities/features/configuration etc. > + type: boolean > + > + mediatek,dmic-irr-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Selects stop band of IIR DC-removal filter. > + 0 = Software programmable custom coeff loaded by the driver. Bindings are for hardware, not drivers. Why is this a property of board DTS? > + 1 = 5Hz if 48KHz mode. > + 2 = 10Hz if 48KHz mode. > + 3 = 25Hz if 48KHz mode. > + 4 = 50Hz if 48KHz mode. > + 5 = 65Hz if 48KHz mode. Use proper unit suffixes - hz. > + enum: > + - 0 > + - 1 > + - 2 > + - 3 > + - 4 > + - 5 > + > + mediatek,dmic-two-wire-mode: > + description: > + Boolean which turns on digital microphone for two wire mode. > + If this property is not present the two wire mode is disabled. This looks like hardware property, but the naming looks like SW. Again you instruct what driver should do. Standard disclaimer: You described the desired Linux feature or behavior, not the actual hardware. The bindings are about the latter, so instead you need to rephrase the property and its description to match actual hardware capabilities/features/configuration etc. > + type: boolean > + > + Just one blank line. > +required: > + - compatible > + - reg > + - interrupts > + - power-domains > + - mediatek,topckgen > + - clocks > + - clock-names > + > +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 { What is AFE? https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "mediatek,mt8365-afe-pcm"; Best regards, Krzysztof
On 26/02/2024 15:01, Alexandre Mergnat wrote: > Add the codec property along with the mt6357.c codec driver support. Describe the hardware, not the Linux drivers. There is no codec driver support in the bindings. https://elixir.bootlin.com/linux/v6.8-rc6/source/Documentation/devicetree/bindings/writing-bindings.rst#L21 > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- > Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > index 37423c2e0fdf..d25a78070744 100644 > --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > @@ -37,6 +37,17 @@ properties: > "#interrupt-cells": > const: 2 > > + codec: > + type: object > + unevaluatedProperties: false > + description: > + MT6357 sound codec. > + properties: > + compatible: > + const: mediatek,mt6357-sound > + required: > + - compatible No resources? Then no need for this node. We have it even documented (if my repeating every time is not enough)... https://elixir.bootlin.com/linux/v6.8-rc6/source/Documentation/devicetree/bindings/writing-bindings.rst#L30 Best regards, Krzysztof
On Mon, Feb 26, 2024 at 03:01:38PM +0100, Alexandre Mergnat wrote: > This serie aim to add the following audio support for the Genio 350-evk: > - Playback > - 2ch Headset Jack (Earphone) > - 1ch Line-out Jack (Speaker) > - 8ch HDMI Tx > - Capture > - 1ch DMIC (On-board Digital Microphone) > - 1ch AMIC (On-board Analogic Microphone) > - 1ch Headset Jack (External Analogic Microphone) > > Of course, HDMI playback need the MT8365 display patches [1] and a DTS > change documented in "mediatek,mt8365-mt6357.yaml". Given the number of custom controls here could you please post the output of mixer-test and pcm-test from a system with this driver running next time you post, this will help with review since it checks a bunch of things around the new controls.
On 27/02/2024 10:01, Krzysztof Kozlowski wrote: > On 26/02/2024 15:01, Alexandre Mergnat wrote: >> Add MT8365 audio front-end bindings >> >> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >> --- >> .../bindings/sound/mediatek,mt8365-afe.yaml | 164 +++++++++++++++++++++ >> 1 file changed, 164 insertions(+) >> >> 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..1d7eb2532ad2 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml >> @@ -0,0 +1,164 @@ >> +# 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: 2 > > You must describe the items. Actually, I've took downstream code that I'm not able to explain because the second reg isn't on my MTK documentation. So I've remove the second reg and passed all functional tests successfully. Then I will remove the extra reg for v2. > >> + >> + interrupts: >> + maxItems: 1 >> + >> + mediatek,topckgen: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: The phandle of the mediatek topckgen controller > > What for? Don't repeat the property name. Say something useful. got it > >> + >> + power-domains: >> + 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 >> + >> + mediatek,i2s-shared-clock: > > Why do you need this property? Linux (and other systems) handle sharing > clocks properly. Indeed, not needed. It was used by the downstream code driver but I remove it for v2. > >> + description: >> + i2s modules can share the same external clock pin. >> + If this property is not present the clock mode is separrate. > > Typo > >> + type: boolean >> + >> + mediatek,dmic-iir-on: >> + description: >> + Boolean which specifies whether the DMIC IIR is enabled. >> + If this property is not present the IIR is disabled. > > "is enabled" or "enable it"? > > You described the desired Linux feature or behavior, not the actual > hardware. The bindings are about the latter, so instead you need to > rephrase the property and its description to match actual hardware > capabilities/features/configuration etc. I will rephrase: True to enable the Infinite Impulse Response (IIR) filter on the digital microphone inputs. > >> + type: boolean >> + >> + mediatek,dmic-irr-mode: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Selects stop band of IIR DC-removal filter. >> + 0 = Software programmable custom coeff loaded by the driver. > > Bindings are for hardware, not drivers. Why is this a property of board DTS? Actually this is a hardware feature. Mode 1 t 5 are predefined filters. Mode 0, the HW will read some "coef filter registers" to setup the custom filter. the "coef filter regs" are written by the driver. Currently the coef values are hardcoded in the driver. > >> + 1 = 5Hz if 48KHz mode. >> + 2 = 10Hz if 48KHz mode. >> + 3 = 25Hz if 48KHz mode. >> + 4 = 50Hz if 48KHz mode. >> + 5 = 65Hz if 48KHz mode. > > Use proper unit suffixes - hz. > > >> + enum: >> + - 0 >> + - 1 >> + - 2 >> + - 3 >> + - 4 >> + - 5 >> + >> + mediatek,dmic-two-wire-mode: >> + description: >> + Boolean which turns on digital microphone for two wire mode. >> + If this property is not present the two wire mode is disabled. > > This looks like hardware property, but the naming looks like SW. Again > you instruct what driver should do. Standard disclaimer: > > You described the desired Linux feature or behavior, not the actual > hardware. The bindings are about the latter, so instead you need to > rephrase the property and its description to match actual hardware > capabilities/features/configuration etc. Actually this is a hardware feature. This is ALL I have to describe the HW behavior from the datasheet: " bit name: ul_dmic_two_wire_ctl Turns on digital microphone for two wire mode. 0: Turn off 1: Turn on " On the board schematic, SoC and DMIC and linked by 3 pins: - clk - data0 - data1 IMHO, "two-wire-mode" means the HW use 2 pins for data, and the SoC must be aware of that by reading the register value written by the driver, using the value found in the DTS. I don't get why you think it wouldn't be hardware behavior. Rephrase description: "True to enable the two wire mode of the digital microphone" Is it better ? About the property name, "mediatek,dmic-two-wire-ctl" sound better for you ? > > >> + type: boolean >> + >> + > > Just one blank line. > >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - power-domains >> + - mediatek,topckgen >> + - clocks >> + - clock-names >> + >> +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 { > > What is AFE? 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. > > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > >> + compatible = "mediatek,mt8365-afe-pcm"; > > > Best regards, > Krzysztof >
On 27/02/2024 16:18, Alexandre Mergnat wrote: >> >>> + type: boolean >>> + >>> + mediatek,dmic-iir-on: >>> + description: >>> + Boolean which specifies whether the DMIC IIR is enabled. >>> + If this property is not present the IIR is disabled. >> >> "is enabled" or "enable it"? >> >> You described the desired Linux feature or behavior, not the actual >> hardware. The bindings are about the latter, so instead you need to >> rephrase the property and its description to match actual hardware >> capabilities/features/configuration etc. > > I will rephrase: > > True to enable the Infinite Impulse Response (IIR) filter > on the digital microphone inputs. I still don't know why this is DT-specific. You still tell driver what to do... > >> >>> + type: boolean >>> + >>> + mediatek,dmic-irr-mode: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: >>> + Selects stop band of IIR DC-removal filter. >>> + 0 = Software programmable custom coeff loaded by the driver. >> >> Bindings are for hardware, not drivers. Why is this a property of board DTS? > > Actually this is a hardware feature. Mode 1 t 5 are predefined filters. > Mode 0, the HW will read some "coef filter registers" to setup the > custom filter. the "coef filter regs" are written by the driver. > Currently the coef values are hardcoded in the driver. You don't get the point. Just because you choose some mode it does not mean is hardware feature for DT. Sampling frequency done by hardware is also "hardware feature", but do you put it to DT? No. Explain why this is board-specific, not runtime configuration. > >> >>> + 1 = 5Hz if 48KHz mode. >>> + 2 = 10Hz if 48KHz mode. >>> + 3 = 25Hz if 48KHz mode. >>> + 4 = 50Hz if 48KHz mode. >>> + 5 = 65Hz if 48KHz mode. >> >> Use proper unit suffixes - hz. >> >> >>> + enum: >>> + - 0 >>> + - 1 >>> + - 2 >>> + - 3 >>> + - 4 >>> + - 5 >>> + >>> + mediatek,dmic-two-wire-mode: >>> + description: >>> + Boolean which turns on digital microphone for two wire mode. >>> + If this property is not present the two wire mode is disabled. >> >> This looks like hardware property, but the naming looks like SW. Again >> you instruct what driver should do. Standard disclaimer: >> >> You described the desired Linux feature or behavior, not the actual >> hardware. The bindings are about the latter, so instead you need to >> rephrase the property and its description to match actual hardware >> capabilities/features/configuration etc. > > Actually this is a hardware feature. This is ALL I have to describe the > HW behavior from the datasheet: > " > bit name: ul_dmic_two_wire_ctl > Turns on digital microphone for two wire mode. > 0: Turn off > 1: Turn on That's rather suggestion it is not a description of hardware but you want driver to do something... > " > > On the board schematic, SoC and DMIC and linked by 3 pins: > - clk > - data0 > - data1 > > IMHO, "two-wire-mode" means the HW use 2 pins for data, and the SoC must > be aware of that by reading the register value written by the driver, > using the value found in the DTS. So this depends on type of connection of DMIC? Then rephrase description property like this. > > I don't get why you think it wouldn't be hardware behavior. Because telling what to write to the registers which is typical sign of people stuffing to DT whatever they need to configure the hardware. > > Rephrase description: > "True to enable the two wire mode of the digital microphone" > Is it better ? No, because again you describe some sort of mode. If you insist on such description, then my answer is: it's runtime, so not suitable for DT. Instead describe what is the hardware problem/configuration, e.g. "DMIC is connected with only CLK and DATA0, without third pin" etc. > > About the property name, "mediatek,dmic-two-wire-ctl" sound better for you ? To sound more like a register less like physical characteristic of the board? No. The name can stay, I don't have better ideas. Best regards, Krzysztof
I think I got it. - mediatek,i2s-shared-clock: will be remove from DT - mediatek,dmic-iir-on: will be remove from DT - mediatek,dmic-irr-mode: will be remove from DT - mediatek,dmic-two-wire-mode: rephrase description needed I've did abstraction (despite me) that IIR settings are runtime config because the driver implement its usage like a one-time-setup -_-' Thanks for the explanations, that help. Regards, Alexandre On 28/02/2024 08:28, Krzysztof Kozlowski wrote: > On 27/02/2024 16:18, Alexandre Mergnat wrote: >>> >>>> + type: boolean >>>> + >>>> + mediatek,dmic-iir-on: >>>> + description: >>>> + Boolean which specifies whether the DMIC IIR is enabled. >>>> + If this property is not present the IIR is disabled. >>> >>> "is enabled" or "enable it"? >>> >>> You described the desired Linux feature or behavior, not the actual >>> hardware. The bindings are about the latter, so instead you need to >>> rephrase the property and its description to match actual hardware >>> capabilities/features/configuration etc. >> >> I will rephrase: >> >> True to enable the Infinite Impulse Response (IIR) filter >> on the digital microphone inputs. > > I still don't know why this is DT-specific. You still tell driver what > to do... > >> >>> >>>> + type: boolean >>>> + >>>> + mediatek,dmic-irr-mode: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: >>>> + Selects stop band of IIR DC-removal filter. >>>> + 0 = Software programmable custom coeff loaded by the driver. >>> >>> Bindings are for hardware, not drivers. Why is this a property of board DTS? >> >> Actually this is a hardware feature. Mode 1 t 5 are predefined filters. >> Mode 0, the HW will read some "coef filter registers" to setup the >> custom filter. the "coef filter regs" are written by the driver. >> Currently the coef values are hardcoded in the driver. > > You don't get the point. Just because you choose some mode it does not > mean is hardware feature for DT. Sampling frequency done by hardware is > also "hardware feature", but do you put it to DT? No. > > Explain why this is board-specific, not runtime configuration. > >> >>> >>>> + 1 = 5Hz if 48KHz mode. >>>> + 2 = 10Hz if 48KHz mode. >>>> + 3 = 25Hz if 48KHz mode. >>>> + 4 = 50Hz if 48KHz mode. >>>> + 5 = 65Hz if 48KHz mode. >>> >>> Use proper unit suffixes - hz. >>> >>> >>>> + enum: >>>> + - 0 >>>> + - 1 >>>> + - 2 >>>> + - 3 >>>> + - 4 >>>> + - 5 >>>> + >>>> + mediatek,dmic-two-wire-mode: >>>> + description: >>>> + Boolean which turns on digital microphone for two wire mode. >>>> + If this property is not present the two wire mode is disabled. >>> >>> This looks like hardware property, but the naming looks like SW. Again >>> you instruct what driver should do. Standard disclaimer: >>> >>> You described the desired Linux feature or behavior, not the actual >>> hardware. The bindings are about the latter, so instead you need to >>> rephrase the property and its description to match actual hardware >>> capabilities/features/configuration etc. >> >> Actually this is a hardware feature. This is ALL I have to describe the >> HW behavior from the datasheet: >> " >> bit name: ul_dmic_two_wire_ctl >> Turns on digital microphone for two wire mode. >> 0: Turn off >> 1: Turn on > > That's rather suggestion it is not a description of hardware but you > want driver to do something... > >> " >> >> On the board schematic, SoC and DMIC and linked by 3 pins: >> - clk >> - data0 >> - data1 >> >> IMHO, "two-wire-mode" means the HW use 2 pins for data, and the SoC must >> be aware of that by reading the register value written by the driver, >> using the value found in the DTS. > > So this depends on type of connection of DMIC? Then rephrase description > property like this. > >> >> I don't get why you think it wouldn't be hardware behavior. > > Because telling what to write to the registers which is typical sign of > people stuffing to DT whatever they need to configure the hardware. > >> >> Rephrase description: >> "True to enable the two wire mode of the digital microphone" >> Is it better ? > > No, because again you describe some sort of mode. If you insist on such > description, then my answer is: it's runtime, so not suitable for DT. > Instead describe what is the hardware problem/configuration, e.g. "DMIC > is connected with only CLK and DATA0, without third pin" etc. > >> >> About the property name, "mediatek,dmic-two-wire-ctl" sound better for you ? > > To sound more like a register less like physical characteristic of the > board? No. The name can stay, I don't have better ideas. > > > Best regards, > Krzysztof >
Il 28/02/24 10:57, Alexandre Mergnat ha scritto: > I think I got it. > > - mediatek,i2s-shared-clock: will be remove from DT > - mediatek,dmic-iir-on: will be remove from DT > - mediatek,dmic-irr-mode: will be remove from DT > - mediatek,dmic-two-wire-mode: rephrase description needed > > I've did abstraction (despite me) that IIR settings are runtime config because the > driver implement its usage like a one-time-setup -_-' > Yes but just one more thing I just noticed: `mediatek,dmic-two-wire-mode` - can we please rename this to `mediatek,dmic-mode` ? That'd be for consistency check mt6359.yaml and mt6358.txt mediatek,dmic-mode: $ref: /schemas/types.yaml#/definitions/uint32 description: | Indicates how many data pins are used to transmit two channels of PDM signal. 0 means two wires, 1 means one wire. Default value is 0. enum: - 0 # one wire - 1 # two wires Cheers, Angelo > Thanks for the explanations, that help. > > Regards, > Alexandre > > On 28/02/2024 08:28, Krzysztof Kozlowski wrote: >> On 27/02/2024 16:18, Alexandre Mergnat wrote: >>>> >>>>> + type: boolean >>>>> + >>>>> + mediatek,dmic-iir-on: >>>>> + description: >>>>> + Boolean which specifies whether the DMIC IIR is enabled. >>>>> + If this property is not present the IIR is disabled. >>>> >>>> "is enabled" or "enable it"? >>>> >>>> You described the desired Linux feature or behavior, not the actual >>>> hardware. The bindings are about the latter, so instead you need to >>>> rephrase the property and its description to match actual hardware >>>> capabilities/features/configuration etc. >>> >>> I will rephrase: >>> >>> True to enable the Infinite Impulse Response (IIR) filter >>> on the digital microphone inputs. >> >> I still don't know why this is DT-specific. You still tell driver what >> to do... >> >>> >>>> >>>>> + type: boolean >>>>> + >>>>> + mediatek,dmic-irr-mode: >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + description: >>>>> + Selects stop band of IIR DC-removal filter. >>>>> + 0 = Software programmable custom coeff loaded by the driver. >>>> >>>> Bindings are for hardware, not drivers. Why is this a property of board DTS? >>> >>> Actually this is a hardware feature. Mode 1 t 5 are predefined filters. >>> Mode 0, the HW will read some "coef filter registers" to setup the >>> custom filter. the "coef filter regs" are written by the driver. >>> Currently the coef values are hardcoded in the driver. >> >> You don't get the point. Just because you choose some mode it does not >> mean is hardware feature for DT. Sampling frequency done by hardware is >> also "hardware feature", but do you put it to DT? No. >> >> Explain why this is board-specific, not runtime configuration. >> >>> >>>> >>>>> + 1 = 5Hz if 48KHz mode. >>>>> + 2 = 10Hz if 48KHz mode. >>>>> + 3 = 25Hz if 48KHz mode. >>>>> + 4 = 50Hz if 48KHz mode. >>>>> + 5 = 65Hz if 48KHz mode. >>>> >>>> Use proper unit suffixes - hz. >>>> >>>> >>>>> + enum: >>>>> + - 0 >>>>> + - 1 >>>>> + - 2 >>>>> + - 3 >>>>> + - 4 >>>>> + - 5 >>>>> + >>>>> + mediatek,dmic-two-wire-mode: >>>>> + description: >>>>> + Boolean which turns on digital microphone for two wire mode. >>>>> + If this property is not present the two wire mode is disabled. >>>> >>>> This looks like hardware property, but the naming looks like SW. Again >>>> you instruct what driver should do. Standard disclaimer: >>>> >>>> You described the desired Linux feature or behavior, not the actual >>>> hardware. The bindings are about the latter, so instead you need to >>>> rephrase the property and its description to match actual hardware >>>> capabilities/features/configuration etc. >>> >>> Actually this is a hardware feature. This is ALL I have to describe the >>> HW behavior from the datasheet: >>> " >>> bit name: ul_dmic_two_wire_ctl >>> Turns on digital microphone for two wire mode. >>> 0: Turn off >>> 1: Turn on >> >> That's rather suggestion it is not a description of hardware but you >> want driver to do something... >> >>> " >>> >>> On the board schematic, SoC and DMIC and linked by 3 pins: >>> - clk >>> - data0 >>> - data1 >>> >>> IMHO, "two-wire-mode" means the HW use 2 pins for data, and the SoC must >>> be aware of that by reading the register value written by the driver, >>> using the value found in the DTS. >> >> So this depends on type of connection of DMIC? Then rephrase description >> property like this. >> >>> >>> I don't get why you think it wouldn't be hardware behavior. >> >> Because telling what to write to the registers which is typical sign of >> people stuffing to DT whatever they need to configure the hardware. >> >>> >>> Rephrase description: >>> "True to enable the two wire mode of the digital microphone" >>> Is it better ? >> >> No, because again you describe some sort of mode. If you insist on such >> description, then my answer is: it's runtime, so not suitable for DT. >> Instead describe what is the hardware problem/configuration, e.g. "DMIC >> is connected with only CLK and DATA0, without third pin" etc. >> >>> >>> About the property name, "mediatek,dmic-two-wire-ctl" sound better for you ? >> >> To sound more like a register less like physical characteristic of the >> board? No. The name can stay, I don't have better ideas. >> >> >> Best regards, >> Krzysztof >> >
On 28/02/2024 11:25, AngeloGioacchino Del Regno wrote: > Il 28/02/24 10:57, Alexandre Mergnat ha scritto: >> I think I got it. >> >> - mediatek,i2s-shared-clock: will be remove from DT >> - mediatek,dmic-iir-on: will be remove from DT >> - mediatek,dmic-irr-mode: will be remove from DT >> - mediatek,dmic-two-wire-mode: rephrase description needed >> >> I've did abstraction (despite me) that IIR settings are runtime config >> because the driver implement its usage like a one-time-setup -_-' >> > > Yes but just one more thing I just noticed: > `mediatek,dmic-two-wire-mode` - can we > please rename this to `mediatek,dmic-mode` ? Sure, I wasn't aware of this property. I will do it. Note: the description isn't consistent with the enum comments " 0 means two wires, 1 means one wire. ... - 0 # one wire - 1 # two wires " > > That'd be for consistency check mt6359.yaml and mt6358.txt > > mediatek,dmic-mode: > $ref: /schemas/types.yaml#/definitions/uint32 > description: | > Indicates how many data pins are used to transmit two channels of > PDM > signal. 0 means two wires, 1 means one wire. Default value is 0. > enum: > - 0 # one wire > - 1 # two wires > > Cheers, > Angelo > > > >> Thanks for the explanations, that help. >> >> Regards, >> Alexandre >> >> On 28/02/2024 08:28, Krzysztof Kozlowski wrote: >>> On 27/02/2024 16:18, Alexandre Mergnat wrote: >>>>> >>>>>> + type: boolean >>>>>> + >>>>>> + mediatek,dmic-iir-on: >>>>>> + description: >>>>>> + Boolean which specifies whether the DMIC IIR is enabled. >>>>>> + If this property is not present the IIR is disabled. >>>>> >>>>> "is enabled" or "enable it"? >>>>> >>>>> You described the desired Linux feature or behavior, not the actual >>>>> hardware. The bindings are about the latter, so instead you need to >>>>> rephrase the property and its description to match actual hardware >>>>> capabilities/features/configuration etc. >>>> >>>> I will rephrase: >>>> >>>> True to enable the Infinite Impulse Response (IIR) filter >>>> on the digital microphone inputs. >>> >>> I still don't know why this is DT-specific. You still tell driver what >>> to do... >>> >>>> >>>>> >>>>>> + type: boolean >>>>>> + >>>>>> + mediatek,dmic-irr-mode: >>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>> + description: >>>>>> + Selects stop band of IIR DC-removal filter. >>>>>> + 0 = Software programmable custom coeff loaded by the driver. >>>>> >>>>> Bindings are for hardware, not drivers. Why is this a property of >>>>> board DTS? >>>> >>>> Actually this is a hardware feature. Mode 1 t 5 are predefined filters. >>>> Mode 0, the HW will read some "coef filter registers" to setup the >>>> custom filter. the "coef filter regs" are written by the driver. >>>> Currently the coef values are hardcoded in the driver. >>> >>> You don't get the point. Just because you choose some mode it does not >>> mean is hardware feature for DT. Sampling frequency done by hardware is >>> also "hardware feature", but do you put it to DT? No. >>> >>> Explain why this is board-specific, not runtime configuration. >>> >>>> >>>>> >>>>>> + 1 = 5Hz if 48KHz mode. >>>>>> + 2 = 10Hz if 48KHz mode. >>>>>> + 3 = 25Hz if 48KHz mode. >>>>>> + 4 = 50Hz if 48KHz mode. >>>>>> + 5 = 65Hz if 48KHz mode. >>>>> >>>>> Use proper unit suffixes - hz. >>>>> >>>>> >>>>>> + enum: >>>>>> + - 0 >>>>>> + - 1 >>>>>> + - 2 >>>>>> + - 3 >>>>>> + - 4 >>>>>> + - 5 >>>>>> + >>>>>> + mediatek,dmic-two-wire-mode: >>>>>> + description: >>>>>> + Boolean which turns on digital microphone for two wire mode. >>>>>> + If this property is not present the two wire mode is disabled. >>>>> >>>>> This looks like hardware property, but the naming looks like SW. Again >>>>> you instruct what driver should do. Standard disclaimer: >>>>> >>>>> You described the desired Linux feature or behavior, not the actual >>>>> hardware. The bindings are about the latter, so instead you need to >>>>> rephrase the property and its description to match actual hardware >>>>> capabilities/features/configuration etc. >>>> >>>> Actually this is a hardware feature. This is ALL I have to describe the >>>> HW behavior from the datasheet: >>>> " >>>> bit name: ul_dmic_two_wire_ctl >>>> Turns on digital microphone for two wire mode. >>>> 0: Turn off >>>> 1: Turn on >>> >>> That's rather suggestion it is not a description of hardware but you >>> want driver to do something... >>> >>>> " >>>> >>>> On the board schematic, SoC and DMIC and linked by 3 pins: >>>> - clk >>>> - data0 >>>> - data1 >>>> >>>> IMHO, "two-wire-mode" means the HW use 2 pins for data, and the SoC >>>> must >>>> be aware of that by reading the register value written by the driver, >>>> using the value found in the DTS. >>> >>> So this depends on type of connection of DMIC? Then rephrase description >>> property like this. >>> >>>> >>>> I don't get why you think it wouldn't be hardware behavior. >>> >>> Because telling what to write to the registers which is typical sign of >>> people stuffing to DT whatever they need to configure the hardware. >>> >>>> >>>> Rephrase description: >>>> "True to enable the two wire mode of the digital microphone" >>>> Is it better ? >>> >>> No, because again you describe some sort of mode. If you insist on such >>> description, then my answer is: it's runtime, so not suitable for DT. >>> Instead describe what is the hardware problem/configuration, e.g. "DMIC >>> is connected with only CLK and DATA0, without third pin" etc. >>> >>>> >>>> About the property name, "mediatek,dmic-two-wire-ctl" sound better >>>> for you ? >>> >>> To sound more like a register less like physical characteristic of the >>> board? No. The name can stay, I don't have better ideas. >>> >>> >>> Best regards, >>> Krzysztof >>> >> >
On 28/02/2024 11:25, AngeloGioacchino Del Regno wrote: > Il 28/02/24 10:57, Alexandre Mergnat ha scritto: >> I think I got it. >> >> - mediatek,i2s-shared-clock: will be remove from DT >> - mediatek,dmic-iir-on: will be remove from DT >> - mediatek,dmic-irr-mode: will be remove from DT >> - mediatek,dmic-two-wire-mode: rephrase description needed >> >> I've did abstraction (despite me) that IIR settings are runtime config because the >> driver implement its usage like a one-time-setup -_-' >> > > Yes but just one more thing I just noticed: `mediatek,dmic-two-wire-mode` - can we > please rename this to `mediatek,dmic-mode` ? > > That'd be for consistency check mt6359.yaml and mt6358.txt > > mediatek,dmic-mode: > $ref: /schemas/types.yaml#/definitions/uint32 > description: | > Indicates how many data pins are used to transmit two channels of PDM > signal. 0 means two wires, 1 means one wire. Default value is 0. > enum: > - 0 # one wire > - 1 # two wires Thanks for checking. Now I wonder if entire binding just ignored existing work and started doing things from scratch... Best regards, Krzysztof
On Mon, 26 Feb 2024 15:01:51 +0100, amergnat@baylibre.com wrote: > Add MT6357 codec entry in the MFD driver. > > Applied, thanks! [13/18] mfd: mt6397-core: register mt6357 sound codec commit: 79d98102a31ab777b4a6632d799ab2bc63654cf8 -- Lee Jones [李琼斯]
On 26/02/2024 16:25, AngeloGioacchino Del Regno wrote: >> + if (enable) { >> + /* set gpio mosi mode */ >> + regmap_write(priv->regmap, MT6357_GPIO_MODE2_CLR, >> GPIO_MODE2_CLEAR_ALL); >> + regmap_write(priv->regmap, MT6357_GPIO_MODE2_SET, >> GPIO8_MODE_SET_AUD_CLK_MOSI | >> + GPIO9_MODE_SET_AUD_DAT_MOSI0 | >> + GPIO10_MODE_SET_AUD_DAT_MOSI1 | >> + GPIO11_MODE_SET_AUD_SYNC_MOSI); > > Are you sure that you need to write to MODE2_SET *and* to MODE2?! This is downstream code and these registers aren't in my documentation. I've removed the MODE2_SET write and test the audio: it's still working. So I will keep the spurious write removed for v2. :)
On Tue, Mar 12, 2024 at 07:03:25PM +0100, Alexandre Mergnat wrote: > On 26/02/2024 17:09, Mark Brown wrote: > > > + case MT6357_ZCD_CON2: > > > + regmap_read(priv->regmap, MT6357_ZCD_CON2, ®); > > > + priv->ana_gain[ANALOG_VOLUME_HPOUTL] = > > > + (reg & AUD_HPL_GAIN_MASK) >> AUD_HPL_GAIN_SFT; > > > + priv->ana_gain[ANALOG_VOLUME_HPOUTR] = > > > + (reg & AUD_HPR_GAIN_MASK) >> AUD_HPR_GAIN_SFT; > > > + break; > > It would probably be less code and would definitely be clearer and > > simpler to just read the values when we need them rather than constatly > > keeping a cache separate to the register cache. > Actually you must save the values because the gain selected by the user will > be override to do a ramp => volume_ramp(.....): > - When you switch on the HP, you start from gain=-40db to final_gain > (selected by user). > - When you switch off the HP, you start from final_gain (selected by user) > to gain=-40db. You can just read the value back when you need to do a ramp? > Also, the microphone's gain change when it's enabled/disabled. I don't understand what this means? > > > + /* ul channel swap */ > > > + SOC_SINGLE("UL LR Swap", MT6357_AFE_UL_DL_CON0, AFE_UL_LR_SWAP_SFT, 1, 0), > > On/off controls should end in Switch. > Sorry, I don't understand your comment. Can you reword it please ? See control-names.rst. Run mixer-test on a card with this driver and fix all the issues it reports. > > > +static int hslo_mux_map_value[] = { > > > + 0x0, 0x1, 0x2, 0x3, > > > +}; > > Why not just use a normal mux here, there's no missing values or > > reordering? Similarly for other muxes. > I've dug into some other codecs and it's done like that, but I've probably > misunderstood something. > The only bad thing I see is enum is missing currently: > > enum { > PGA_MUX_OPEN = 0, > PGA_MUX_DACR, > PGA_MUX_PB, > PGA_MUX_TM, > PGA_MUX_MASK = 0x3, > }; The whole thing with explicitly specfying the mapping is just completely redundant, you may as well remove it.
On 13/03/2024 18:23, Mark Brown wrote: > On Tue, Mar 12, 2024 at 07:03:25PM +0100, Alexandre Mergnat wrote: >> On 26/02/2024 17:09, Mark Brown wrote: > >>>> + case MT6357_ZCD_CON2: >>>> + regmap_read(priv->regmap, MT6357_ZCD_CON2, ®); >>>> + priv->ana_gain[ANALOG_VOLUME_HPOUTL] = >>>> + (reg & AUD_HPL_GAIN_MASK) >> AUD_HPL_GAIN_SFT; >>>> + priv->ana_gain[ANALOG_VOLUME_HPOUTR] = >>>> + (reg & AUD_HPR_GAIN_MASK) >> AUD_HPR_GAIN_SFT; >>>> + break; > >>> It would probably be less code and would definitely be clearer and >>> simpler to just read the values when we need them rather than constatly >>> keeping a cache separate to the register cache. > >> Actually you must save the values because the gain selected by the user will >> be override to do a ramp => volume_ramp(.....): >> - When you switch on the HP, you start from gain=-40db to final_gain >> (selected by user). >> - When you switch off the HP, you start from final_gain (selected by user) >> to gain=-40db. > > You can just read the value back when you need to do a ramp? You can't. Because you will read -40db when HP isn't playing sound. That is why the gain is saved into the struct. Let me know, when you change de gain to do a ramp down (start from user gain to gain=-40db), next time for the ramp up, how/where do you find the user gain ? > >> Also, the microphone's gain change when it's enabled/disabled. > > I don't understand what this means? When microphone isn't capturing, the gain read back from the register is 0dB. I've put some logs in my code and do capture to show how it works: root@i350-evk:~# arecord -D hw:mt8365evk,2,0 -r 48000 -c2 -f s32_le -d 10 recorded_file.wav [Mar15 09:31] mt8365-afe-pcm 11220000.audio-controller: mt8365_afe_fe_hw_params AWB period = 6000 rate = 48000 channels = 2 [ +0.000126] mt8365-afe-pcm 11220000.audio-controller: mt8365_dai_int_adda_prepare 'Capture' rate = 48000 [ +0.107688] mt6357-sound mt6357-sound: TOTO set mic to stored value [ +10.072648] mt6357-sound mt6357-sound: TOTO set mic to 0dB root@i350-evk:~# arecord -D hw:mt8365evk,2,0 -r 48000 -c2 -f s32_le -d 10 recorded_file.wav [Mar15 09:32] mt8365-afe-pcm 11220000.audio-controller: mt8365_afe_fe_hw_params AWB period = 6000 rate = 48000 channels = 2 [ +0.000133] mt8365-afe-pcm 11220000.audio-controller: mt8365_dai_int_adda_prepare 'Capture' rate = 48000 [ +0.109418] mt6357-sound mt6357-sound: TOTO set mic to stored value [ +10.164197] mt6357-sound mt6357-sound: TOTO set mic to 0dB > >>>> + /* ul channel swap */ >>>> + SOC_SINGLE("UL LR Swap", MT6357_AFE_UL_DL_CON0, AFE_UL_LR_SWAP_SFT, 1, 0), > >>> On/off controls should end in Switch. > >> Sorry, I don't understand your comment. Can you reword it please ? > > See control-names.rst. Run mixer-test on a card with this driver and > fix all the issues it reports. Ok the name is the issue for you AFAII. This control isn't for on/off but swap Left and Right. From the codec documentation: "Swaps audio UL L/R channel before UL SRC" This control is overkill, I will remove it I'm stuck to run mixer-test, please check the following message: https://lore.kernel.org/all/7ddad394-e880-4ef8-8591-cb803a2086ae@baylibre.com/
On Fri, Mar 15, 2024 at 12:01:12PM +0100, Alexandre Mergnat wrote: > On 13/03/2024 18:23, Mark Brown wrote: > > On Tue, Mar 12, 2024 at 07:03:25PM +0100, Alexandre Mergnat wrote: > > > Actually you must save the values because the gain selected by the user will > > > be override to do a ramp => volume_ramp(.....): > > > - When you switch on the HP, you start from gain=-40db to final_gain > > > (selected by user). > > > - When you switch off the HP, you start from final_gain (selected by user) > > > to gain=-40db. > > You can just read the value back when you need to do a ramp? > You can't. Because you will read -40db when HP isn't playing sound. That is > why the gain is saved into the struct. > Let me know, when you change de gain to do a ramp down (start from user gain > to gain=-40db), next time for the ramp up, how/where do you find the user > gain ? In the register. You only need to reset the gain to -40dB at the start of the ramp. > > > Also, the microphone's gain change when it's enabled/disabled. > > I don't understand what this means? > When microphone isn't capturing, the gain read back from the register is > 0dB. I've put some logs in my code and do capture to show how it works: Is this a property of the hardware or a property of your driver? > > > > > + /* ul channel swap */ > > > > > + SOC_SINGLE("UL LR Swap", MT6357_AFE_UL_DL_CON0, AFE_UL_LR_SWAP_SFT, 1, 0), > > > > On/off controls should end in Switch. > > > Sorry, I don't understand your comment. Can you reword it please ? > > See control-names.rst. Run mixer-test on a card with this driver and > > fix all the issues it reports. > Ok the name is the issue for you AFAII. > This control isn't for on/off but swap Left and Right. > From the codec documentation: > "Swaps audio UL L/R channel before UL SRC" > This control is overkill, I will remove it This is turning the swapping on and off.
On 15/03/2024 16:15, Mark Brown wrote: > On Fri, Mar 15, 2024 at 04:05:21PM +0100, Alexandre Mergnat wrote: >> On 15/03/2024 15:30, Mark Brown wrote: > >>>> Let me know, when you change de gain to do a ramp down (start from user gain >>>> to gain=-40db), next time for the ramp up, how/where do you find the user >>>> gain ? > >>> In the register. You only need to reset the gain to -40dB at the start >>> of the ramp. > >> Sorry but I don't understand your logic, I'm not able to implement it... >> If I'm at -10dB and doing a ramp to reach -40dB, next time I will read the >> register the value will be -40dB. > > After we've done the ramp and turned the amplifier off we can just > restore the desired value? The hardware is not going to care what the > volume is while it's not enabled. If you do that, HP will be enabled at the saved gain, and after that you will do the ramp. To avoid pop, the driver should be rewrite to: Read gain in the reg and save it locally Set -40dB in the reg Enable HP Do ramp And for the shutdown: Read gain in the reg and save it locally Do ramp Disable HP Set saved gain in the reg To resume, that add 4 more steps to save 2 integers into the driver structure. IMHO, I don't think it make the code more readable or optimized, but I don't have a strong opinion about that, so if you think it's better, I will change it. > >> This implementation is also done in other MTK audio codec drivers. > > Perhaps they should be updated too? > >>>> When microphone isn't capturing, the gain read back from the register is >>>> 0dB. I've put some logs in my code and do capture to show how it works: > >>> Is this a property of the hardware or a property of your driver? > >> At the end of the capture, the gain is set to 0dB by the driver. >> At the start of the capture, the gain is set to the setup gain. > > So that's a property of the driver then? Yes > >> AFAII from the comment in the code, it's done to avoid the "pop noises". > > Yes, that's the usual reason to ramp gains. Though if you've just > copied the code without checking that it's needed it's possible that > this is something that's been fixed in current hardware. I did the test at 24dB with and without the "pop filter". Isn't big but I ear the pop at the start of the record without the "pop filter". To be clear, the algo/behavior of this code is an implementation based on the 6k+ lines downstream code for this specific audio codec. But the shape/style is based on upstreamed drivers like mt6358.c.
Hi Angelo On 26/02/2024 15:54, AngeloGioacchino Del Regno wrote: > Il 26/02/24 15:01, Alexandre Mergnat ha scritto: >> This serie aim to add the following audio support for the Genio 350-evk: >> - Playback >> - 2ch Headset Jack (Earphone) >> - 1ch Line-out Jack (Speaker) >> - 8ch HDMI Tx >> - Capture >> - 1ch DMIC (On-board Digital Microphone) >> - 1ch AMIC (On-board Analogic Microphone) >> - 1ch Headset Jack (External Analogic Microphone) >> >> Of course, HDMI playback need the MT8365 display patches [1] and a DTS >> change documented in "mediatek,mt8365-mt6357.yaml". >> >> [1]: >> https://lore.kernel.org/all/20231023-display-support-v1-0-5c860ed5c33b@baylibre.com/ >> >> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > > Actually, I am cooking a series (I'm finishing the testing....) that > brings quite > a bit of cleanups in MTK ASoC, including the commonization of the > machine driver > probe, with the dai-link DT nodes, and which also modernizes most of the > existing > drivers to use that instead. > > If you wait for a day or two, your mt8365-mt6357.c driver's probe > function can be > shrunk to ~3 lines or something like that.. very easily :-) Just to inform you. I'm aware of your serie. Currently, I've fixed my patches according to the comments. The next step will be to rebase my serie over yours and do the changes to be aligned with your new implementation. I've planned to review your serie during my last task, but it seems already approved and already (partially) merged into linux-next, sorry.
This serie aim to add the following audio support for the Genio 350-evk: - Playback - 2ch Headset Jack (Earphone) - 1ch Line-out Jack (Speaker) - 8ch HDMI Tx - Capture - 1ch DMIC (On-board Digital Microphone) - 1ch AMIC (On-board Analogic Microphone) - 1ch Headset Jack (External Analogic Microphone) Of course, HDMI playback need the MT8365 display patches [1] and a DTS change documented in "mediatek,mt8365-mt6357.yaml". [1]: https://lore.kernel.org/all/20231023-display-support-v1-0-5c860ed5c33b@baylibre.com/ Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> --- Alexandre Mergnat (15): ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC ASoC: mediatek: mt8365: Add common header SoC: mediatek: mt8365: support audio clock control ASoC: mediatek: mt8365: Add I2S DAI support ASoC: mediatek: mt8365: Add ADDA DAI support ASoC: mediatek: mt8365: Add DMIC DAI support ASoC: mediatek: mt8365: Add PCM DAI support ASoC: mediatek: mt8365: Add platform driver ASoC: mediatek: Add MT8365 support arm64: defconfig: enable mt8365 sound arm64: dts: mediatek: add mt6357 audio codec support arm64: dts: mediatek: add afe support for mt8365 SoC arm64: dts: mediatek: add audio support for mt8365-evk Fabien Parent (1): mfd: mt6397-core: register mt6357 sound codec Nicolas Belin (2): ASoc: mediatek: mt8365: Add a specific soundcard for EVK ASoC: codecs: mt6357: add MT6357 codec .../devicetree/bindings/mfd/mediatek,mt6357.yaml | 11 + .../bindings/sound/mediatek,mt8365-afe.yaml | 164 ++ .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 ++ arch/arm64/boot/dts/mediatek/mt6357.dtsi | 6 +- arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 95 +- arch/arm64/boot/dts/mediatek/mt8365.dtsi | 47 +- arch/arm64/configs/defconfig | 2 + drivers/mfd/mt6397-core.c | 3 + sound/soc/codecs/Kconfig | 7 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/mt6357.c | 1805 +++++++++++++++ sound/soc/codecs/mt6357.h | 674 ++++++ sound/soc/mediatek/Kconfig | 20 + sound/soc/mediatek/Makefile | 1 + sound/soc/mediatek/mt8365/Makefile | 15 + sound/soc/mediatek/mt8365/mt8365-afe-clk.c | 455 ++++ sound/soc/mediatek/mt8365/mt8365-afe-clk.h | 55 + sound/soc/mediatek/mt8365/mt8365-afe-common.h | 495 +++++ sound/soc/mediatek/mt8365/mt8365-afe-pcm.c | 2347 ++++++++++++++++++++ sound/soc/mediatek/mt8365/mt8365-dai-adda.c | 355 +++ sound/soc/mediatek/mt8365/mt8365-dai-dmic.c | 475 ++++ sound/soc/mediatek/mt8365/mt8365-dai-i2s.c | 901 ++++++++ sound/soc/mediatek/mt8365/mt8365-dai-pcm.c | 298 +++ sound/soc/mediatek/mt8365/mt8365-mt6357.c | 379 ++++ sound/soc/mediatek/mt8365/mt8365-reg.h | 987 ++++++++ 25 files changed, 9718 insertions(+), 8 deletions(-) --- base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d change-id: 20240226-audio-i350-4e11da088e55 Best regards,