diff mbox series

[v4,2/4] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support

Message ID dcc060c3ada7a56eda02b586c16c47f0a0905c61.1718697954.git.unicorn_wang@outlook.com (mailing list archive)
State New
Headers show
Series mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support | expand

Commit Message

Chen Wang June 18, 2024, 8:38 a.m. UTC
From: Chen Wang <unicorn_wang@outlook.com>

SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.

SG2042 defines 3 clocks for SD/eMMC controllers.
- AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
  and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
  source, so reuse existing "core".
- 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
  existing "timer" which was added for rockchip specified.
- EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".

Adding example for sg2042.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
---
 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 69 +++++++++++++------
 1 file changed, 49 insertions(+), 20 deletions(-)

Comments

Krzysztof Kozlowski June 18, 2024, 9:39 a.m. UTC | #1
On 18/06/2024 10:38, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.
> 
> SG2042 defines 3 clocks for SD/eMMC controllers.
> - AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
>   and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
>   source, so reuse existing "core".
> - 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
>   existing "timer" which was added for rockchip specified.
> - EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".
> 
> Adding example for sg2042.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> ---
>  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 69 +++++++++++++------
>  1 file changed, 49 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> index 4d3031d9965f..b53f20733f79 100644
> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> @@ -21,6 +21,7 @@ properties:
>        - snps,dwcmshc-sdhci
>        - sophgo,cv1800b-dwcmshc
>        - sophgo,sg2002-dwcmshc
> +      - sophgo,sg2042-dwcmshc
>        - thead,th1520-dwcmshc
>  
>    reg:
> @@ -29,25 +30,6 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> -  clocks:

Widest constraints stay here.

> -    minItems: 1
> -    items:
> -      - description: core clock
> -      - description: bus clock for optional
> -      - description: axi clock for rockchip specified
> -      - description: block clock for rockchip specified
> -      - description: timer clock for rockchip specified
> -
> -
> -  clock-names:
> -    minItems: 1

Widest constraints stay here.

> -    items:
> -      - const: core
> -      - const: bus
> -      - const: axi
> -      - const: block
> -      - const: timer
> -
>    resets:
>      maxItems: 5
>  
> @@ -63,6 +45,43 @@ properties:
>      description: Specify the number of delay for tx sampling.
>      $ref: /schemas/types.yaml#/definitions/uint8
>  
> +if:

This should be under allOf block and move the allOf to expected place
like in example schema. So after required: block.

> +  properties:
> +    compatible:
> +      contains:
> +        const: sophgo,sg2042-dwcmshc
> +then:
> +  properties:
> +    clocks:
> +      items:
> +        - description: core clock
> +        - description: timer clock
> +        - description: card clock

What is the card clock? It's suspicious to see new clock not matching
anything from previous ones.

> +
> +    clock-names:
> +      items:
> +        - const: core
> +        - const: timer
> +        - const: card
> +else:
> +  properties:
> +    clocks:
> +      minItems: 1
> +      items:
> +        - description: core clock
> +        - description: bus clock for optional
> +        - description: axi clock for rockchip specified
> +        - description: block clock for rockchip specified
> +        - description: timer clock for rockchip specified
> +
> +    clock-names:
> +      minItems: 1
> +      items:
> +        - const: core
> +        - const: bus
> +        - const: axi
> +        - const: block
> +        - const: timer
>  
>  required:
>    - compatible
> @@ -96,5 +115,15 @@ examples:
>        #address-cells = <1>;
>        #size-cells = <0>;
>      };
> -
> +  - |
> +    mmc@bb0000 {
> +      compatible = "sophgo,sg2042-dwcmshc";
> +      reg = <0xcc000 0x1000>;
> +      interrupts = <0 25 0x4>;

Use defines... or actually drop entire example, no point to keep growing
them.

> +      clocks = <&cru 17>, <&cru 18>, <&cru 19>;
> +      clock-names = "core", "timer", "card";
> +      bus-width = <8>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +    };
>  ...

Best regards,
Krzysztof
Jisheng Zhang June 20, 2024, 4:15 p.m. UTC | #2
On Tue, Jun 18, 2024 at 04:38:30PM +0800, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.
> 
> SG2042 defines 3 clocks for SD/eMMC controllers.
> - AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
>   and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
>   source, so reuse existing "core".

