2022-03-19 20:19:33

by Tony Huang

[permalink] [raw]
Subject: [PATCH v4 1/2] dt-binding: mmc: Add mmc yaml file for Sunplus SP7021

Add mmc yaml file for Sunplus SP7021

Signed-off-by: Tony Huang <[email protected]>
---
Changes in v4:
- Addressed comments from Ulf Hansson.

.../devicetree/bindings/mmc/sunplus,mmc.yaml | 79 ++++++++++++++++++++++
MAINTAINERS | 6 ++
2 files changed, 85 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml

diff --git a/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml b/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml
new file mode 100644
index 0000000..13ed07c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Ltd. Co. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/sunplus,mmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: sunplus MMC controller
+
+allOf:
+ - $ref: "mmc-controller.yaml"
+
+maintainers:
+ - Tony Huang <[email protected]>
+ - Li-hao Kuo <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - sunplus,mmc-v1
+ - sunplus,mmc-v2
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+
+ resets:
+ maxItems: 1
+
+ max-frequency: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - resets
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ mmc0: mmc@9c003b00 {
+ compatible = "sunplus,mmc-v2";
+ reg = <0x9c003b00 0x180>;
+ interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clkc 0x4e>;
+ resets = <&rstc 0x3e>;
+ bus-width = <8>;
+ max-frequency = <52000000>;
+ non-removable;
+ disable-wp;
+ cap-mmc-highspeed;
+ mmc-ddr-3_3v;
+ no-sdio;
+ no-sd;
+ };
+
+ mmc1: mmc@9c003e80 {
+ compatible = "sunplus,mmc-v1";
+ reg = <0x9c003e80 0x280>;
+ interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clkc 0x4f>;
+ resets = <&rstc 0x3f>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc1_mux &mmc1_mux_cd>;
+ max-frequency = <52000000>;
+ disable-wp;
+ cap-sd-highspeed;
+ no-sdio;
+ no-mmc;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index fb18ce7..2d91431 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18242,6 +18242,12 @@ L: [email protected]
S: Maintained
F: drivers/net/ethernet/dlink/sundance.c

+SUNPLUS MMC DRIVER
+M: Tony Huang <[email protected]>
+M: Li-hao Kuo <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/mmc/sunplu,mmc.yaml
+
SUPERH
M: Yoshinori Sato <[email protected]>
M: Rich Felker <[email protected]>
--
2.7.4


2022-03-21 18:24:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-binding: mmc: Add mmc yaml file for Sunplus SP7021

On 19/03/2022 02:50, Tony Huang wrote:
> Add mmc yaml file for Sunplus SP7021
>
> Signed-off-by: Tony Huang <[email protected]>
> ---
> Changes in v4:
> - Addressed comments from Ulf Hansson.
>
> .../devicetree/bindings/mmc/sunplus,mmc.yaml | 79 ++++++++++++++++++++++
> MAINTAINERS | 6 ++
> 2 files changed, 85 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml
>
> diff --git a/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml b/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml
> new file mode 100644
> index 0000000..13ed07c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) Sunplus Ltd. Co. 2021
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/sunplus,mmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: sunplus MMC controller

s/sunplus/Sunplus/

> +
> +allOf:
> + - $ref: "mmc-controller.yaml"

allOf below maintainers,

> +
> +maintainers:
> + - Tony Huang <[email protected]>
> + - Li-hao Kuo <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - sunplus,mmc-v1
> + - sunplus,mmc-v2

How did this happen? Versions of blocks are usually discouraged, unless
you have clear specification,. The previous discussion did not end with
"add v1 and v2".

Do you have clear documentation/specification of these blocks? If no,
please use SoC compatibles.

> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + minItems: 1

maxItems

