[v3,2/7] dt-bindings: mmc: mtk-sd: Set clocks based on compatible

Message ID 20221023091247.70586-3-linux@fw-web.de
State New
Headers
Series Add mmc-support for mt7986 |

Commit Message

Frank Wunderlich Oct. 23, 2022, 9:12 a.m. UTC
  From: Nícolas F. R. A. Prado <nfraprado@collabora.com>

The binding was describing a single clock list for all platforms, but
that's not really suitable:

Most platforms using at least 2 clocks (source, hclk), some of them
a third "source_cg". Mt2712 requires an extra 'bus_clk' on some of
its controllers, while mt8192 requires 8 clocks.

Move the clock definitions inside if blocks that match on the
compatibles.

I used Patch from Nícolas F. R. A. Prado and modified it to not using
"not" statement.

Fixes: 59a23395d8aa ("dt-bindings: mmc: Add support for MT8192 SoC")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

---
v2:
- add this patch
v3:
- add blank lines and change "not" to matchlist
- reorder entries - make generic first then order alphanumeric
- rewrite commit description
- drop soc-specific mt8183 - constraints were also set for it above
---
 .../devicetree/bindings/mmc/mtk-sd.yaml       | 113 +++++++++++++-----
 1 file changed, 83 insertions(+), 30 deletions(-)
  

Comments

Krzysztof Kozlowski Oct. 23, 2022, 12:56 p.m. UTC | #1
On 23/10/2022 05:12, Frank Wunderlich wrote:
> From: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> The binding was describing a single clock list for all platforms, but
> that's not really suitable:
> 
> Most platforms using at least 2 clocks (source, hclk), some of them
> a third "source_cg". Mt2712 requires an extra 'bus_clk' on some of
> its controllers, while mt8192 requires 8 clocks.
> 
> Move the clock definitions inside if blocks that match on the
> compatibles.
> 
> I used Patch from Nícolas F. R. A. Prado and modified it to not using
> "not" statement.
> 
> Fixes: 59a23395d8aa ("dt-bindings: mmc: Add support for MT8192 SoC")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
  
Nícolas F. R. A. Prado Oct. 24, 2022, 4:43 p.m. UTC | #2
Hi,

thank you for picking this up.

On Sun, Oct 23, 2022 at 11:12:42AM +0200, Frank Wunderlich wrote:
> From: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> The binding was describing a single clock list for all platforms, but
> that's not really suitable:
> 
> Most platforms using at least 2 clocks (source, hclk), some of them
> a third "source_cg". Mt2712 requires an extra 'bus_clk' on some of
> its controllers, while mt8192 requires 8 clocks.
> 
> Move the clock definitions inside if blocks that match on the
> compatibles.
> 
> I used Patch from Nícolas F. R. A. Prado and modified it to not using
> "not" statement.
> 
> Fixes: 59a23395d8aa ("dt-bindings: mmc: Add support for MT8192 SoC")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> 
> ---
> v2:
> - add this patch
> v3:
> - add blank lines and change "not" to matchlist
> - reorder entries - make generic first then order alphanumeric
> - rewrite commit description
> - drop soc-specific mt8183 - constraints were also set for it above

This is wrong, see below.

> ---
>  .../devicetree/bindings/mmc/mtk-sd.yaml       | 113 +++++++++++++-----
>  1 file changed, 83 insertions(+), 30 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> index 3cbf0208f1b4..31bb6dc329d2 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
[..]
>  
> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        const: mediatek,mt8183-mmc
> -then:
> -  properties:
> -    reg:
> -      minItems: 2

You can't drop this. Nodes with the mt8183 compatible should keep requiring two
reg values. It's not covered by the branch below.

Thanks,
Nícolas

> +allOf:
> +  - $ref: mmc-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - mediatek,mt2701-mmc
> +            - mediatek,mt6779-mmc
> +            - mediatek,mt6795-mmc
> +            - mediatek,mt7620-mmc
> +            - mediatek,mt7622-mmc
> +            - mediatek,mt7623-mmc
> +            - mediatek,mt8135-mmc
> +            - mediatek,mt8173-mmc
> +            - mediatek,mt8183-mmc
> +            - mediatek,mt8186-mmc
> +            - mediatek,mt8188-mmc
> +            - mediatek,mt8195-mmc
> +            - mediatek,mt8516-mmc
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          items:
> +            - description: source clock
> +            - description: HCLK which used for host
> +            - description: independent source clock gate
> +        clock-names:
> +          minItems: 2
> +          items:
> +            - const: source
> +            - const: hclk
> +            - const: source_cg
> +
[..]
  