No, this seems not correct. This should be the "bus" clk, and your above
sentence "aclk/hclk(Bus interface clocks in DWC_mshc)" implies this clk is
for bus

> - 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
>   existing "timer" which was added for rockchip specified.
> - EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".

I think this is "core" clk, no? Plz check which internal clks' clock
source is the so called EMMC_100M/SD_100M.

> 
> Adding example for sg2042.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> ---
>  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 69 +++++++++++++------
>  1 file changed, 49 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> index 4d3031d9965f..b53f20733f79 100644
> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> @@ -21,6 +21,7 @@ properties:
>        - snps,dwcmshc-sdhci
>        - sophgo,cv1800b-dwcmshc
>        - sophgo,sg2002-dwcmshc
> +      - sophgo,sg2042-dwcmshc
>        - thead,th1520-dwcmshc
>  
>    reg:
> @@ -29,25 +30,6 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> -  clocks:
> -    minItems: 1
> -    items:
> -      - description: core clock
> -      - description: bus clock for optional
> -      - description: axi clock for rockchip specified
> -      - description: block clock for rockchip specified
> -      - description: timer clock for rockchip specified
> -
> -
> -  clock-names:
> -    minItems: 1
> -    items:
> -      - const: core
> -      - const: bus
> -      - const: axi
> -      - const: block
> -      - const: timer
> -
>    resets:
>      maxItems: 5
>  
> @@ -63,6 +45,43 @@ properties:
>      description: Specify the number of delay for tx sampling.
>      $ref: /schemas/types.yaml#/definitions/uint8
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: sophgo,sg2042-dwcmshc
> +then:
> +  properties:
> +    clocks:
> +      items:
> +        - description: core clock
> +        - description: timer clock
> +        - description: card clock
> +
> +    clock-names:
> +      items:
> +        - const: core
> +        - const: timer
> +        - const: card
> +else:
> +  properties:
> +    clocks:
> +      minItems: 1
> +      items:
> +        - description: core clock
> +        - description: bus clock for optional
> +        - description: axi clock for rockchip specified
> +        - description: block clock for rockchip specified
> +        - description: timer clock for rockchip specified
> +
> +    clock-names:
> +      minItems: 1
> +      items:
> +        - const: core
> +        - const: bus
> +        - const: axi
> +        - const: block
> +        - const: timer
>  
>  required:
>    - compatible
> @@ -96,5 +115,15 @@ examples:
>        #address-cells = <1>;
>        #size-cells = <0>;
>      };
> -
> +  - |
> +    mmc@bb0000 {
> +      compatible = "sophgo,sg2042-dwcmshc";
> +      reg = <0xcc000 0x1000>;
> +      interrupts = <0 25 0x4>;
> +      clocks = <&cru 17>, <&cru 18>, <&cru 19>;
> +      clock-names = "core", "timer", "card";
> +      bus-width = <8>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +    };
>  ...
> -- 
> 2.25.1
>
Chen Wang June 24, 2024, 6:18 a.m. UTC | #3
On 2024/6/21 0:15, Jisheng Zhang wrote:
> On Tue, Jun 18, 2024 at 04:38:30PM +0800, Chen Wang wrote:
>> From: Chen Wang <unicorn_wang@outlook.com>
>>
>> SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.
>>
>> SG2042 defines 3 clocks for SD/eMMC controllers.
>> - AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
>>    and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
>>    source, so reuse existing "core".
> No, this seems not correct. This should be the "bus" clk, and your above
> sentence "aclk/hclk(Bus interface clocks in DWC_mshc)" implies this clk is
> for bus
>
>> - 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
>>    existing "timer" which was added for rockchip specified.
>> - EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".
> I think this is "core" clk, no? Plz check which internal clks' clock
> source is the so called EMMC_100M/SD_100M.

hi, Jisheng,

Just want to double-confirm, corresponding to the definition of clock in 
the dwc-mshc specification, what's the "core" clock in the 
snps,dwcmshc-sdhci.yaml?

I get following clock definitions in user-guide of dwc-mshc 
specification, in section 1.8 "Speed and Clock Requirements"