> +
> + resets:
> + maxItems: 1
> +
> + max-frequency: true
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - resets
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + mmc0: mmc@9c003b00 {
> + compatible = "sunplus,mmc-v2";
> + reg = <0x9c003b00 0x180>;
> + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clkc 0x4e>;
> + resets = <&rstc 0x3e>;
> + bus-width = <8>;
> + max-frequency = <52000000>;
> + non-removable;
> + disable-wp;
> + cap-mmc-highspeed;
> + mmc-ddr-3_3v;
> + no-sdio;
> + no-sd;
> + };
> +
> + mmc1: mmc@9c003e80 {
> + compatible = "sunplus,mmc-v1";
> + reg = <0x9c003e80 0x280>;
> + interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clkc 0x4f>;
> + resets = <&rstc 0x3f>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc1_mux &mmc1_mux_cd>;
> + max-frequency = <52000000>;
> + disable-wp;
> + cap-sd-highspeed;
> + no-sdio;
> + no-mmc;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fb18ce7..2d91431 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18242,6 +18242,12 @@ L: [email protected]
> S: Maintained
> F: drivers/net/ethernet/dlink/sundance.c
>
> +SUNPLUS MMC DRIVER
> +M: Tony Huang <[email protected]>
> +M: Li-hao Kuo <[email protected]>

Wrong indentation.

> +S: Maintained
> +F: Documentation/devicetree/bindings/mmc/sunplu,mmc.yaml

Wrong file name.

> +
> SUPERH
> M: Yoshinori Sato <[email protected]>
> M: Rich Felker <[email protected]>


Best regards,
Krzysztof

2022-03-21 21:54:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-binding: mmc: Add mmc yaml file for Sunplus SP7021

On 21/03/2022 03:35, Tony Huang 黃懷厚 wrote:
> Dear Krzysztof:
>
>>> +++ b/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml
>>> @@ -0,0 +1,79 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
>>> +(C) Sunplus Ltd. Co. 2021 %YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mmc/sunplus,mmc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: sunplus MMC controller
>>
>> s/sunplus/Sunplus/
>>
>
> Below is my modification:
> title: Sunplus MMC Controller
>
>>> +
>>> +allOf:
>>> + - $ref: "mmc-controller.yaml"
>>
>> allOf below maintainers,
>>
>
> I will modify it.
>
>>> +
>>> +maintainers:
>>> + - Tony Huang <[email protected]>
>>> + - Li-hao Kuo <[email protected]>
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - sunplus,mmc-v1
>>> + - sunplus,mmc-v2
>>
>> How did this happen? Versions of blocks are usually discouraged, unless you
>> have clear specification,. The previous discussion did not end with "add v1 and
>> v2".
>>
>> Do you have clear documentation/specification of these blocks? If no, please
>> use SoC compatibles.
>>
>
> v1: support SDIO 2.0
> v2: support eMMC 4.41

That's spec is not enough. You need to provide detailed specification of
each of them. See for example versioned blocks of Synopsys DeisgnWare
(e.g. eMMC/SDHCI).


> The register base address and offset address of emmc and sd card are different.
> eMMC and sdcard are individual hardware settings
> We need version to distinguish different speeds

That's not the answer. How did it happen that you have now versions and
before there were SoC compatibles? No one pointed to this and you did
not mention it in description of changes.

Best regards,
Krzysztof

2022-03-21 22:04:19

by Tony Huang 黃懷厚

[permalink] [raw]
Subject: RE: [PATCH v4 1/2] dt-binding: mmc: Add mmc yaml file for Sunplus SP7021

Dear Krzysztof:

> > +++ b/Documentation/devicetree/bindings/mmc/sunplus,mmc.yaml
> > @@ -0,0 +1,79 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
> > +(C) Sunplus Ltd. Co. 2021 %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/sunplus,mmc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: sunplus MMC controller
>
> s/sunplus/Sunplus/
>

Below is my modification:
title: Sunplus MMC Controller

> > +
> > +allOf:
> > + - $ref: "mmc-controller.yaml"
>
> allOf below maintainers,
>

I will modify it.

> > +
> > +maintainers:
> > + - Tony Huang <[email protected]>
> > + - Li-hao Kuo <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - sunplus,mmc-v1
> > + - sunplus,mmc-v2
>
> How did this happen? Versions of blocks are usually discouraged, unless you
> have clear specification,. The previous discussion did not end with "add v1 and
> v2".
>
> Do you have clear documentation/specification of these blocks? If no, please
> use SoC compatibles.
>

v1: support SDIO 2.0
v2: support eMMC 4.41
The register base address and offset address of emmc and sd card are different.
eMMC and sdcard are individual hardware settings
We need version to distinguish different speeds

> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 1
>
> maxItems
>

I will modify it.

> > +
> > + resets:
> > + maxItems: 1
> > +
> > + max-frequency: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - resets
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + mmc0: mmc@9c003b00 {
> > + compatible = "sunplus,mmc-v2";
> > + reg = <0x9c003b00 0x180>;
> > + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clkc 0x4e>;
> > + resets = <&rstc 0x3e>;
> > + bus-width = <8>;
> > + max-frequency = <52000000>;
> > + non-removable;
> > + disable-wp;
> > + cap-mmc-highspeed;
> > + mmc-ddr-3_3v;
> > + no-sdio;
> > + no-sd;
> > + };
> > +
> > + mmc1: mmc@9c003e80 {
> > + compatible = "sunplus,mmc-v1";
> > + reg = <0x9c003e80 0x280>;
> > + interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clkc 0x4f>;
> > + resets = <&rstc 0x3f>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mmc1_mux &mmc1_mux_cd>;
> > + max-frequency = <52000000>;
> > + disable-wp;
> > + cap-sd-highspeed;
> > + no-sdio;
> > + no-mmc;
> > + };
> > diff --git a/MAINTAINERS b/MAINTAINERS index fb18ce7..2d91431 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -18242,6 +18242,12 @@ L: [email protected]
> > S: Maintained
> > F: drivers/net/ethernet/dlink/sundance.c
> >
> > +SUNPLUS MMC DRIVER
> > +M: Tony Huang <[email protected]>
> > +M: Li-hao Kuo <[email protected]>
>
> Wrong indentation.

I will modify it.

>
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/mmc/sunplu,mmc.yaml
>
> Wrong file name.
>

I will modify it.