diff mbox series

[7/7] ASoC: dt-bindings: mediatek,mt79xx-afe: add audio afe document

Message ID 20230612105250.15441-8-maso.huang@mediatek.com (mailing list archive)
State New, archived
Headers show
Series ASoC: mediatek: Add support for MT79xx SoC | expand

Commit Message

Maso Huang (黃加竹) June 12, 2023, 10:52 a.m. UTC
From: Maso Huang <maso.huang@mediatek.com>

Add mt79xx audio afe document.

Signed-off-by: Maso Huang <maso.huang@mediatek.com>
---
 .../bindings/sound/mediatek,mt79xx-afe.yaml   | 102 ++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml

Comments

Krzysztof Kozlowski June 13, 2023, 8:51 a.m. UTC | #1
On 12/06/2023 12:52, Maso Hunag wrote:
> From: Maso Huang <maso.huang@mediatek.com>
> 
> Add mt79xx audio afe document.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

> 
> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
> ---
>  .../bindings/sound/mediatek,mt79xx-afe.yaml   | 102 ++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
> new file mode 100644
> index 000000000000..11ef1cfdf49b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mediatek,mt79xx-afe.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek AFE PCM controller for MT79xx

79XX sounds weird. Are you sure you are not using wildcards (which are
not allowed)?

> +
> +maintainers:
> +  - Maso Huang <maso.huang@mediatek.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: mediatek,mt79xx-afe
> +      - items:
> +          - enum:
> +              - mediatek,mt7981-afe
> +              - mediatek,mt7986-afe
> +              - mediatek,mt7988-afe
> +          - const: mediatek,mt79xx-afe

I already saw AFE, why it cannot be part of existing bindings?

This list is odd. 79xx, 7981? So it is wildcard?

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 5
> +    items:
> +      - description: audio bus clock
> +      - description: audio 26M clock
> +      - description: audio intbus clock
> +      - description: audio hopping clock
> +      - description: audio pll clock
> +      - description: mux for pcm_mck
> +      - description: audio i2s/pcm mck
> +
> +  clock-names:
> +    minItems: 5
> +    items:
> +      - const: aud_bus_ck
> +      - const: aud_26m_ck
> +      - const: aud_l_ck
> +      - const: aud_aud_ck
> +      - const: aud_eg2_ck
> +      - const: aud_sel
> +      - const: aud_i2s_m

Why this is variable?

> +
> +  assigned-clocks:
> +    minItems: 3
> +    maxItems: 4

Drop assigned-clocks

> +
> +  assigned-clock-parents:
> +    minItems: 3
> +    maxItems: 4

Drop



Best regards,
Krzysztof
Krzysztof Kozlowski June 14, 2023, 6:34 a.m. UTC | #2
On 14/06/2023 05:17, Maso Huang (黃加竹) wrote:
> On Tue, 2023-06-13 at 10:51 +0200, Krzysztof Kozlowski wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  
>> On 12/06/2023 12:52, Maso Hunag wrote:
>>> From: Maso Huang <maso.huang@mediatek.com>
>>>
>>> Add mt79xx audio afe document.
>>
>> Please use scripts/get_maintainers.pl to get a list of necessary
>> people
>> and lists to CC.  It might happen, that command when run on an older
>> kernel, gives you outdated entries.  Therefore please be sure you
>> base
>> your patches on recent Linux kernel.
>>
> 
> Hi Krzysztif, 
> 
> Thanks for your review. And sorry for missing some necessary
> maintainers.
> What's your suggestion, resend these patches again with them, or add
> them back in v2 patch?


You need to fix the patch anyway, so use get_maintainers.pl in v2. I
don't understand why you Cc here many unrelated people but not the
actual maintainers which get_maintainers.pl asks you to Cc!

> 
>>>
>>> Signed-off-by: Maso Huang <maso.huang@mediatek.com>
>>> ---
>>>  .../bindings/sound/mediatek,mt79xx-afe.yaml   | 102
>> ++++++++++++++++++
>>>  1 file changed, 102 insertions(+)
>>>  create mode 100644
>> Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
>> b/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
>>> new file mode 100644
>>> index 000000000000..11ef1cfdf49b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt79xx-
>> afe.yaml
>>> @@ -0,0 +1,102 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/sound/mediatek,mt79xx-afe.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek AFE PCM controller for MT79xx
>>
>> 79XX sounds weird. Are you sure you are not using wildcards (which
>> are
>> not allowed)?
>>
> 
> We would like to use mt79xx for mt7986/mt7981/mt7988 series.
> Or is it better to just use mt7986 for this series?

You cannot use wildcard. Get some internal review of your patches prior
to submission to mailing list.

https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst

> 
>>> +
>>> +maintainers:
>>> +  - Maso Huang <maso.huang@mediatek.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - const: mediatek,mt79xx-afe
>>> +      - items:
>>> +          - enum:
>>> +              - mediatek,mt7981-afe
>>> +              - mediatek,mt7986-afe
>>> +              - mediatek,mt7988-afe
>>> +          - const: mediatek,mt79xx-afe
>>
>> I already saw AFE, why it cannot be part of existing bindings?