- 1.8.1 Bus Interface Clocks,which are aclk and m_hclk/hclk
- 1.8.2 Timer Clocks,which are tmclk,cqetmclk
- 1.8.3 Card Clocks, whichi is cclk
- 1.8.4 Core Base Clock,which is bclk

I used to think "core" is "Core Base Clock".

[......]
Chen Wang July 17, 2024, 8:01 a.m. UTC | #4
On 2024/6/18 17:39, Krzysztof Kozlowski wrote:
> On 18/06/2024 10:38, Chen Wang wrote:
>> From: Chen Wang <unicorn_wang@outlook.com>
>>
>> SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.
>>
>> SG2042 defines 3 clocks for SD/eMMC controllers.
>> - AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
>>    and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
>>    source, so reuse existing "core".
>> - 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
>>    existing "timer" which was added for rockchip specified.
>> - EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".
>>
>> Adding example for sg2042.
>>
>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>> ---
>>   .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 69 +++++++++++++------
>>   1 file changed, 49 insertions(+), 20 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>> index 4d3031d9965f..b53f20733f79 100644
>> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>> @@ -21,6 +21,7 @@ properties:
>>         - snps,dwcmshc-sdhci
>>         - sophgo,cv1800b-dwcmshc
>>         - sophgo,sg2002-dwcmshc
>> +      - sophgo,sg2042-dwcmshc
>>         - thead,th1520-dwcmshc
>>   
>>     reg:
>> @@ -29,25 +30,6 @@ properties:
>>     interrupts:
>>       maxItems: 1
>>   
>> -  clocks:
> Widest constraints stay here.
>
>> -    minItems: 1
>> -    items:
>> -      - description: core clock
>> -      - description: bus clock for optional
>> -      - description: axi clock for rockchip specified
>> -      - description: block clock for rockchip specified
>> -      - description: timer clock for rockchip specified
>> -
>> -
>> -  clock-names:
>> -    minItems: 1
> Widest constraints stay here.

hi, Krzysztof,

Please ask you a question about this widest constraints, I write 
bindings as below:

```yaml

properties:

......

   clocks:
     minItems: 1

   clock-names:
     minItems: 1
......

allOf:
   - $ref: mmc-controller.yaml#

   - if:
       properties:
         compatible:
           contains:
             const: sophgo,sg2042-dwcmshc

     then:
       properties:
         clocks:
           minItems: 1
           items:
             - description: core clock
             - description: bus clock
             - description: timer
         clock-names:
           minItems: 1
           items:
             - const: core
             - const: bus
             - const: timer
     else:
       properties:
         clocks:
           minItems: 1
           items:
             - description: core clock
             - description: bus clock for optional
             - description: axi clock for rockchip specified
             - description: block clock for rockchip specified
             - description: timer clock for rockchip specified
         clock-names:
           minItems: 1
           items:
             - const: core
             - const: bus
             - const: axi
             - const: block
             - const: timer

```

and with DTS as below:

```dts

sd: mmc@704002b000 {
             compatible = "sophgo,sg2042-dwcmshc";
             reg = <0x70 0x4002b000 0x0 0x1000>;
             interrupt-parent = <&intc>;
             interrupts = <136 IRQ_TYPE_LEVEL_HIGH>;
             clocks = <&clkgen GATE_CLK_SD_100M>,
                  <&clkgen GATE_CLK_AXI_SD>,
                  <&clkgen GATE_CLK_100K_SD>;
             clock-names = "core",
                       "bus",
                       "timer";
             status = "disabled";
         };

```

dtb check will report error:

```

.../arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dtb: mmc@704002b000: 
clocks: [[84, 38], [84, 64], [84, 76]] is too long
     from schema $id: 
http://devicetree.org/schemas/mmc/snps,dwcmshc-sdhci.yaml#
.../arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dtb: mmc@704002b000: 
clock-names: ['core', 'bus', 'timer'] is too long
     from schema $id: 
http://devicetree.org/schemas/mmc/snps,dwcmshc-sdhci.yaml#
```

After I removed the widest constraints from the binding file, dtb check 
passes.

Not sure if my understanding is wrong. Do you know what's the problem 
here? Seems the widest constraints is not needed,

Thanks,

Chen


