Re: [v5 1/4] ASoC: dt-bindings: mt8192-mt6359: add new compatible and new properties

[Jiaxin Yu <jiaxin.yu@mediatek.com>]

On Mon, 2022-03-21 at 18:33 -0500, Rob Herring wrote:
> On Sat, Mar 19, 2022 at 07:41:08PM +0800, Jiaxin Yu wrote:
> > 1. Adds new compatible string "mt8192_mt6359_rt1015p_rt5682s" for
> > machines
> > with rt1015p and rt5682s.
> > 2. Adds new property "mediatek,headset-codec" for getting headset
> > codecs.
> > 3. Adds new property "mediatek,speaker-codec" for getting speaker
> > codecs.
> > 
> > Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
> > ---
> >  .../sound/mt8192-mt6359-rt1015-rt5682.yaml    | 29
> > +++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/mt8192-mt6359-
> > rt1015-rt5682.yaml
> > b/Documentation/devicetree/bindings/sound/mt8192-mt6359-rt1015-
> > rt5682.yaml
> > index a781e7aaaa38..aa0476eedd38 100644
> > --- a/Documentation/devicetree/bindings/sound/mt8192-mt6359-rt1015-
> > rt5682.yaml
> > +++ b/Documentation/devicetree/bindings/sound/mt8192-mt6359-rt1015-
> > rt5682.yaml
> > @@ -18,6 +18,7 @@ properties:
> >      enum:
> >        - mediatek,mt8192_mt6359_rt1015_rt5682
> >        - mediatek,mt8192_mt6359_rt1015p_rt5682
> > +      - mediatek,mt8192_mt6359_rt1015p_rt5682s
> >  
> >    mediatek,platform:
> >      $ref: "/schemas/types.yaml#/definitions/phandle"
> > @@ -27,6 +28,25 @@ properties:
> >      $ref: "/schemas/types.yaml#/definitions/phandle"
> >      description: The phandle of HDMI codec.
> >  
> > +patternProperties:
> > +  "^mediatek,headset-codec$":
> 
> Fixed string, not a pattern. Move to 'properties'. Drop the vendor 
> prefix too.
> 
Hi Rob,

I originally referred to simple-card.yaml and move "xxx.yyy" to the
patternProperties.

Such as:
# use patternProperties to avoid naming "xxx,yyy" issue
patternProperties:
  "^simple-audio-card,widgets$":
    $ref: "#/definitions/widgets"

But your comment is more reasonable. I will move them to 'properties'
and drop the vendor prefix. Thanks for your review.


> > +    description: Holds subnode which indicates headset dai.
> > +    type: object
> > +    properties:
> > +      sound-dai:
> > +        $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> Standard property, don't need a type or description. Just how many
> and 
> what each one is if more than 1.

Should I describe them as below?

properties:
...
  speaker-codec:
    type: object
    properties:
      sound-dai:
         maxItems: 1
...

> 
> > +        description: List of phandles to the headset codec nodes.
> 
> More than 1?
> 

Sorry, the description here is incorrect, there is only one phandle.

> > +    additionalProperties: false
> > +
> > +  "^mediatek,speaker-codec$":
> > +    description: Holds subnode which indicates speaker dai.
> > +    type: object
> > +    properties:
> > +      sound-dai:
> > +        $ref: /schemas/types.yaml#/definitions/phandle-array
> > +        description: List of phandles to the speaker codec nodes.
> 
> Same here.
> 
> > +    additionalProperties: false
> > +
> >  additionalProperties: false
> >  
> >  required:
> > @@ -44,6 +64,15 @@ examples:
> >                          "aud_clk_mosi_on";
> >          pinctrl-0 = <&aud_clk_mosi_off>;
> >          pinctrl-1 = <&aud_clk_mosi_on>;
> > +
> > +        mediatek,headset-codec {
> > +            sound-dai = <&rt5682>;
> > +        };
> > +
> > +        mediatek,speaker-codec {
> > +            sound-dai = <&rt1015_l>,
> > +                        <&rt1015_r>;
> > +        };
> >      };
> >  
> >  ...
> > -- 
> > 2.18.0
> > 
> >