diff mbox series

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

Message ID 20230421100905.28045-8-trevor.wu@mediatek.com
State New
Headers
Series ASoC: mediatek: mt8188: revise AFE driver |

Commit Message

Trevor Wu (吳文良) April 21, 2023, 10:09 a.m. UTC 
  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@mediatek.com>
---
 .../bindings/sound/mediatek,mt8188-afe.yaml      | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski April 21, 2023, 4:43 p.m. UTC | #1 
On 21/04/2023 12:09, 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@mediatek.com>
> ---
>  .../bindings/sound/mediatek,mt8188-afe.yaml      | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml
> index 82ccb32f08f2..812e0702ca36 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
>  
> @@ -52,6 +56,7 @@ properties:
>        - description: mux for i2si1_mck
>        - description: mux for i2si2_mck
>        - description: audio 26m clock
> +      - description: audio pll1 divide 4
>  
>    clock-names:
>      items:
> @@ -73,6 +78,7 @@ properties:
>        - const: i2si1_m_sel
>        - const: i2si2_m_sel
>        - const: adsp_audio_26m
> +      - const: apll1_d4
>  
>    mediatek,etdm-in1-cowork-source:
>      $ref: /schemas/types.yaml#/definitions/uint32
> @@ -147,6 +153,8 @@ required:
>    - power-domains
>    - clocks
>    - clock-names
> +  - assigned-clocks
> +  - assigned-clock-parents

You were explaining it last time, but it did not solve my concerns.
Requiring these properties means that your hardware boots with incorrect
clock parents, including result of any firmware, and there is no way it
can correctly work without reparenting. What's more, this means that
your clock hierarchy does not include these clocks for some reason, e.g.
you need to reparent parents of some parent of your clock input,
otherwise device cannot work. Cannot work never ever.

Is this the case?

Have in mind that bindings are used also by other OS and projects, like
bootloaders, firmware etc.

Best regards,
Krzysztof
Trevor Wu (吳文良) April 24, 2023, 3:31 a.m. UTC | #2 
On Fri, 2023-04-21 at 18:43 +0200, Krzysztof Kozlowski wrote:
> On 21/04/2023 12:09, 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@mediatek.com>
> > ---
> >  .../bindings/sound/mediatek,mt8188-afe.yaml      | 16
> > ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml
> > b/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml
> > index 82ccb32f08f2..812e0702ca36 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
> > 
> > @@ -52,6 +56,7 @@ properties:
> >        - description: mux for i2si1_mck
> >        - description: mux for i2si2_mck
> >        - description: audio 26m clock
> > +      - description: audio pll1 divide 4
> > 
> >    clock-names:
> >      items:
> > @@ -73,6 +78,7 @@ properties:
> >        - const: i2si1_m_sel
> >        - const: i2si2_m_sel
> >        - const: adsp_audio_26m
> > +      - const: apll1_d4
> > 
> >    mediatek,etdm-in1-cowork-source:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > @@ -147,6 +153,8 @@ required:
> >    - power-domains
> >    - clocks
> >    - clock-names
> > +  - assigned-clocks
> > +  - assigned-clock-parents
> 
> You were explaining it last time, but it did not solve my concerns.
> Requiring these properties means that your hardware boots with
> incorrect
> clock parents, including result of any firmware, and there is no way
> it
> can correctly work without reparenting. What's more, this means that
> your clock hierarchy does not include these clocks for some reason,
> e.g.
> you need to reparent parents of some parent of your clock input,
> otherwise device cannot work. Cannot work never ever.
> 
> Is this the case?

Thanks for your detailed explanation.
I just tried to initialize the clock to another clock parent for power
saving, so this is not the case. In other words, it's not a "must be"
property. I will remove them in V3.

Thanks,
Trevor

> 
> Have in mind that bindings are used also by other OS and projects,
> like
> bootloaders, firmware etc.
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-afe.yaml
index 82ccb32f08f2..812e0702ca36 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
 
@@ -52,6 +56,7 @@  properties:
       - description: mux for i2si1_mck
       - description: mux for i2si2_mck
       - description: audio 26m clock
+      - description: audio pll1 divide 4
 
   clock-names:
     items:
@@ -73,6 +78,7 @@  properties:
       - const: i2si1_m_sel
       - const: i2si2_m_sel
       - const: adsp_audio_26m
+      - const: apll1_d4
 
   mediatek,etdm-in1-cowork-source:
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -147,6 +153,8 @@  required:
   - power-domains
   - clocks
   - clock-names
+  - assigned-clocks
+  - assigned-clock-parents
 
 additionalProperties: false
 
@@ -184,7 +192,8 @@  examples:
                  <&topckgen 78>, //CLK_TOP_I2SO2
                  <&topckgen 79>, //CLK_TOP_I2SI1
                  <&topckgen 80>, //CLK_TOP_I2SI2
-                 <&adsp_audio26m 0>; //CLK_AUDIODSP_AUDIO26M
+                 <&adsp_audio26m 0>, //CLK_AUDIODSP_AUDIO26M
+                 <&topckgen 136>; //CLK_TOP_APLL1_D4
         clock-names = "clk26m",
                       "apll1",
                       "apll2",
@@ -202,7 +211,10 @@  examples:
                       "i2so2_m_sel",
                       "i2si1_m_sel",
                       "i2si2_m_sel",
-                      "adsp_audio_26m";
+                      "adsp_audio_26m",
+                      "apll1_d4";
+        assigned-clocks = <&topckgen 83>; //CLK_TOP_A1SYS_HP
+        assigned-clock-parents =  <&clk26m>;
     };
 
 ...