diff mbox series

[V6,4/4] dt-bindings: mmc: Add dt-bindings for realtek mmc driver

Message ID 20231109082043.27147-5-jyanchou@realtek.com
State New
Headers show
Series Add DesignWare Mobile mmc driver | expand

Commit Message

Jyan Chou [周芷安] Nov. 9, 2023, 8:20 a.m. UTC
Document the device-tree bindings for Realtek SoCs mmc driver.

Signed-off-by: Jyan Chou <jyanchou@realtek.com>

---
v5 -> v6:
- Drop the incorrect, generic compatible and modify it to specific.
- Drop useless properties.
- Modify bindings to make DTS and driver match.

v4 -> v5:
- Remove unused property, e.g.,cqe, resets, clock-freq-min-max.
- Fix indentation.

v3 -> v4:
- Describe the items to make properties and item easy to understand.
- Fix examples' indentation and compiling error.
- Drop useless properties.

v2 -> v3:
- Modify dt-bindings' content and description.
- Fix coding style.
- Update the list of maintainers.

v1 -> v2:
- Add dt-bindings.
---
 .../bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml | 162 ++++++++++++++++++
 1 file changed, 162 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml

Comments

Krzysztof Kozlowski Nov. 9, 2023, 4:22 p.m. UTC | #1
On 09/11/2023 09:20, Jyan Chou wrote:
> Document the device-tree bindings for Realtek SoCs mmc driver.

Subject and commit msg - drop driver and describe hardware instead.
s/mmc/MMC/

> 
> Signed-off-by: Jyan Chou <jyanchou@realtek.com>
> 
> ---
> v5 -> v6:
> - Drop the incorrect, generic compatible and modify it to specific.
> - Drop useless properties.

Which ones?

> - Modify bindings to make DTS and driver match.
> 
> v4 -> v5:
> - Remove unused property, e.g.,cqe, resets, clock-freq-min-max.
> - Fix indentation.
> 
> v3 -> v4:
> - Describe the items to make properties and item easy to understand.
> - Fix examples' indentation and compiling error.
> - Drop useless properties.
> 
> v2 -> v3:
> - Modify dt-bindings' content and description.
> - Fix coding style.
> - Update the list of maintainers.
> 
> v1 -> v2:
> - Add dt-bindings.
> ---
>  .../bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml | 162 ++++++++++++++++++
>  1 file changed, 162 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml
> new file mode 100644
> index 000000000000..d7118cf457e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml
> @@ -0,0 +1,162 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/realtek,rtd-dw-cqe-emmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek DesignWare mobile storage host controller
> +
> +description:
> +  Realtek uses the Synopsys DesignWare mobile storage host controller
> +  to interface a SoC with storage medium. This file documents the Realtek
> +  specific extensions.
> +
> +maintainers:
> +  - Jyan Chou <jyanchou@realtek.com>
> +
> +allOf:
> +  - $ref: synopsys-dw-mshc-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - realtek,rtd1325-dw-cqe-emmc
> +      - realtek,rtd1319-dw-cqe-emmc
> +      - realtek,rtd1315e-dw-cqe-emmc
> +      - realtek,rtd1619b-dw-cqe-emmc
> +
> +  reg:
> +    items:
> +      - description: emmc base address
> +      - description: cqhci base address
> +
> +  reg-names:
> +    items:
> +      - const: emmc
> +      - const: cqhci
> +
> +  realtek,m2tmx:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to m2tmx syscon register region.

There is no such stuff as "syscon register region". Syscon is Linux
term. Please write here instead the full name of the hardware block and
describe its purpose (for what is it needed for).

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 4
> +
> +  clock-names:
> +    items:
> +      - const: biu
> +      - const: ciu
> +      - const: vp0
> +      - const: vp1
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: reset

Nothing improved here.

Go back to previous versions and implement the feedback you received.

Best regards,
Krzysztof
Jyan Chou [周芷安] Nov. 20, 2023, 10:50 a.m. UTC | #2
>> Document the device-tree bindings for Realtek SoCs mmc driver.

> Subject and commit msg - drop driver and describe hardware instead.
> s/mmc/MMC/

