[v2,2/2] devicetree bindings SPI Add bindings doc for Sunplus SP7021

Commit Message

Li-hao Kuo Nov. 9, 2021, 9:01 a.m. UTC

Add devicetree bindings SPI Add bindings doc for Sunplus SP7021

Signed-off-by: LH.Kuo <[e-mail]>
---
 - Addressed all comments from Mr. Mark Brown
 - Addressed all comments from Mr. Philipp Zabel
 - Modified the structure and register access method.

 .../bindings/spi/spi-sunplus-sp7021.yaml           | 95 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml

Comments

Mark Brown Nov. 9, 2021, 2:57 p.m. UTC | #1

On Tue, Nov 09, 2021 at 05:01:28PM +0800, LH.Kuo wrote:
> Add devicetree bindings SPI Add bindings doc for Sunplus SP7021

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> +  reg:
> +    items:
> +      - description: Base address and length of the SPI master registers
> +      - description: Base address and length of the SPI slave registers
> +
> +  reg-names:
> +    items:
> +      - const: spi_master
> +      - const: spi_slave

What exactly is the physical overlap between the two controllers - is it
just the pinmux?

Mark Brown Nov. 10, 2021, 2:11 p.m. UTC | #2

On Wed, Nov 10, 2021 at 02:47:09AM +0000, Lh Kuo 郭力豪 wrote:

> > > +  reg-names:
> > > +    items:
> > > +      - const: spi_master
> > > +      - const: spi_slave

> > What exactly is the physical overlap between the two controllers - is it just the
> > pinmux?

> According to the designer not only pinmux, The power source and more design are overlap.

> That is why I hope set it in one driver.

The power source just sounds like a power domain which doesn't need a
shared driver, and shared elements in the design aren't really relevant
if they're not visible in the usage of the device which given the very
limited code sharing there seems to be in the driver doesn't seem to be
the case here.

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml b/Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml
new file mode 100644
index 0000000..5502f15
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml
@@ -0,0 +1,95 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/spi-sunplus-sp7021.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus sp7021 SPI controller
+
+allOf:
+  - $ref: "spi-controller.yaml"
+
+maintainers:
+  - lh.kuo <[e-mail]>
+
+properties:
+  compatible:
+    enum:
+      - sunplus,sp7021-spi-controller
+
+  reg:
+    items:
+      - description: Base address and length of the SPI master registers
+      - description: Base address and length of the SPI slave registers
+
+  reg-names:
+    items:
+      - const: spi_master
+      - const: spi_slave
+
+  interrupt-names:
+    items:
+      - const: dma_w_intr
+      - const: mas_risc_intr
+      - const: slave_risc_intr
+
+  interrupts:
+    minItems: 3
+
+  clocks:
+    maxItems: 1
+
+  clocks-names:
+    items:
+      - const: sys_pll
+
+  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.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clocks-names
+  - resets
+  - pinctrl-names
+  - pinctrl-0
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/sp-sp7021.h>
+    #include <dt-bindings/reset/sp-sp7021.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi@9C002D80 {
+        compatible = "sunplus,sp7021-spi-controller";
+        reg = <0x9C002D80 0x80>, <0x9C002E00 0x80>;
+        reg-names = "spi_master", "spi_slave";
+        interrupt-parent = <&intc>;
+        interrupt-names = "dma_w_intr",
+                          "mas_risc_intr",
+                          "slave_risc_intr";
+        interrupts = <144 IRQ_TYPE_LEVEL_HIGH>,
+                     <146 IRQ_TYPE_LEVEL_HIGH>,
+                     <145 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clkc SPI_COMBO_0>;
+        clocks-names = "sys_pll";
+        resets = <&rstc RST_SPI_COMBO_0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pins_spi0>;
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 34868d0..ef416a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18193,6 +18193,7 @@  SUNPLUS SPI CONTROLLER INTERFACE DRIVER
 M:	LH Kuo <[e-mail]>
 L:	[e-mail]
 S:	Maintained
+F:	Documentation/devicetree/bindings/spi/spi-sunplus-sp7021.yaml
 F:	drivers/spi/spi-sunplus-sp7021.c
 
 SUPERH