Can you answer this?

>>
>> This list is odd. 79xx, 7981? So it is wildcard?
>>
> 
> Yes, it is wildcard for mt7986/mt7981/mt7988 series.
> Is it better to just use mt7986 for this series? 

No wildcards.


Best regards,
Krzysztof
Krzysztof Kozlowski June 14, 2023, 8:21 a.m. UTC | #3
On 14/06/2023 09:37, Maso Huang (黃加竹) wrote:
>>>> I already saw AFE, why it cannot be part of existing bindings?
>>
>> Can you answer this?
>>
> 
> Did you mean mtk-afe-pcm.txt?
> If yes, I'll modify mtk-afe-pcm.txt to yaml format, and add mt7986 to
> its compatible list.
> 

No, I meant mediatek,mt8188-afe.yaml.

Aren't you working on some old tree? If so, please don't...

Best regards,
Krzysztof
Krzysztof Kozlowski June 14, 2023, 10:26 a.m. UTC | #4
On 14/06/2023 11:19, Maso Huang (黃加竹) wrote:
> On Wed, 2023-06-14 at 10:21 +0200, Krzysztof Kozlowski wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  On 14/06/2023 09:37, Maso Huang (黃加竹) wrote:
>>>>>> I already saw AFE, why it cannot be part of existing bindings?
>>>>
>>>> Can you answer this?
>>>>
>>>
>>> Did you mean mtk-afe-pcm.txt?
>>> If yes, I'll modify mtk-afe-pcm.txt to yaml format, and add mt7986
>> to
>>> its compatible list.
>>>
>>
>> No, I meant mediatek,mt8188-afe.yaml.
>>
>> Aren't you working on some old tree? If so, please don't...
>>
>> Best regards,
>> Krzysztof
>>
> 
> Hi Krzysztof,
> AFE is common name for our audio hardware, and the design might be
> different for soc, like clock.
> 
> And the design is the same for mt7981/mt7986/mt7988.
> Is it better to create a new dtbinding file mediatk,mt7986-afe.yaml?

Is it different? That was my question whether it can be part of existing
bindings.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
new file mode 100644
index 000000000000..11ef1cfdf49b
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt79xx-afe.yaml
@@ -0,0 +1,102 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mediatek,mt79xx-afe.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek AFE PCM controller for MT79xx
+
+maintainers:
+  - Maso Huang <maso.huang@mediatek.com>
+
+properties:
+  compatible:
+    oneOf:
+      - const: mediatek,mt79xx-afe
+      - items:
+          - enum:
+              - mediatek,mt7981-afe
+              - mediatek,mt7986-afe
+              - mediatek,mt7988-afe
+          - const: mediatek,mt79xx-afe
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 5
+    items:
+      - description: audio bus clock
+      - description: audio 26M clock
+      - description: audio intbus clock
+      - description: audio hopping clock
+      - description: audio pll clock
+      - description: mux for pcm_mck
+      - description: audio i2s/pcm mck
+
+  clock-names:
+    minItems: 5
+    items:
+      - const: aud_bus_ck
+      - const: aud_26m_ck
+      - const: aud_l_ck
+      - const: aud_aud_ck
+      - const: aud_eg2_ck
+      - const: aud_sel
+      - const: aud_i2s_m
+
+  assigned-clocks:
+    minItems: 3
+    maxItems: 4
+
+  assigned-clock-parents:
+    minItems: 3
+    maxItems: 4
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - assigned-clocks
+  - assigned-clock-parents
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/mediatek,mt7981-clk.h>
+
+    afe@11210000 {
+        compatible = "mediatek,mt7981-afe","mediatek,mt79xx-afe";
+        reg = <0x11210000 0x9000>;
+        interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&infracfg_ao CLK_INFRA_AUD_BUS_CK>,
+                 <&infracfg_ao CLK_INFRA_AUD_26M_CK>,
+                 <&infracfg_ao CLK_INFRA_AUD_L_CK>,
+                 <&infracfg_ao CLK_INFRA_AUD_AUD_CK>,
+                 <&infracfg_ao CLK_INFRA_AUD_EG2_CK>,
+                 <&topckgen CLK_TOP_AUD_SEL>;
+        clock-names = "aud_bus_ck",
+                      "aud_26m_ck",
+                      "aud_l_ck",
+                      "aud_aud_ck",
+                      "aud_eg2_ck",
+                      "aud_sel";
+        assigned-clocks = <&topckgen CLK_TOP_AUD_SEL>,
+                          <&topckgen CLK_TOP_A1SYS_SEL>,
+                          <&topckgen CLK_TOP_AUD_L_SEL>,
+                          <&topckgen CLK_TOP_A_TUNER_SEL>;
+        assigned-clock-parents = <&topckgen CLK_TOP_CB_APLL2_196M>,
+                                 <&topckgen CLK_TOP_APLL2_D4>,
+                                 <&topckgen CLK_TOP_CB_APLL2_196M>,
+                                 <&topckgen CLK_TOP_APLL2_D4>;
+    };
+
+...