Re: [PATCH v4 13/13] dt-bindings: mediatek: mt8188: add mt8188-mt6359 document

From: "Trevor Wu (吳文良)" <Trevor.Wu@mediatek.com>
To: "robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski@linaro.org" <krzysztof.kozlowski@linaro.org>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
	"tiwai@suse.com" <tiwai@suse.com>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"krzysztof.kozlowski+dt@linaro.org" 
	<krzysztof.kozlowski+dt@linaro.org>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"perex@perex.cz" <perex@perex.cz>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mediatek@lists.infradead.org" 
	<linux-mediatek@lists.infradead.org>,
	"angelogioacchino.delregno@collabora.com" 
	<angelogioacchino.delregno@collabora.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Project_Global_Chrome_Upstream_Group 
	<Project_Global_Chrome_Upstream_Group@mediatek.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH v4 13/13] dt-bindings: mediatek: mt8188: add mt8188-mt6359 document
Date: Tue, 3 Jan 2023 06:57:13 +0000	[thread overview]
Message-ID: <a3c4df4278e64d9e7e9a77c44215de281b0d6d75.camel@mediatek.com> (raw)
In-Reply-To: <a885bf4b-253d-75fe-0224-02f8dfb44006@linaro.org>

On Mon, 2023-01-02 at 17:32 +0100, Krzysztof Kozlowski wrote:
> On 30/12/2022 06:54, Trevor Wu wrote:
> > Add document for mt8188 board with mt6359.
> 
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the
> directory
> your patch is touching).
> 
Hi Krzysztof,

I'm sorry, and I'll add "ASoC" prefix in v5.

> > 
> > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> > ---
> >  .../sound/mediatek,mt8188-mt6359.yaml         | 93
> > +++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > mt6359.yaml
> > b/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > mt6359.yaml
> > new file mode 100644
> > index 000000000000..5754c1d460db
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > mt6359.yaml
> > @@ -0,0 +1,93 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt8188-mt6359.yaml*__;Iw!!CTRNKA9wMg0ARbw!nY7vxevqo8ILGHUn3JesshmMbBP7WhMOLpwDruvd_r_YINu99BUXrajVZU9xY2zEGFUXxtmrBOofaabZqwt22P8zOMZtkA$ 
> >  
> > +$schema: 
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!nY7vxevqo8ILGHUn3JesshmMbBP7WhMOLpwDruvd_r_YINu99BUXrajVZU9xY2zEGFUXxtmrBOofaabZqwt22P9Ehlwj2w$ 
> >  
> > +
> > +title: MediaTek MT8188 ASoC sound card
> > +
> > +maintainers:
> > +  - Trevor Wu <trevor.wu@mediatek.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt8188-mt6359-evb
> > +
> > +  model:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description: User specified audio sound card name
> > +
> > +  audio-routing:
> > +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> > +    description:
> > +      A list of the connections between audio components. Each
> > entry is a
> > +      sink/source pair of strings. Valid names could be the input
> > or output
> > +      widgets of audio components, power supplies, MicBias of
> > codec and the
> > +      software switch.
> > +
> > +  mediatek,platform:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: The phandle of MT8188 ASoC platform.
> > +
> > +patternProperties:
> > +  "^dai-link-[0-9]+$":
> > +    type: object
> 
> On this level:
> additionalProperties: false
> 
OK. I'll add it.

> > +    description: |
> > +      Container for dai-link level properties and CODEC sub-nodes.
> > +
> > +    properties:
> > +      dai-link-name:
> 
> Instead:
> link-name

OK. I'll replace it in v5.

> > +        description: |
> > +          This property corresponds to the name of the BE dai-link 
> > to which
> > +          we are going to update parameters in this node.
> > +        items:
> > +          enum:
> > +            - ADDA_BE
> > +            - DPTX_BE
> > +            - ETDM1_IN_BE
> > +            - ETDM2_IN_BE
> > +            - ETDM1_OUT_BE
> > +            - ETDM2_OUT_BE
> > +            - ETDM3_OUT_BE
> > +            - PCM1_BE
> > +
> > +      codec:
> > +        description: Holds subnode which indicates codec dai.
> > +        type: object
> > +        additionalProperties: false
> > +        properties:
> > +          sound-dai:
> > +            minItems: 1
> > +            maxItems: 2
> 
> required sound-dai
> 
Thanks, I will add it in v5.

> > +
> > +    required:
> > +      - dai-link-name
> > +      - codec
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - mediatek,platform
> > +
> 
> Your example looks pretty straightforward. Maybe you can use
> simple-card.yaml?	
> 

There is a major difference between simple card and MediaTek sound
card.
In MediaTek sound card, all supported dai-links are listed in the
machine driver, so the dai-link property is only used to introduce the
connected codec nodes. 
On the other hand, we hope to inherit the implementation of mt8195 in
which one machine driver supports both sof and non-sof cards, so we
make use of "mediatek, platform" and "mediatek,adsp" properties.
If I don't misunderstand the implementation of simple card, I have to
specify every platform node by dai-link array in dts file and it's more
complex than the current implementation.

Thanks,
Trevor