2021-11-09 15:49:17

by Li-hao Kuo

[permalink] [raw]
Subject: [PATCH v2 0/2] Add SD/SDIO control driver for Sunplus SP7021 SoC

This is a patch series for SD/SDIO driver for Sunplus SP7021 SoC.

Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates
many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and
etc.) into a single chip. It is designed for industrial control.

Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html

LH.Kuo (2):
mmc: Add SD/SDIO driver for Sunplus SP7021
devicetree bindings mmc Add bindings doc for Sunplus SP7021

.../devicetree/bindings/mmc/sunplus-sd2.yaml | 82 ++
MAINTAINERS | 7 +
drivers/mmc/host/Kconfig | 10 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sunplus_sd2.c | 1068 ++++++++++++++++++++
drivers/mmc/host/sunplus_sd2.h | 155 +++
6 files changed, 1323 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
create mode 100644 drivers/mmc/host/sunplus_sd2.c
create mode 100644 drivers/mmc/host/sunplus_sd2.h

--
2.7.4


2021-11-09 07:58:37

by Li-hao Kuo

[permalink] [raw]
Subject: [PATCH v2 2/2] devicetree bindings mmc Add bindings doc for Sunplus SP7021

Add devicetree bindings mmc Add bindings doc for Sunplus SP7021

Signed-off-by: LH.Kuo <[email protected]>
---
Changes in v2:
- Addressed all comments from Mr. Philipp Zabel
- Modified the structure and register access method.
- Modifiedthe path about MAINTAINERS. ( wrong messages PATH in v1).

.../devicetree/bindings/mmc/sunplus-sd2.yaml | 82 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 83 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml

diff --git a/Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml b/Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
new file mode 100644
index 0000000..95dc0bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/sunplus-sd2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus SD/SDIO controller
+
+maintainers:
+ - lh.kuo <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - sunplus,sp7021-card1
+ - sunplus,sp7021-sdio
+
+ reg:
+ items:
+ - description: Base address and length of the SD/SDIO registers
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ pinctrl-names:
+ description:
+ A pinctrl state named "default" must be defined.
+ const: default
+
+ pinctrl-0:
+ description:
+ A phandle to the default pinctrl state.
+
+ max-frequency: true
+
+allOf:
+ - $ref: "mmc-controller.yaml"
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - resets
+ - pinctrl-names
+ - pinctrl-0
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/sp-sp7021.h>
+ #include <dt-bindings/reset/sp-sp7021.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ mmc1: mmc@9C003e80 {
+ compatible = "sunplus,sp7021-card1";
+ reg = <0x9c003e80 0x280>;
+ interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clkc CARD_CTL1>;
+ resets = <&rstc RST_CARD_CTL1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc1_mux &mmc1_mux_cd>;
+ max-frequency = <52000000>;
+ };
+ sdio: mmc@9C008400 {
+ compatible = "sunplus,sp7021-sdio";
+ reg = <0x9c008400 0x280>;
+ interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clkc CARD_CTL1>;
+ resets = <&rstc RST_CARD_CTL1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pins_sdio>;
+ max-frequency = <52000000>;
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 2746084..e653a1d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18193,6 +18193,7 @@ SUNPLUS SD/SDIO HOST CONTROLLER INTERFACE DRIVER
M: LH Kuo <[email protected]>
L: [email protected]
S: Maintained
+F: Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
F: drivers/mmc/host/sunplus_sd2.*

SUPERH
--
2.7.4


2021-11-29 01:39:40

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] devicetree bindings mmc Add bindings doc for Sunplus SP7021

On Tue, Nov 09, 2021 at 03:58:25PM +0800, LH.Kuo wrote:
> Add devicetree bindings mmc Add bindings doc for Sunplus SP7021
>
> Signed-off-by: LH.Kuo <[email protected]>
> ---
> Changes in v2:
> - Addressed all comments from Mr. Philipp Zabel
> - Modified the structure and register access method.
> - Modifiedthe path about MAINTAINERS. ( wrong messages PATH in v1).
>
> .../devicetree/bindings/mmc/sunplus-sd2.yaml | 82 ++++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 83 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
>
> diff --git a/Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml b/Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
> new file mode 100644
> index 0000000..95dc0bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) Sunplus Co., Ltd. 2021
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/sunplus-sd2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sunplus SD/SDIO controller
> +
> +maintainers:
> + - lh.kuo <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - sunplus,sp7021-card1
> + - sunplus,sp7021-sdio

What's the difference between these 2 blocks?

> +
> + reg:
> + items:
> + - description: Base address and length of the SD/SDIO registers

Just 'maxItems: 1' is sufficient.

> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + pinctrl-names:
> + description:
> + A pinctrl state named "default" must be defined.
> + const: default
> +
> + pinctrl-0:
> + description:
> + A phandle to the default pinctrl state.
> +
> + max-frequency: true
> +
> +allOf:
> + - $ref: "mmc-controller.yaml"
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - resets
> + - pinctrl-names
> + - pinctrl-0
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/sp-sp7021.h>
> + #include <dt-bindings/reset/sp-sp7021.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + mmc1: mmc@9C003e80 {

Use lower case hex.

> + compatible = "sunplus,sp7021-card1";
> + reg = <0x9c003e80 0x280>;
> + interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clkc CARD_CTL1>;
> + resets = <&rstc RST_CARD_CTL1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc1_mux &mmc1_mux_cd>;
> + max-frequency = <52000000>;
> + };
> + sdio: mmc@9C008400 {

Use lower case hex.

> + compatible = "sunplus,sp7021-sdio";
> + reg = <0x9c008400 0x280>;
> + interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clkc CARD_CTL1>;
> + resets = <&rstc RST_CARD_CTL1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pins_sdio>;
> + max-frequency = <52000000>;
> + };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2746084..e653a1d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18193,6 +18193,7 @@ SUNPLUS SD/SDIO HOST CONTROLLER INTERFACE DRIVER
> M: LH Kuo <[email protected]>
> L: [email protected]
> S: Maintained
> +F: Documentation/devicetree/bindings/mmc/sunplus-sd2.yaml
> F: drivers/mmc/host/sunplus_sd2.*
>
> SUPERH
> --
> 2.7.4
>
>

2021-11-29 19:57:58

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] devicetree bindings mmc Add bindings doc for Sunplus SP7021

On Sun, Nov 28, 2021 at 11:30 PM Lh Kuo 郭力豪 <[email protected]> wrote:
>
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - sunplus,sp7021-card1
> > > + - sunplus,sp7021-sdio
> >
> > What's the difference between these 2 blocks?
> >
>
> One for SD card One for SDIO

If the programming model is the same, then it should be the same
compatible string. We have various properties to handle differences
like bus width, card detect or not, etc.

> > > + reg:
> > > + items:
> > > + - description: Base address and length of the SD/SDIO registers
> >
> > Just 'maxItems: 1' is sufficient.
> >
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > +
> > > + resets:
> > > + maxItems: 1
> > > +
> > > + pinctrl-names:
> > > + description:
> > > + A pinctrl state named "default" must be defined.
> > > + const: default
> > > +
> > > + pinctrl-0:
> > > + description:
> > > + A phandle to the default pinctrl state.
> > > +
> > > + max-frequency: true
> > > +
> > > +allOf:
> > > + - $ref: "mmc-controller.yaml"
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - interrupts
> > > + - clocks
> > > + - resets
> > > + - pinctrl-names
> > > + - pinctrl-0
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/clock/sp-sp7021.h>
> > > + #include <dt-bindings/reset/sp-sp7021.h>
> > > + #include <dt-bindings/interrupt-controller/irq.h>
> > > + mmc1: mmc@9C003e80 {
> >
> > Use lower case hex.
>
> Do you mean as follows? ?
>
> mmc1: mmc@3e80 {

No, 'mmc@9c003e80 {'

You also don't need 'mmc1'.

> > > + compatible = "sunplus,sp7021-card1";
> > > + reg = <0x9c003e80 0x280>;
> > > + interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&clkc CARD_CTL1>;
> > > + resets = <&rstc RST_CARD_CTL1>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&mmc1_mux &mmc1_mux_cd>;
> > > + max-frequency = <52000000>;
> > > + };
> > > + sdio: mmc@9C008400 {
> >
>
> Do you mean as follows? ?
>
> mmc1: mmc@8400 {
>
>
> > Use lower case hex.
> >
> > > + compatible = "sunplus,sp7021-sdio";
>

2022-01-04 20:31:48

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] devicetree bindings mmc Add bindings doc for Sunplus SP7021

On Tue, Nov 30, 2021 at 7:59 PM Lh Kuo 郭力豪 <[email protected]> wrote:
>
> > > > > +properties:
> > > > > + compatible:
> > > > > + enum:
> > > > > + - sunplus,sp7021-card1
> > > > > + - sunplus,sp7021-sdio
> > > >
> > > > What's the difference between these 2 blocks?
> > > >
> > >
> > > One for SD card One for SDIO
> >
> > If the programming model is the same, then it should be the same compatible string. We have various
> > properties to handle differences like bus width, card detect or not, etc.
> >
>
> SDIO and SDCARD still need to set the date and CMD decoding differences.

I still don't understand. A host controller should be able to
initialize a card enough to tell what kind it is. And we have things
defined in DT like 'no-sd' and 'no-mmc'.

Looking at the driver, the difference appears to be just setting a
register to the mode (eMMC/SD/SDIO). That's not a difference in the
h/w block which is when different compatibles would be appropriate. A
property, if anything, is the right thing to do here.

Rob