On 5/20/24 12:39 PM, Krzysztof Kozlowski wrote: > On 18/05/2024 10:16, Kartik Agarwala wrote: >> Convert Mediatek MT6358 Audio Codec bindings from text to dtschema. >> >> Signed-off-by: Kartik Agarwala <agarwala.kartik@xxxxxxxxx> >> --- >> .../bindings/sound/mediatek,mt6358.yaml | 47 +++++++++++++++++++ >> .../devicetree/bindings/sound/mt6358.txt | 26 ---------- >> 2 files changed, 47 insertions(+), 26 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml >> delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt >> >> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml >> new file mode 100644 >> index 000000000..f57ef2aa5 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml >> @@ -0,0 +1,47 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/sound/mediatek,mt6358.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Mediatek MT6358 Audio Codec >> + >> +maintainers: >> + - Kartik Agarwala <agarwala.kartik@xxxxxxxxx> >> + >> +description: | > > Do not need '|' unless you need to preserve formatting. Noted > >> + The communication between MT6358 and SoC is through Mediatek PMIC wrapper. >> + For more detail, please visit Mediatek PMIC wrapper documentation. >> + Must be a child node of PMIC wrapper. > > Did you update the PMIC wrapper binding with ref to this? I am sorry but if I understand this comment, you are asking me to update this file [1], correct? 1. https://www.kernel.org/doc/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt > >> + >> +properties: >> + compatible: >> + enum: >> + - mediatek,mt6358-sound >> + - mediatek,mt6366-sound > > You did not test the DTS. > > I think I raised the issue already: please make necessary fixes to the > binding (with explanation) or to the DTS, when converting the binding. > Apologies again. Just to be sure, am I correct to assume that you want me to fix the dts file [1] as it has both these compatibles mentioned instead of only one and I should fix that by dropping one of the two compatibles? [1] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8186-corsola.dtsi#L1246 >> + >> + Avdd-supply: >> + description: power source of AVDD >> + >> + mediatek,dmic-mode: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: | > > Do not need '|' unless you need to preserve formatting. > >> + 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 >> + >> +required: >> + - compatible >> + - Avdd-supply >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + mt6358_snd { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > Definitely no underscores. Probably this is "codec" or "audio-codec". Noted > > Best regards, > Krzysztof > Thanks for the review! Regards, Kartik Agarwala
