| Message ID | 20240226-audio-i350-v1-2-4fa1cea1667f@baylibre.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add audio support for the MediaTek Genio 350-evk board | expand |
On 26/02/2024 16:30, AngeloGioacchino Del Regno wrote: > Il 26/02/24 15:01, Alexandre Mergnat ha scritto: >> Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec. >> >> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >> --- >> .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 >> +++++++++++++++++++++ >> 1 file changed, 127 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >> b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >> new file mode 100644 >> index 000000000000..f469611ec6b6 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >> @@ -0,0 +1,127 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-mt6357.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Mediatek MT8365 sound card with MT6357 sound codec. >> + >> +maintainers: >> + - Alexandre Mergnat <amergnat@baylibre.com> >> + >> +description: >> + This binding describes the MT8365 sound card. >> + >> +properties: >> + compatible: >> + const: mediatek,mt8365-mt6357 >> + >> + mediatek,hp-pull-down: >> + description: >> + Earphone driver positive output stage short to the >> + audio reference ground. >> + Default value is false. >> + type: boolean >> + >> + mediatek,micbias0-microvolt: >> + description: | > > description: Selects MIC Bias 0 output voltage > >> + Selects MIC Bias 0 output voltage. >> + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] >> + enum: [0, 1, 2, 3, 4, 5, 6, 7] > > No, you don't say 0 1 2 3 4 to a property that says "microvolt", that's > simply > wrong. > > mediatek,micbias0-microvolt = <2100000>; > > ...so you want a binding that says > enum: [ 1700000, 1800000, this, that, 2700000] > Is it correct if I put "description: Selects MIC Bias 0 output voltage index" ? >> + >> + mediatek,micbias1-microvolt: >> + description: | >> + Selects MIC Bias 1 output voltage. >> + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] >> + enum: [0, 1, 2, 3, 4, 5, 6, 7] > > same here. > >> + >> + mediatek,platform: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: The phandle of MT8365 ASoC platform. >> + >> + pinctrl-names: >> + minItems: 1 >> + items: >> + - const: aud_default >> + - const: aud_dmic >> + - const: aud_miso_off >> + - const: aud_miso_on >> + - const: aud_mosi_off >> + - const: aud_mosi_on >> + >> + vaud28-supply: >> + description: >> + 2.8 volt supply for the audio codec >> + >> +patternProperties: >> + "^dai-link-[0-9]+$": >> + type: object >> + description: >> + Container for dai-link level properties and CODEC sub-nodes. >> + >> + properties: >> + codec: >> + type: object >> + description: Holds subnode which indicates codec dai. >> + >> + properties: >> + sound-dai: >> + maxItems: 1 >> + description: phandle of the codec DAI >> + >> + additionalProperties: false >> + >> + link-name: >> + description: >> + This property corresponds to the name of the BE dai-link to >> which >> + we are going to update parameters in this node. >> + items: >> + const: 2ND I2S BE >> + >> + sound-dai: >> + maxItems: 1 >> + description: phandle of the CPU DAI >> + >> + additionalProperties: false >> + >> + required: >> + - link-name >> + - sound-dai >> + >> +additionalProperties: false >> + >> +required: >> + - compatible >> + - mediatek,platform >> + - pinctrl-names >> + - vaud28-supply >> + >> +examples: >> + - | >> + sound { >> + compatible = "mediatek,mt8365-mt6357"; >> + mediatek,platform = <&afe>; > > Please: > > https://docs.kernel.org/devicetree/bindings/dts-coding-style.html Is it about the wrong pinctrl-names tab alignment ? Also, 2ND I2S BE => 2ND_I2S_BE ? Otherwise, I don't get it sorry. > > Regards, > Angelo > >> + pinctrl-names = "aud_default", >> + "aud_dmic", >> + "aud_miso_off", >> + "aud_miso_on", >> + "aud_mosi_off", >> + "aud_mosi_on"; >> + pinctrl-0 = <&aud_default_pins>; >> + pinctrl-1 = <&aud_dmic_pins>; >> + pinctrl-2 = <&aud_miso_off_pins>; >> + pinctrl-3 = <&aud_miso_on_pins>; >> + pinctrl-4 = <&aud_mosi_off_pins>; >> + pinctrl-5 = <&aud_mosi_on_pins>; >> + vaud28-supply = <&mt6357_vaud28_reg>; >> + >> + /* hdmi interface */ >> + dai-link-0 { >> + sound-dai = <&afe>; >> + link-name = "2ND I2S BE"; >> + >> + codec { >> + sound-dai = <&it66121hdmitx>; >> + }; >> + }; >> + }; >> >
On 27/02/2024 11:23, Alexandre Mergnat wrote: >>> + >>> +examples: >>> + - | >>> + sound { >>> + compatible = "mediatek,mt8365-mt6357"; >>> + mediatek,platform = <&afe>; >> >> Please: >> >> https://docs.kernel.org/devicetree/bindings/dts-coding-style.html > > Is it about the wrong pinctrl-names tab alignment ? > Also, 2ND I2S BE => 2ND_I2S_BE ? > Otherwise, I don't get it sorry. Alignment of continued lines, order of properties. Best regards, Krzysztof
Il 27/02/24 11:23, Alexandre Mergnat ha scritto: > > > On 26/02/2024 16:30, AngeloGioacchino Del Regno wrote: >> Il 26/02/24 15:01, Alexandre Mergnat ha scritto: >>> Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec. >>> >>> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >>> --- >>> .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 +++++++++++++++++++++ >>> 1 file changed, 127 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >>> b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >>> new file mode 100644 >>> index 000000000000..f469611ec6b6 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >>> @@ -0,0 +1,127 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-mt6357.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Mediatek MT8365 sound card with MT6357 sound codec. >>> + >>> +maintainers: >>> + - Alexandre Mergnat <amergnat@baylibre.com> >>> + >>> +description: >>> + This binding describes the MT8365 sound card. >>> + >>> +properties: >>> + compatible: >>> + const: mediatek,mt8365-mt6357 >>> + >>> + mediatek,hp-pull-down: >>> + description: >>> + Earphone driver positive output stage short to the >>> + audio reference ground. >>> + Default value is false. >>> + type: boolean >>> + >>> + mediatek,micbias0-microvolt: >>> + description: | >> >> description: Selects MIC Bias 0 output voltage >> >>> + Selects MIC Bias 0 output voltage. >>> + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] >>> + enum: [0, 1, 2, 3, 4, 5, 6, 7] >> >> No, you don't say 0 1 2 3 4 to a property that says "microvolt", that's simply >> wrong. >> >> mediatek,micbias0-microvolt = <2100000>; >> >> ...so you want a binding that says >> enum: [ 1700000, 1800000, this, that, 2700000] >> > > Is it correct if I put "description: Selects MIC Bias 0 output voltage index" ? > No, it's still not correct. You have to pass microvolt values. The driver will then transform the microvolt values to the index and subsequently write the index value to the hardware registers. The bindings shall be generic, in the sense that they shall not express hardware register values... and this is especially true when we have a value that *does* actually have means to be expressed in common units. Besides, in the cases in which there's no common units involved, the values most probably won't be suited for devicetree//bindings, so those would be hardcoded in the driver as platform data. This is not the case, so, please keep this property but specify microvolts in the bindings (and obviously in devicetree). >>> + >>> + mediatek,micbias1-microvolt: >>> + description: | >>> + Selects MIC Bias 1 output voltage. >>> + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] >>> + enum: [0, 1, 2, 3, 4, 5, 6, 7] >> >> same here. >> >>> + >>> + mediatek,platform: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: The phandle of MT8365 ASoC platform. >>> + >>> + pinctrl-names: >>> + minItems: 1 >>> + items: >>> + - const: aud_default >>> + - const: aud_dmic >>> + - const: aud_miso_off >>> + - const: aud_miso_on >>> + - const: aud_mosi_off >>> + - const: aud_mosi_on >>> + >>> + vaud28-supply: >>> + description: >>> + 2.8 volt supply for the audio codec >>> + >>> +patternProperties: >>> + "^dai-link-[0-9]+$": >>> + type: object >>> + description: >>> + Container for dai-link level properties and CODEC sub-nodes. >>> + >>> + properties: >>> + codec: >>> + type: object >>> + description: Holds subnode which indicates codec dai. >>> + >>> + properties: >>> + sound-dai: >>> + maxItems: 1 >>> + description: phandle of the codec DAI >>> + >>> + additionalProperties: false >>> + >>> + link-name: >>> + description: >>> + This property corresponds to the name of the BE dai-link to which >>> + we are going to update parameters in this node. >>> + items: >>> + const: 2ND I2S BE >>> + >>> + sound-dai: >>> + maxItems: 1 >>> + description: phandle of the CPU DAI >>> + >>> + additionalProperties: false >>> + >>> + required: >>> + - link-name >>> + - sound-dai >>> + >>> +additionalProperties: false >>> + >>> +required: >>> + - compatible >>> + - mediatek,platform >>> + - pinctrl-names >>> + - vaud28-supply >>> + >>> +examples: >>> + - | >>> + sound { >>> + compatible = "mediatek,mt8365-mt6357"; >>> + mediatek,platform = <&afe>; >> >> Please: >> >> https://docs.kernel.org/devicetree/bindings/dts-coding-style.html > > Is it about the wrong pinctrl-names tab alignment ? > Also, 2ND I2S BE => 2ND_I2S_BE ? > Otherwise, I don't get it sorry. > ...as Krzysztof already clarified, won't repeat :-P Cheers! >> >> Regards, >> Angelo >> >>> + pinctrl-names = "aud_default", >>> + "aud_dmic", >>> + "aud_miso_off", >>> + "aud_miso_on", >>> + "aud_mosi_off", >>> + "aud_mosi_on"; >>> + pinctrl-0 = <&aud_default_pins>; >>> + pinctrl-1 = <&aud_dmic_pins>; >>> + pinctrl-2 = <&aud_miso_off_pins>; >>> + pinctrl-3 = <&aud_miso_on_pins>; >>> + pinctrl-4 = <&aud_mosi_off_pins>; >>> + pinctrl-5 = <&aud_mosi_on_pins>; >>> + vaud28-supply = <&mt6357_vaud28_reg>; >>> + >>> + /* hdmi interface */ >>> + dai-link-0 { >>> + sound-dai = <&afe>; >>> + link-name = "2ND I2S BE"; >>> + >>> + codec { >>> + sound-dai = <&it66121hdmitx>; >>> + }; >>> + }; >>> + }; >>> >> >
On 27/02/2024 13:38, AngeloGioacchino Del Regno wrote: > Il 27/02/24 11:23, Alexandre Mergnat ha scritto: >> >> >> On 26/02/2024 16:30, AngeloGioacchino Del Regno wrote: >>> Il 26/02/24 15:01, Alexandre Mergnat ha scritto: >>>> Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec. >>>> >>>> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> --- >>>> .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 >>>> +++++++++++++++++++++ >>>> 1 file changed, 127 insertions(+) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >>>> new file mode 100644 >>>> index 000000000000..f469611ec6b6 >>>> --- /dev/null >>>> +++ >>>> b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml >>>> @@ -0,0 +1,127 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-mt6357.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Mediatek MT8365 sound card with MT6357 sound codec. >>>> + >>>> +maintainers: >>>> + - Alexandre Mergnat <amergnat@baylibre.com> >>>> + >>>> +description: >>>> + This binding describes the MT8365 sound card. >>>> + >>>> +properties: >>>> + compatible: >>>> + const: mediatek,mt8365-mt6357 >>>> + >>>> + mediatek,hp-pull-down: >>>> + description: >>>> + Earphone driver positive output stage short to the >>>> + audio reference ground. >>>> + Default value is false. >>>> + type: boolean >>>> + >>>> + mediatek,micbias0-microvolt: >>>> + description: | >>> >>> description: Selects MIC Bias 0 output voltage >>> >>>> + Selects MIC Bias 0 output voltage. >>>> + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] >>>> + enum: [0, 1, 2, 3, 4, 5, 6, 7] >>> >>> No, you don't say 0 1 2 3 4 to a property that says "microvolt", >>> that's simply >>> wrong. >>> >>> mediatek,micbias0-microvolt = <2100000>; >>> >>> ...so you want a binding that says >>> enum: [ 1700000, 1800000, this, that, 2700000] >>> >> >> Is it correct if I put "description: Selects MIC Bias 0 output voltage >> index" ? >> > > No, it's still not correct. You have to pass microvolt values. > > The driver will then transform the microvolt values to the index and > subsequently > write the index value to the hardware registers. > > The bindings shall be generic, in the sense that they shall not express > hardware > register values... and this is especially true when we have a value that > *does* > actually have means to be expressed in common units. > > Besides, in the cases in which there's no common units involved, the > values most > probably won't be suited for devicetree//bindings, so those would be > hardcoded in > the driver as platform data. > > This is not the case, so, please keep this property but specify > microvolts in the > bindings (and obviously in devicetree). Got it, thx ! > >>>> + >>>> + mediatek,micbias1-microvolt: >>>> + description: | >>>> + Selects MIC Bias 1 output voltage. >>>> + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] >>>> + enum: [0, 1, 2, 3, 4, 5, 6, 7] >>> >>> same here. >>> >>>> + >>>> + mediatek,platform: >>>> + $ref: /schemas/types.yaml#/definitions/phandle >>>> + description: The phandle of MT8365 ASoC platform. >>>> + >>>> + pinctrl-names: >>>> + minItems: 1 >>>> + items: >>>> + - const: aud_default >>>> + - const: aud_dmic >>>> + - const: aud_miso_off >>>> + - const: aud_miso_on >>>> + - const: aud_mosi_off >>>> + - const: aud_mosi_on >>>> + >>>> + vaud28-supply: >>>> + description: >>>> + 2.8 volt supply for the audio codec >>>> + >>>> +patternProperties: >>>> + "^dai-link-[0-9]+$": >>>> + type: object >>>> + description: >>>> + Container for dai-link level properties and CODEC sub-nodes. >>>> + >>>> + properties: >>>> + codec: >>>> + type: object >>>> + description: Holds subnode which indicates codec dai. >>>> + >>>> + properties: >>>> + sound-dai: >>>> + maxItems: 1 >>>> + description: phandle of the codec DAI >>>> + >>>> + additionalProperties: false >>>> + >>>> + link-name: >>>> + description: >>>> + This property corresponds to the name of the BE dai-link >>>> to which >>>> + we are going to update parameters in this node. >>>> + items: >>>> + const: 2ND I2S BE >>>> + >>>> + sound-dai: >>>> + maxItems: 1 >>>> + description: phandle of the CPU DAI >>>> + >>>> + additionalProperties: false >>>> + >>>> + required: >>>> + - link-name >>>> + - sound-dai >>>> + >>>> +additionalProperties: false >>>> + >>>> +required: >>>> + - compatible >>>> + - mediatek,platform >>>> + - pinctrl-names >>>> + - vaud28-supply >>>> + >>>> +examples: >>>> + - | >>>> + sound { >>>> + compatible = "mediatek,mt8365-mt6357"; >>>> + mediatek,platform = <&afe>; >>> >>> Please: >>> >>> https://docs.kernel.org/devicetree/bindings/dts-coding-style.html >> >> Is it about the wrong pinctrl-names tab alignment ? >> Also, 2ND I2S BE => 2ND_I2S_BE ? >> Otherwise, I don't get it sorry. >> > > ...as Krzysztof already clarified, won't repeat :-P Yes it is, I will fix that. > > Cheers! > >>> >>> Regards, >>> Angelo >>> >>>> + pinctrl-names = "aud_default", >>>> + "aud_dmic", >>>> + "aud_miso_off", >>>> + "aud_miso_on", >>>> + "aud_mosi_off", >>>> + "aud_mosi_on"; >>>> + pinctrl-0 = <&aud_default_pins>; >>>> + pinctrl-1 = <&aud_dmic_pins>; >>>> + pinctrl-2 = <&aud_miso_off_pins>; >>>> + pinctrl-3 = <&aud_miso_on_pins>; >>>> + pinctrl-4 = <&aud_mosi_off_pins>; >>>> + pinctrl-5 = <&aud_mosi_on_pins>; >>>> + vaud28-supply = <&mt6357_vaud28_reg>; >>>> + >>>> + /* hdmi interface */ >>>> + dai-link-0 { >>>> + sound-dai = <&afe>; >>>> + link-name = "2ND I2S BE"; >>>> + >>>> + codec { >>>> + sound-dai = <&it66121hdmitx>; >>>> + }; >>>> + }; >>>> + }; >>>> >>> >> > >
diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml new file mode 100644 index 000000000000..f469611ec6b6 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml @@ -0,0 +1,127 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-mt6357.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Mediatek MT8365 sound card with MT6357 sound codec. + +maintainers: + - Alexandre Mergnat <amergnat@baylibre.com> + +description: + This binding describes the MT8365 sound card. + +properties: + compatible: + const: mediatek,mt8365-mt6357 + + mediatek,hp-pull-down: + description: + Earphone driver positive output stage short to the + audio reference ground. + Default value is false. + type: boolean + + mediatek,micbias0-microvolt: + description: | + Selects MIC Bias 0 output voltage. + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] + enum: [0, 1, 2, 3, 4, 5, 6, 7] + + mediatek,micbias1-microvolt: + description: | + Selects MIC Bias 1 output voltage. + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] + enum: [0, 1, 2, 3, 4, 5, 6, 7] + + mediatek,platform: + $ref: /schemas/types.yaml#/definitions/phandle + description: The phandle of MT8365 ASoC platform. + + pinctrl-names: + minItems: 1 + items: + - const: aud_default + - const: aud_dmic + - const: aud_miso_off + - const: aud_miso_on + - const: aud_mosi_off + - const: aud_mosi_on + + vaud28-supply: + description: + 2.8 volt supply for the audio codec + +patternProperties: + "^dai-link-[0-9]+$": + type: object + description: + Container for dai-link level properties and CODEC sub-nodes. + + properties: + codec: + type: object + description: Holds subnode which indicates codec dai. + + properties: + sound-dai: + maxItems: 1 + description: phandle of the codec DAI + + additionalProperties: false + + link-name: + description: + This property corresponds to the name of the BE dai-link to which + we are going to update parameters in this node. + items: + const: 2ND I2S BE + + sound-dai: + maxItems: 1 + description: phandle of the CPU DAI + + additionalProperties: false + + required: + - link-name + - sound-dai + +additionalProperties: false + +required: + - compatible + - mediatek,platform + - pinctrl-names + - vaud28-supply + +examples: + - | + sound { + compatible = "mediatek,mt8365-mt6357"; + mediatek,platform = <&afe>; + pinctrl-names = "aud_default", + "aud_dmic", + "aud_miso_off", + "aud_miso_on", + "aud_mosi_off", + "aud_mosi_on"; + pinctrl-0 = <&aud_default_pins>; + pinctrl-1 = <&aud_dmic_pins>; + pinctrl-2 = <&aud_miso_off_pins>; + pinctrl-3 = <&aud_miso_on_pins>; + pinctrl-4 = <&aud_mosi_off_pins>; + pinctrl-5 = <&aud_mosi_on_pins>; + vaud28-supply = <&mt6357_vaud28_reg>; + + /* hdmi interface */ + dai-link-0 { + sound-dai = <&afe>; + link-name = "2ND I2S BE"; + + codec { + sound-dai = <&it66121hdmitx>; + }; + }; + };
Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec. Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> --- .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 +++++++++++++++++++++ 1 file changed, 127 insertions(+)