diff mbox series

[3/3] dt-bindings: mmc: dw-mshc-hi3798cv200: rename to dw-mshc-histb

Message ID 20240216-b4-mmc-hi3798mv200-v1-3-7d46db845ae6@outlook.com
State New
Headers
Series mmc: add hi3798mv200 specific extensions of DWMMC |

Commit Message

Yang Xiwen via B4 Relay Feb. 15, 2024, 5:46 p.m. UTC 
  From: Yang Xiwen <forbidden405@outlook.com>

Add binding for Hi3798MV200 DWMMC specific extension.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 ...hi3798cv200-dw-mshc.yaml => histb-dw-mshc.yaml} | 60 +++++++++++++++++++---
 1 file changed, 52 insertions(+), 8 deletions(-)

Comments

Krzysztof Kozlowski Feb. 16, 2024, 8:21 a.m. UTC | #1 
On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
> 
> Add binding for Hi3798MV200 DWMMC specific extension.
> 
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---
>  ...hi3798cv200-dw-mshc.yaml => histb-dw-mshc.yaml} | 60 +++++++++++++++++++---
>  1 file changed, 52 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> similarity index 57%
> rename from Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
> rename to Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> index 5db99cd94b90..d2f5b7bb7a58 100644
> --- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> @@ -1,11 +1,11 @@
>  # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>  %YAML 1.2
>  ---
> -$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
> +$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#

Really, one wrong filename into another...

>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
>  title:
> -  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
> +  Hisilicon HiSTB SoCs specific extensions to the Synopsys DWMMC controller
>  
>  maintainers:
>    - Yang Xiwen <forbidden405@outlook.com>
> @@ -14,16 +14,14 @@ description:
>    The Synopsys designware mobile storage host controller is used to interface
>    a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
>    differences between the core Synopsys dw mshc controller properties described
> -  by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
> -  specific extensions to the Synopsys Designware Mobile Storage Host Controller.

Just drop this sentence in previous/conversion patch. It's useless.

> -
> -allOf:
> -  - $ref: synopsys-dw-mshc-common.yaml#

Put it in correct place in the first time. Don't needlessly shuffle the
code right after previous patch.


> +  by synopsys-dw-mshc.txt and the properties used by the Hisilicon HiSTB specific
> +  extensions to the Synopsys Designware Mobile Storage Host Controller.
>  
>  properties:
>    compatible:
>      enum:
>        - hisilicon,hi3798cv200-dw-mshc
> +      - hisilicon,hi3798mv200-dw-mshc
>  
>    reg:
>      maxItems: 1
> @@ -48,6 +46,12 @@ properties:
>        control the clock phases, "ciu-sample" is required for tuning
>        high speed modes.
>  
> +  hisilicon,sap-dll-reg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A phandle points to the sample delay-locked-loop(DLL)
> +      syscon node, used for tuning.

Does hi3798cv200 have it?

> +
>  required:
>    - compatible
>    - reg
> @@ -55,13 +59,25 @@ required:
>    - clocks
>    - clock-names
>  
> +allOf:
> +  - $ref: synopsys-dw-mshc-common.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: hisilicon,hi3798mv200-dw-mshc
> +    then:
> +      required:
> +        - hisilicon,sap-dll-reg
> +
>  unevaluatedProperties: false
>  
>  examples:
>    - |
>      #include <dt-bindings/clock/histb-clock.h>
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> -    emmc: mmc@9830000 {
> +    mmc@9830000 {

???

>        compatible = "hisilicon,hi3798cv200-dw-mshc";
>        reg = <0x9830000 0x10000>;
>        interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> @@ -84,3 +100,31 @@ examples:
>        bus-width = <8>;
>        status = "okay";
>      };
> +  - |
> +    #include <dt-bindings/clock/histb-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    mmc@9830000 {
> +      compatible = "hisilicon,hi3798mv200-dw-mshc";

No need for new example.

> +      reg = <0x9830000 0x10000>;
> +      interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> +      clocks = <&crg HISTB_MMC_CIU_CLK>,
> +               <&crg HISTB_MMC_BIU_CLK>,
> +               <&crg HISTB_MMC_SAMPLE_CLK>,
> +               <&crg HISTB_MMC_DRV_CLK>;
> +      clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
> +      resets = <&crg 0xa0 4>;
> +      reset-names = "reset";
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&emmc_pins>;
> +      fifo-depth = <256>;
> +      clock-frequency = <50000000>;
> +      max-frequency = <150000000>;
> +      cap-mmc-highspeed;
> +      mmc-ddr-1_8v;
> +      mmc-hs200-1_8v;
> +      mmc-hs400-1_8v;
> +      non-removable;
> +      bus-width = <8>;
> +      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
> +      status = "okay";

No, really...

> +    };
> 