Frank Wunderlich Oct. 24, 2022, 6:42 p.m. UTC | #3
Am 24. Oktober 2022 18:43:53 MESZ schrieb "Nícolas F. R. A. Prado" <nfraprado@collabora.com>:
>Hi,
>
>thank you for picking this up.
>
>On Sun, Oct 23, 2022 at 11:12:42AM +0200, Frank Wunderlich wrote:
>> From: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>> 
>> The binding was describing a single clock list for all platforms, but
>> that's not really suitable:
>> 
>> Most platforms using at least 2 clocks (source, hclk), some of them
>> a third "source_cg". Mt2712 requires an extra 'bus_clk' on some of
>> its controllers, while mt8192 requires 8 clocks.
>> 
>> Move the clock definitions inside if blocks that match on the
>> compatibles.
>> 
>> I used Patch from Nícolas F. R. A. Prado and modified it to not using
>> "not" statement.
>> 
>> Fixes: 59a23395d8aa ("dt-bindings: mmc: Add support for MT8192 SoC")
>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>> 
>> ---
>> v2:
>> - add this patch
>> v3:
>> - add blank lines and change "not" to matchlist
>> - reorder entries - make generic first then order alphanumeric
>> - rewrite commit description
>> - drop soc-specific mt8183 - constraints were also set for it above
>
>This is wrong, see below.

You are right,will fix in v4

>> ---
>>  .../devicetree/bindings/mmc/mtk-sd.yaml       | 113 +++++++++++++-----
>>  1 file changed, 83 insertions(+), 30 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>> index 3cbf0208f1b4..31bb6dc329d2 100644
>> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>[..]
>>  
>> -if:
>> -  properties:
>> -    compatible:
>> -      contains:
>> -        const: mediatek,mt8183-mmc
>> -then:
>> -  properties:
>> -    reg:
>> -      minItems: 2
>
>You can't drop this. Nodes with the mt8183 compatible should keep requiring two
>reg values. It's not covered by the branch below.
>
>Thanks,
>Nícolas


regards Frank
  

Patch

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
index 3cbf0208f1b4..31bb6dc329d2 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
@@ -10,9 +10,6 @@  maintainers:
   - Chaotian Jing <chaotian.jing@mediatek.com>
   - Wenbin Mei <wenbin.mei@mediatek.com>
 
-allOf:
-  - $ref: mmc-controller.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -49,27 +46,11 @@  properties:
     description:
       Should contain phandle for the clock feeding the MMC controller.
     minItems: 2
-    items:
-      - description: source clock (required).
-      - description: HCLK which used for host (required).
-      - description: independent source clock gate (required for MT2712).
-      - description: bus clock used for internal register access (required for MT2712 MSDC0/3).
-      - description: msdc subsys clock gate (required for MT8192).
-      - description: peripheral bus clock gate (required for MT8192).
-      - description: AXI bus clock gate (required for MT8192).
-      - description: AHB bus clock gate (required for MT8192).
+    maxItems: 7
 
   clock-names:
     minItems: 2
-    items:
-      - const: source
-      - const: hclk
-      - const: source_cg
-      - const: bus_clk
-      - const: sys_cg
-      - const: pclk_cg
-      - const: axi_cg
-      - const: ahb_cg
+    maxItems: 7
 
   interrupts:
     description:
@@ -191,15 +172,87 @@  required:
   - vmmc-supply
   - vqmmc-supply
 
-if:
-  properties:
-    compatible:
-      contains:
-        const: mediatek,mt8183-mmc
-then:
-  properties:
-    reg:
-      minItems: 2
+allOf:
+  - $ref: mmc-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mediatek,mt2701-mmc
+            - mediatek,mt6779-mmc
+            - mediatek,mt6795-mmc
+            - mediatek,mt7620-mmc
+            - mediatek,mt7622-mmc
+            - mediatek,mt7623-mmc
+            - mediatek,mt8135-mmc
+            - mediatek,mt8173-mmc
+            - mediatek,mt8183-mmc
+            - mediatek,mt8186-mmc
+            - mediatek,mt8188-mmc
+            - mediatek,mt8195-mmc
+            - mediatek,mt8516-mmc
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          items:
+            - description: source clock
+            - description: HCLK which used for host
+            - description: independent source clock gate
+        clock-names:
+          minItems: 2
+          items:
+            - const: source
+            - const: hclk
+            - const: source_cg
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt2712-mmc
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          items:
+            - description: source clock
+            - description: HCLK which used for host
+            - description: independent source clock gate
+            - description: bus clock used for internal register access (required for MSDC0/3).
+        clock-names:
+          minItems: 3
+          items:
+            - const: source
+            - const: hclk
+            - const: source_cg
+            - const: bus_clk
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8192-mmc
+    then:
+      properties:
+        clocks:
+          items:
+            - description: source clock
+            - description: HCLK which used for host
+            - description: independent source clock gate
+            - description: msdc subsys clock gate
+            - description: peripheral bus clock gate
+            - description: AXI bus clock gate
+            - description: AHB bus clock gate
+        clock-names:
+          items:
+            - const: source
+            - const: hclk
+            - const: source_cg
+            - const: sys_cg
+            - const: pclk_cg
+            - const: axi_cg
+            - const: ahb_cg
 
 unevaluatedProperties: false