Re: [PATCH 7/7] ASoC: dt-bindings: mediatek,mt8188-afe: add audio properties

[Trevor Wu (吳文良)]

On Sat, 2023-04-15 at 11:00 +0200, Krzysztof Kozlowski wrote:
> 
> On 13/04/2023 12:47, Trevor Wu wrote:
> > Assign top_a1sys_hp clock to 26M, and add apll1_d4 to clocks for
> > switching
> > the parent of top_a1sys_hp dynamically
> > On the other hand, "mediatek,infracfg" is included for bus
> > protection.
> > 
> > Signed-off-by: Trevor Wu <trevor.wu@xxxxxxxxxxxx>
> > ---
> >  .../bindings/sound/mediatek,mt8188-afe.yaml    | 18
> > ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml
> > b/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml
> > index 82ccb32f08f2..03301d5082f3 100644
> > --- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > afe.yaml
> > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > afe.yaml
> > @@ -29,6 +29,10 @@ properties:
> >      $ref: /schemas/types.yaml#/definitions/phandle
> >      description: The phandle of the mediatek topckgen controller
> > 
> > +  mediatek,infracfg:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: The phandle of the mediatek infracfg controller
> > +
> >    power-domains:
> >      maxItems: 1
> > 
> > @@ -37,6 +41,7 @@ properties:
> >        - description: 26M clock
> >        - description: audio pll1 clock
> >        - description: audio pll2 clock
> > +      - description: audio pll1 divide 4
> >        - description: clock divider for i2si1_mck
> >        - description: clock divider for i2si2_mck
> >        - description: clock divider for i2so1_mck
> > @@ -58,6 +63,7 @@ properties:
> >        - const: clk26m
> >        - const: apll1
> >        - const: apll2
> > +      - const: apll1_d4
> 
> Why do you add clocks in the middle? The order is strict, so you just
> broke all DTS.
> 

In DTS file, I only need to make sure that the order in clocks should
be the same as clock-names, so I misunderstood that I can add the clock
in the middle based on the clock type.

Sorry, I didn't know the order is strict. I will move the new clock to
the last one in v2.

> >        - const: apll12_div0
> >        - const: apll12_div1
> >        - const: apll12_div2
> > @@ -74,6 +80,12 @@ properties:
> >        - const: i2si2_m_sel
> >        - const: adsp_audio_26m
> > 
> > +  assigned-clocks:
> > +    maxItems: 1
> > +
> > +  assigned-clock-parents:
> > +    maxItems: 1
> 
> Usually these two are not needed.
> 
OK. I will remove these two properties in v2.

> > +
> >    mediatek,etdm-in1-cowork-source:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> >      description:
> > @@ -147,6 +159,8 @@ required:
> >    - power-domains
> >    - clocks
> >    - clock-names
> > +  - assigned-clocks
> > +  - assigned-clock-parents
> 
> Why making them required?

As I mentioned in the commit message, switching the parent of
top_a1sys_hp was included in the series. I make use of these two
properties to initialize the clock to 26MHz, so APLL1 can be enabled
only when it's really required.


Thanks,
Trevor
>