Best regards,
Krzysztof
Yang Xiwen Feb. 16, 2024, 8:29 a.m. UTC | #2 
On 2/16/2024 4:21 PM, Krzysztof Kozlowski wrote:
> On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
>>
>> Add binding for Hi3798MV200 DWMMC specific extension.
>>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>>   ...hi3798cv200-dw-mshc.yaml => histb-dw-mshc.yaml} | 60 +++++++++++++++++++---
>>   1 file changed, 52 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> similarity index 57%
>> rename from Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
>> rename to Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> index 5db99cd94b90..d2f5b7bb7a58 100644
>> --- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> @@ -1,11 +1,11 @@
>>   # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>   %YAML 1.2
>>   ---
>> -$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
>> +$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#
> Really, one wrong filename into another...
How about "hisilicon,dw-mshc.yaml"? I found rockchip using a similar 
naming: "rockchip-dw-mshc.yaml"
>
>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>   
>>   title:
>> -  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
>> +  Hisilicon HiSTB SoCs specific extensions to the Synopsys DWMMC controller
>>   
>>   maintainers:
>>     - Yang Xiwen <forbidden405@outlook.com>
>> @@ -14,16 +14,14 @@ description:
>>     The Synopsys designware mobile storage host controller is used to interface
>>     a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
>>     differences between the core Synopsys dw mshc controller properties described
>> -  by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
>> -  specific extensions to the Synopsys Designware Mobile Storage Host Controller.
> Just drop this sentence in previous/conversion patch. It's useless.
Will do in v2.
>
>> -
>> -allOf:
>> -  - $ref: synopsys-dw-mshc-common.yaml#
> Put it in correct place in the first time. Don't needlessly shuffle the
> code right after previous patch.
Will fix in v2.
>
>
>> +  by synopsys-dw-mshc.txt and the properties used by the Hisilicon HiSTB specific
>> +  extensions to the Synopsys Designware Mobile Storage Host Controller.
>>   
>>   properties:
>>     compatible:
>>       enum:
>>         - hisilicon,hi3798cv200-dw-mshc
>> +      - hisilicon,hi3798mv200-dw-mshc
>>   
>>     reg:
>>       maxItems: 1
>> @@ -48,6 +46,12 @@ properties:
>>         control the clock phases, "ciu-sample" is required for tuning
>>         high speed modes.
>>   
>> +  hisilicon,sap-dll-reg:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      A phandle points to the sample delay-locked-loop(DLL)
>> +      syscon node, used for tuning.
> Does hi3798cv200 have it?
No it does not. Currently only hi3798mv200 has it (it's called himci 
v300 in downstream, while cv200 is using himci v200).
>
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -55,13 +59,25 @@ required:
>>     - clocks
>>     - clock-names
>>   
>> +allOf:
>> +  - $ref: synopsys-dw-mshc-common.yaml#
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: hisilicon,hi3798mv200-dw-mshc
>> +    then:
>> +      required:
>> +        - hisilicon,sap-dll-reg
>> +
>>   unevaluatedProperties: false
>>   
>>   examples:
>>     - |
>>       #include <dt-bindings/clock/histb-clock.h>
>>       #include <dt-bindings/interrupt-controller/arm-gic.h>
>> -    emmc: mmc@9830000 {
>> +    mmc@9830000 {
> ???
It's complaining about duplicated label when i added emmc label to both 
nodes. I'll remove it in previous patch in v2.
>>         compatible = "hisilicon,hi3798cv200-dw-mshc";
>>         reg = <0x9830000 0x10000>;
>>         interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>> @@ -84,3 +100,31 @@ examples:
>>         bus-width = <8>;
>>         status = "okay";
>>       };
>> +  - |
>> +    #include <dt-bindings/clock/histb-clock.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    mmc@9830000 {
>> +      compatible = "hisilicon,hi3798mv200-dw-mshc";
> No need for new example.
>
>> +      reg = <0x9830000 0x10000>;
>> +      interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>> +      clocks = <&crg HISTB_MMC_CIU_CLK>,
>> +               <&crg HISTB_MMC_BIU_CLK>,
>> +               <&crg HISTB_MMC_SAMPLE_CLK>,
>> +               <&crg HISTB_MMC_DRV_CLK>;
>> +      clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
>> +      resets = <&crg 0xa0 4>;
>> +      reset-names = "reset";
>> +      pinctrl-names = "default";
>> +      pinctrl-0 = <&emmc_pins>;
>> +      fifo-depth = <256>;
>> +      clock-frequency = <50000000>;
>> +      max-frequency = <150000000>;
>> +      cap-mmc-highspeed;
>> +      mmc-ddr-1_8v;
>> +      mmc-hs200-1_8v;
>> +      mmc-hs400-1_8v;
>> +      non-removable;
>> +      bus-width = <8>;
>> +      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
>> +      status = "okay";
> No, really...
The property "hisilicon,sap-dll-reg" is introduced in this patch, i want 
to add an example for it here since the common dtsi will use this 
binding and will be submitted when it gets ready.
>
>> +    };
>>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 16, 2024, 8:41 a.m. UTC | #3 
On 16/02/2024 09:29, Yang Xiwen wrote:
>>>     reg:
>>>       maxItems: 1
>>> @@ -48,6 +46,12 @@ properties:
>>>         control the clock phases, "ciu-sample" is required for tuning
>>>         high speed modes.
>>>   
>>> +  hisilicon,sap-dll-reg:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description:
>>> +      A phandle points to the sample delay-locked-loop(DLL)
>>> +      syscon node, used for tuning.
>> Does hi3798cv200 have it?
> No it does not. Currently only hi3798mv200 has it (it's called himci 
> v300 in downstream, while cv200 is using himci v200).

