diff mbox series

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

Message ID 20230426055124.16529-8-trevor.wu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series ASoC: mediatek: mt8188: revise AFE driver | expand

Commit Message

Trevor Wu (吳文良) April 26, 2023, 5:51 a.m. UTC
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          | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) April 27, 2023, 2:48 p.m. UTC | #1
On Wed, Apr 26, 2023 at 01:51:24PM +0800, Trevor Wu wrote:
> 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.

Bus protection? Meaning access controls for the device? If so, there's a 
proposed binding[1] for just that. If that's something Mediatek needs 
too, please participate in that discussion. It's easier to define new 
common bindings if there is more than one user.

> 
> Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> ---
>  .../bindings/sound/mediatek,mt8188-afe.yaml          | 12 ++++++++++--
>  1 file changed, 10 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..eb58de8c0e68 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

A new required entry may break the ABI. If that's okay, it needs to be 
explained in the commit msg.

Rob

[1] https://lore.kernel.org/lkml/cover.1668070216.git.oleksii_moisieiev@epam.com/
Trevor Wu (吳文良) May 5, 2023, 7:31 a.m. UTC | #2
On Thu, 2023-04-27 at 09:48 -0500, Rob Herring wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Wed, Apr 26, 2023 at 01:51:24PM +0800, Trevor Wu wrote:
> > 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.
> 
> Bus protection? Meaning access controls for the device? If so,
> there's a
> proposed binding[1] for just that. If that's something Mediatek needs
> too, please participate in that discussion. It's easier to define new
> common bindings if there is more than one user.
> 

Hi Rob,

Bus protection is a function used to prevent from the unexpected glitch
during the reset. The suggested reset control flow is listed as below.

bus protection on
module reset
bus protection off

I think it is not the same as access control of the device. In
addition, no framework or controller was implemented for MTK bus
protection. Here, I just make use of the property to get the phandle of
infracfg to access the register for bus protection.

> > 
> > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> > ---
> >  .../bindings/sound/mediatek,mt8188-afe.yaml          | 12
> > ++++++++++--
> >  1 file changed, 10 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..eb58de8c0e68 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
> 
> A new required entry may break the ABI. If that's okay, it needs to
> be
> explained in the commit msg.
> 

It should be OK, because the project is still developing and there is
no device tree file for mt8188 upstream. I will add the reason to the
message in v4.

Sorry, I didn't notice that before. I will review my future work and
check if any required property or item should be included to the
binding before mt8188 audio device tree upstream.

Thanks,
Trevor

> Rob
> 
> [1] 
> https://lore.kernel.org/lkml/cover.1668070216.git.oleksii_moisieiev@epam.com/
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..eb58de8c0e68 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
@@ -184,7 +190,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 +209,8 @@  examples:
                       "i2so2_m_sel",
                       "i2si1_m_sel",
                       "i2si2_m_sel",
-                      "adsp_audio_26m";
+                      "adsp_audio_26m",
+                      "apll1_d4";
     };
 
 ...