[......]
Krzysztof Kozlowski July 17, 2024, 1:55 p.m. UTC | #5
On 17/07/2024 10:01, Chen Wang wrote:
> 
> On 2024/6/18 17:39, Krzysztof Kozlowski wrote:
>> On 18/06/2024 10:38, Chen Wang wrote:
>>> From: Chen Wang <unicorn_wang@outlook.com>
>>>
>>> SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.
>>>
>>> SG2042 defines 3 clocks for SD/eMMC controllers.
>>> - AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
>>>    and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
>>>    source, so reuse existing "core".
>>> - 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
>>>    existing "timer" which was added for rockchip specified.
>>> - EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".
>>>
>>> Adding example for sg2042.
>>>
>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>>> ---
>>>   .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 69 +++++++++++++------
>>>   1 file changed, 49 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>> index 4d3031d9965f..b53f20733f79 100644
>>> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>> @@ -21,6 +21,7 @@ properties:
>>>         - snps,dwcmshc-sdhci
>>>         - sophgo,cv1800b-dwcmshc
>>>         - sophgo,sg2002-dwcmshc
>>> +      - sophgo,sg2042-dwcmshc
>>>         - thead,th1520-dwcmshc
>>>   
>>>     reg:
>>> @@ -29,25 +30,6 @@ properties:
>>>     interrupts:
>>>       maxItems: 1
>>>   
>>> -  clocks:
>> Widest constraints stay here.
>>
>>> -    minItems: 1
>>> -    items:
>>> -      - description: core clock
>>> -      - description: bus clock for optional
>>> -      - description: axi clock for rockchip specified
>>> -      - description: block clock for rockchip specified
>>> -      - description: timer clock for rockchip specified
>>> -
>>> -
>>> -  clock-names:
>>> -    minItems: 1
>> Widest constraints stay here.
> 
> hi, Krzysztof,
> 
> Please ask you a question about this widest constraints, I write 
> bindings as below:
> 
> ```yaml
> 
> properties:
> 
> ......
> 
>    clocks:
>      minItems: 1
> 
>    clock-names:
>      minItems: 1

So 1000 clocks is correct? You can always look at helpful examples from
my slides... or another example:

https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
index 4d3031d9965f..b53f20733f79 100644
--- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -21,6 +21,7 @@  properties:
       - snps,dwcmshc-sdhci
       - sophgo,cv1800b-dwcmshc
       - sophgo,sg2002-dwcmshc
+      - sophgo,sg2042-dwcmshc
       - thead,th1520-dwcmshc
 
   reg:
@@ -29,25 +30,6 @@  properties:
   interrupts:
     maxItems: 1
 
-  clocks:
-    minItems: 1
-    items:
-      - description: core clock
-      - description: bus clock for optional
-      - description: axi clock for rockchip specified
-      - description: block clock for rockchip specified
-      - description: timer clock for rockchip specified
-
-
-  clock-names:
-    minItems: 1
-    items:
-      - const: core
-      - const: bus
-      - const: axi
-      - const: block
-      - const: timer
-
   resets:
     maxItems: 5
 
@@ -63,6 +45,43 @@  properties:
     description: Specify the number of delay for tx sampling.
     $ref: /schemas/types.yaml#/definitions/uint8
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: sophgo,sg2042-dwcmshc
+then:
+  properties:
+    clocks:
+      items:
+        - description: core clock
+        - description: timer clock
+        - description: card clock
+
+    clock-names:
+      items:
+        - const: core
+        - const: timer
+        - const: card
+else:
+  properties:
+    clocks:
+      minItems: 1
+      items:
+        - description: core clock
+        - description: bus clock for optional
+        - description: axi clock for rockchip specified
+        - description: block clock for rockchip specified
+        - description: timer clock for rockchip specified
+
+    clock-names:
+      minItems: 1
+      items:
+        - const: core
+        - const: bus
+        - const: axi
+        - const: block
+        - const: timer
 
 required:
   - compatible
@@ -96,5 +115,15 @@  examples:
       #address-cells = <1>;
       #size-cells = <0>;
     };
-
+  - |
+    mmc@bb0000 {
+      compatible = "sophgo,sg2042-dwcmshc";
+      reg = <0xcc000 0x1000>;
+      interrupts = <0 25 0x4>;
+      clocks = <&cru 17>, <&cru 18>, <&cru 19>;
+      clock-names = "core", "timer", "card";
+      bus-width = <8>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };
 ...