then in your if:
else:
  properties:
    hisilicon,sap-dll-reg: false

>>
>>> +
>>>   required:
>>>     - compatible
>>>     - reg
>>> @@ -55,13 +59,25 @@ required:
>>>     - clocks
>>>     - clock-names
>>>   
>>> +allOf:
>>> +  - $ref: synopsys-dw-mshc-common.yaml#
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: hisilicon,hi3798mv200-dw-mshc
>>> +    then:
>>> +      required:
>>> +        - hisilicon,sap-dll-reg
>>> +
>>>   unevaluatedProperties: false
>>>   
>>>   examples:
>>>     - |
>>>       #include <dt-bindings/clock/histb-clock.h>
>>>       #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> -    emmc: mmc@9830000 {
>>> +    mmc@9830000 {
>> ???
> It's complaining about duplicated label when i added emmc label to both 
> nodes. I'll remove it in previous patch in v2.
>>>         compatible = "hisilicon,hi3798cv200-dw-mshc";
>>>         reg = <0x9830000 0x10000>;
>>>         interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>>> @@ -84,3 +100,31 @@ examples:
>>>         bus-width = <8>;
>>>         status = "okay";
>>>       };
>>> +  - |
>>> +    #include <dt-bindings/clock/histb-clock.h>
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    mmc@9830000 {
>>> +      compatible = "hisilicon,hi3798mv200-dw-mshc";
>> No need for new example.
>>
>>> +      reg = <0x9830000 0x10000>;
>>> +      interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>>> +      clocks = <&crg HISTB_MMC_CIU_CLK>,
>>> +               <&crg HISTB_MMC_BIU_CLK>,
>>> +               <&crg HISTB_MMC_SAMPLE_CLK>,
>>> +               <&crg HISTB_MMC_DRV_CLK>;
>>> +      clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
>>> +      resets = <&crg 0xa0 4>;
>>> +      reset-names = "reset";
>>> +      pinctrl-names = "default";
>>> +      pinctrl-0 = <&emmc_pins>;
>>> +      fifo-depth = <256>;
>>> +      clock-frequency = <50000000>;
>>> +      max-frequency = <150000000>;
>>> +      cap-mmc-highspeed;
>>> +      mmc-ddr-1_8v;
>>> +      mmc-hs200-1_8v;
>>> +      mmc-hs400-1_8v;
>>> +      non-removable;
>>> +      bus-width = <8>;
>>> +      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
>>> +      status = "okay";
>> No, really...
> The property "hisilicon,sap-dll-reg" is introduced in this patch, i want 
> to add an example for it here since the common dtsi will use this 
> binding and will be submitted when it gets ready.

One new property does not justify new example.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
similarity index 57%
rename from Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
rename to Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
index 5db99cd94b90..d2f5b7bb7a58 100644
--- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
+++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
@@ -1,11 +1,11 @@ 
 # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
+$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title:
-  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
+  Hisilicon HiSTB SoCs specific extensions to the Synopsys DWMMC controller
 
 maintainers:
   - Yang Xiwen <forbidden405@outlook.com>
@@ -14,16 +14,14 @@  description:
   The Synopsys designware mobile storage host controller is used to interface
   a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
   differences between the core Synopsys dw mshc controller properties described
-  by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
-  specific extensions to the Synopsys Designware Mobile Storage Host Controller.
-
-allOf:
-  - $ref: synopsys-dw-mshc-common.yaml#
+  by synopsys-dw-mshc.txt and the properties used by the Hisilicon HiSTB specific
+  extensions to the Synopsys Designware Mobile Storage Host Controller.
 
 properties:
   compatible:
     enum:
       - hisilicon,hi3798cv200-dw-mshc
+      - hisilicon,hi3798mv200-dw-mshc
 
   reg:
     maxItems: 1
@@ -48,6 +46,12 @@  properties:
       control the clock phases, "ciu-sample" is required for tuning
       high speed modes.
 
+  hisilicon,sap-dll-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      A phandle points to the sample delay-locked-loop(DLL)
+      syscon node, used for tuning.
+
 required:
   - compatible
   - reg
@@ -55,13 +59,25 @@  required:
   - clocks
   - clock-names
 
+allOf:
+  - $ref: synopsys-dw-mshc-common.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: hisilicon,hi3798mv200-dw-mshc
+    then:
+      required:
+        - hisilicon,sap-dll-reg
+
 unevaluatedProperties: false
 
 examples:
   - |
     #include <dt-bindings/clock/histb-clock.h>
     #include <dt-bindings/interrupt-controller/arm-gic.h>
-    emmc: mmc@9830000 {
+    mmc@9830000 {
       compatible = "hisilicon,hi3798cv200-dw-mshc";
       reg = <0x9830000 0x10000>;
       interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
@@ -84,3 +100,31 @@  examples:
       bus-width = <8>;
       status = "okay";
     };
+  - |
+    #include <dt-bindings/clock/histb-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    mmc@9830000 {
+      compatible = "hisilicon,hi3798mv200-dw-mshc";
+      reg = <0x9830000 0x10000>;
+      interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&crg HISTB_MMC_CIU_CLK>,
+               <&crg HISTB_MMC_BIU_CLK>,
+               <&crg HISTB_MMC_SAMPLE_CLK>,
+               <&crg HISTB_MMC_DRV_CLK>;
+      clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
+      resets = <&crg 0xa0 4>;
+      reset-names = "reset";
+      pinctrl-names = "default";
+      pinctrl-0 = <&emmc_pins>;
+      fifo-depth = <256>;
+      clock-frequency = <50000000>;
+      max-frequency = <150000000>;
+      cap-mmc-highspeed;
+      mmc-ddr-1_8v;
+      mmc-hs200-1_8v;
+      mmc-hs400-1_8v;
+      non-removable;
+      bus-width = <8>;
+      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
+      status = "okay";
+    };