>>
>> Signed-off-by: Jyan Chou <jyanchou@realtek.com>
>>
>> ---
>> v5 -> v6:
>> - Drop the incorrect, generic compatible and modify it to specific.
>> - Drop useless properties.

> Which ones?

I will describe it clearly in our new version, the useless properties we dropped were

like some were not be used in our driver and some you asked us to dropped.

>> - Modify bindings to make DTS and driver match.
>>
>> v4 -> v5:
>> - Remove unused property, e.g.,cqe, resets, clock-freq-min-max.
>> - Fix indentation.
>>
>> v3 -> v4:
>> - Describe the items to make properties and item easy to understand.
>> - Fix examples' indentation and compiling error.
>> - Drop useless properties.
>>
>> v2 -> v3:
>> - Modify dt-bindings' content and description.
>> - Fix coding style.
>> - Update the list of maintainers.
>>
>> v1 -> v2:
>> - Add dt-bindings.
>> ---
>>  .../bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml | 162 
>> ++++++++++++++++++
>>  1 file changed, 162 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml 
>> b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml
>> new file mode 100644
>> index 000000000000..d7118cf457e8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.ya
>> +++ ml
>> @@ -0,0 +1,162 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mmc/realtek,rtd-dw-cqe-emmc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Realtek DesignWare mobile storage host controller
>> +
>> +description:
>> +  Realtek uses the Synopsys DesignWare mobile storage host controller
>> +  to interface a SoC with storage medium. This file documents the 
>> +Realtek
>> +  specific extensions.
>> +
>> +maintainers:
>> +  - Jyan Chou <jyanchou@realtek.com>
>> +
>> +allOf:
>> +  - $ref: synopsys-dw-mshc-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - realtek,rtd1325-dw-cqe-emmc
>> +      - realtek,rtd1319-dw-cqe-emmc
>> +      - realtek,rtd1315e-dw-cqe-emmc
>> +      - realtek,rtd1619b-dw-cqe-emmc
>> +
>> +  reg:
>> +    items:
>> +      - description: emmc base address
>> +      - description: cqhci base address
>> +
>> +  reg-names:
>> +    items:
>> +      - const: emmc
>> +      - const: cqhci
>> +
>> +  realtek,m2tmx:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description: Phandle to m2tmx syscon register region.

> There is no such stuff as "syscon register region". Syscon is Linux term. Please write here instead the full name of the hardware block and describe its purpose (for what is it needed for).

Okay, we will correct it in our new version, thanks.

>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 4
>> +
>> +  clock-names:
>> +    items:
>> +      - const: biu
>> +      - const: ciu
>> +      - const: vp0
>> +      - const: vp1
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  reset-names:
>> +    const: reset

> Nothing improved here.

> Go back to previous versions and implement the feedback you received.

Sorry for asking, but I would like to check if I exactly know your meaning first and 

then push our new patch. According to previous versions, I found out that 

>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  reset-names:
>> +    const: reset

> And here you describe the item? The only place when it is not needed?
> Drop entierly.

does it means we need to drop it? But we need this to determine whether to

setup a reset control or not, if we didn't add it in our bindings, check patch will

show error. Did I misunderstand your meaning? Thanks.

Best regards,
Jyan
Krzysztof Kozlowski Nov. 20, 2023, 12:41 p.m. UTC | #3
On 20/11/2023 11:50, Jyan Chou [周芷安] wrote:
>>> +
>>> +  resets:
>>> +    maxItems: 1
>>> +
>>> +  reset-names:
>>> +    const: reset
> 
>> And here you describe the item? The only place when it is not needed?
>> Drop entierly.
> 
> does it means we need to drop it? But we need this to determine whether to
> 
> setup a reset control or not, if we didn't add it in our bindings, check patch will
> 
> show error. Did I misunderstand your meaning? Thanks.

Please drop entirely reset-names. From the bindings, so obviously from
the DTS and driver code as well. I do not ask to drop resets, just
reset-names.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml
new file mode 100644
index 000000000000..d7118cf457e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/realtek,rtd-dw-cqe-emmc.yaml
@@ -0,0 +1,162 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/realtek,rtd-dw-cqe-emmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek DesignWare mobile storage host controller
+
+description:
+  Realtek uses the Synopsys DesignWare mobile storage host controller
+  to interface a SoC with storage medium. This file documents the Realtek
+  specific extensions.
+
+maintainers:
+  - Jyan Chou <jyanchou@realtek.com>
+
+allOf:
+  - $ref: synopsys-dw-mshc-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - realtek,rtd1325-dw-cqe-emmc
+      - realtek,rtd1319-dw-cqe-emmc
+      - realtek,rtd1315e-dw-cqe-emmc
+      - realtek,rtd1619b-dw-cqe-emmc
+
+  reg:
+    items:
+      - description: emmc base address
+      - description: cqhci base address
+
+  reg-names:
+    items:
+      - const: emmc
+      - const: cqhci
+
+  realtek,m2tmx:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Phandle to m2tmx syscon register region.
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: biu
+      - const: ciu
+      - const: vp0
+      - const: vp1
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: reset
+
+  pinctrl-0:
+    description:
+      should contain default/high speed pin ctrl.
+    maxItems: 1
+
+  pinctrl-1:
+    description:
+      should contain sdr50 mode pin ctrl.
+    maxItems: 1
+
+  pinctrl-2:
+    description:
+      should contain ddr50 mode pin ctrl.
+    maxItems: 1
+
+  pinctrl-3:
+    description:
+      should contain hs200 speed pin ctrl.
+    maxItems: 1
+
+  pinctrl-4:
+    description:
+      should contain hs400 speed pin ctrl.
+    maxItems: 1
+
+  pinctrl-5:
+    description:
+      should contain tune0 pin ctrl.
+    maxItems: 1
+
+  pinctrl-6:
+    description:
+      should contain tune1 pin ctrl.
+    maxItems: 1
+
+  pinctrl-7:
+    description:
+      should contain tune2 pin ctrl.
+    maxItems: 1
+
+  pinctrl-8:
+    description:
+      should contain tune3 pin ctrl.
+    maxItems: 1
+
+  pinctrl-9:
+    description:
+      should contain tune4 pin ctrl.
+    maxItems: 1
+
+  pinctrl-names:
+    maxItems: 10
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - clocks
+  - clock-names
+  - vmmc-supply
+  - pinctrl-names
+  - pinctrl-0
+  - pinctrl-1
+  - pinctrl-3
+  - pinctrl-4
+  - pinctrl-5
+  - pinctrl-6
+  - pinctrl-7
+  - pinctrl-8
+  - pinctrl-9
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    emmc: mmc@12000 {
+      compatible = "realtek,rtd1315e-dw-cqe-emmc";
+      reg = <0x00012000 0x00600>,
+            <0x00012180 0x00060>;
+      reg-names = "emmc", "cqhci";
+      realtek,m2tmx = <&m2tmx>;
+      interrupts = <0 42 4>;
+      clocks = <&cc 22>, <&cc 26>, <&cc 121>, <&cc 122>;
+      clock-names = "biu", "ciu", "vp0", "vp1";
+      resets = <&rst 20>;
+      reset-names = "reset";
+      vmmc-supply = <&reg_vcc1v8>;
+      pinctrl-names = "default", "sdr50", "ddr50", "hs200", "hs400",
+                      "tune0","tune1", "tune2","tune3", "tune4";
+      pinctrl-0 = <&emmc_pins_sdr50>;
+      pinctrl-1 = <&emmc_pins_sdr50>;
+      pinctrl-2 = <&emmc_pins_ddr50>;
+      pinctrl-3 = <&emmc_pins_hs200>;
+      pinctrl-4 = <&emmc_pins_hs400>;
+      pinctrl-5 = <&emmc_pins_tune0>;
+      pinctrl-6 = <&emmc_pins_tune1>;
+      pinctrl-7 = <&emmc_pins_tune2>;
+      pinctrl-8 = <&emmc_pins_tune3>;
+      pinctrl-9 = <&emmc_pins_tune4>;
+      supports-cqe;
+    };