devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v16 02/16] dt-bindings: add Canaan boards compatible strings
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 03/16] dt-bindings: update risc-v cpu properties Damien Le Moal
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Introduce the file riscv/canaan.yaml to document compatible strings
related to the Canaan Kendryte K210 SoC. The compatible string
"canaan,kendryte-k210" used to indicate the use of this SoC to the
early SoC init code is added. This new file also defines the compatible
strings of all supported boards based on this SoC.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/riscv/canaan.yaml     | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/riscv/canaan.yaml

diff --git a/Documentation/devicetree/bindings/riscv/canaan.yaml b/Documentation/devicetree/bindings/riscv/canaan.yaml
new file mode 100644
index 000000000000..f8f3f286bd55
--- /dev/null
+++ b/Documentation/devicetree/bindings/riscv/canaan.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/canaan.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Canaan SoC-based boards
+
+maintainers:
+  - Damien Le Moal <damien.lemoal@wdc.com>
+
+description:
+  Canaan Kendryte K210 SoC-based boards
+
+properties:
+  $nodename:
+    const: '/'
+  compatible:
+    oneOf:
+      - items:
+          - const: sipeed,maix-bit
+          - const: sipeed,maix-bitm
+          - const: canaan,kendryte-k210
+
+      - items:
+          - const: sipeed,maix-go
+          - const: canaan,kendryte-k210
+
+      - items:
+          - const: sipeed,maix-dock-m1
+          - const: sipeed,maix-dock-m1w
+          - const: canaan,kendryte-k210
+
+      - items:
+          - const: sipeed,maixduino
+          - const: canaan,kendryte-k210
+
+      - items:
+          - const: canaan,kendryte-kd233
+          - const: canaan,kendryte-k210
+
+      - items:
+          - const: canaan,kendryte-k210
+
+additionalProperties: true
+
+...
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 03/16] dt-bindings: update risc-v cpu properties
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
  2021-02-05  6:58 ` [PATCH v16 02/16] dt-bindings: add Canaan boards compatible strings Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05 19:47   ` Rob Herring
  2021-02-05  6:58 ` [PATCH v16 04/16] dt-bindings: update sifive plic compatible string Damien Le Moal
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Paul Walmsley,
	Rob Herring, devicetree

The Canaan Kendryte K210 SoC CPU cores are based on a rocket chip
version using a draft verion of the RISC-V ISA specifications. To avoid
any confusion with CPU cores using stable specifications, add the
compatible string "canaan,k210" for this SoC CPU cores.

Also add the "riscv,none" value to the mmu-type property to allow a DT
to indicate that the CPU being described does not have an MMU or that
it has an MMU that is not usable (which is the case for the K210 SoC).

Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index eb6843f69f7c..e534f6a7cfa1 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -39,6 +39,7 @@ properties:
               - sifive,u74
               - sifive,u5
               - sifive,u7
+              - canaan,k210
           - const: riscv
       - const: riscv    # Simulator only
     description:
@@ -56,6 +57,7 @@ properties:
       - riscv,sv32
       - riscv,sv39
       - riscv,sv48
+      - riscv,none
 
   riscv,isa:
     description:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 04/16] dt-bindings: update sifive plic compatible string
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
  2021-02-05  6:58 ` [PATCH v16 02/16] dt-bindings: add Canaan boards compatible strings Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 03/16] dt-bindings: update risc-v cpu properties Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05 19:47   ` Rob Herring
  2021-02-05  6:58 ` [PATCH v16 05/16] dt-bindings: update sifive clint " Damien Le Moal
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Paul Walmsley,
	Rob Herring, devicetree

Add the compatible string "canaan,k210-plic" to the Sifive plic bindings
to indicate the use of the "sifive,plic-1.0.0" IP block in the Canaan
Kendryte K210 SoC. The description is also updated to reflect this
change, that is, that SoCs from other vendors may also use this plic
implementation.

Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 .../interrupt-controller/sifive,plic-1.0.0.yaml     | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
index b9a61c9f7530..08d5a57ce00f 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
@@ -8,10 +8,11 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: SiFive Platform-Level Interrupt Controller (PLIC)
 
 description:
-  SiFive SOCs include an implementation of the Platform-Level Interrupt Controller
-  (PLIC) high-level specification in the RISC-V Privileged Architecture
-  specification. The PLIC connects all external interrupts in the system to all
-  hart contexts in the system, via the external interrupt source in each hart.
+  SiFive SoCs and other RISC-V SoCs include an implementation of the
+  Platform-Level Interrupt Controller (PLIC) high-level specification in
+  the RISC-V Privileged Architecture specification. The PLIC connects all
+  external interrupts in the system to all hart contexts in the system, via
+  the external interrupt source in each hart.
 
   A hart context is a privilege mode in a hardware execution thread. For example,
   in an 4 core system with 2-way SMT, you have 8 harts and probably at least two
@@ -42,7 +43,9 @@ maintainers:
 properties:
   compatible:
     items:
-      - const: sifive,fu540-c000-plic
+      - enum:
+          - sifive,fu540-c000-plic
+          - canaan,k210-plic
       - const: sifive,plic-1.0.0
 
   reg:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 05/16] dt-bindings: update sifive clint compatible string
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (2 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 04/16] dt-bindings: update sifive plic compatible string Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05 19:48   ` Rob Herring
  2021-02-05  6:58 ` [PATCH v16 06/16] dt-bindings: update sifive uart " Damien Le Moal
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Add the "canaan,k210-clint" compatible string to the Sifive clint
bindings to indicate the use of the "sifive,clint0" IP block in the
Canaan Kendryte K210 SoC. The description of the compatible string
property is also updated to reflect this addition.

Cc: Anup Patel <anup.patel@wdc.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 .../devicetree/bindings/timer/sifive,clint.yaml      | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
index 2a0e9cd9fbcf..a35952f48742 100644
--- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml
+++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
@@ -23,15 +23,19 @@ description:
 properties:
   compatible:
     items:
-      - const: sifive,fu540-c000-clint
+      - enum:
+          - sifive,fu540-c000-clint
+          - canaan,k210-clint
       - const: sifive,clint0
 
     description:
-      Should be "sifive,<chip>-clint" and "sifive,clint<version>".
+      Should be "<vendor>,<chip>-clint" and "sifive,clint<version>".
       Supported compatible strings are -
       "sifive,fu540-c000-clint" for the SiFive CLINT v0 as integrated
-      onto the SiFive FU540 chip, and "sifive,clint0" for the SiFive
-      CLINT v0 IP block with no chip integration tweaks.
+      onto the SiFive FU540 chip, "canaan,k210-clint" for the SiFive
+      CLINT v0 as integrated onto the Canaan Kendryte K210 chip, and
+      "sifive,clint0" for the SiFive CLINT v0 IP block with no chip
+      integration tweaks.
       Please refer to sifive-blocks-ip-versioning.txt for details
 
   reg:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 06/16] dt-bindings: update sifive uart compatible string
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (3 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 05/16] dt-bindings: update sifive clint " Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 07/16] dt-bindings: fix sifive gpio properties Damien Le Moal
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Paul Walmsley,
	Rob Herring, devicetree

Add the compatible string "canaan,k210-uarths" to the sifive uart
bindings to indicate the use of this IP block in the Canaan Kendryte
K210 SoC.

Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/serial/sifive-serial.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/sifive-serial.yaml b/Documentation/devicetree/bindings/serial/sifive-serial.yaml
index 3ac5c7ff2758..5fa94dacbba9 100644
--- a/Documentation/devicetree/bindings/serial/sifive-serial.yaml
+++ b/Documentation/devicetree/bindings/serial/sifive-serial.yaml
@@ -20,6 +20,7 @@ properties:
       - enum:
           - sifive,fu540-c000-uart
           - sifive,fu740-c000-uart
+          - canaan,k210-uarths
       - const: sifive,uart0
 
     description:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 07/16] dt-bindings: fix sifive gpio properties
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (4 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 06/16] dt-bindings: update sifive uart " Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 08/16] dt-bindings: add resets property to dw-apb-timer Damien Le Moal
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Paul Walmsley,
	Rob Herring, devicetree

The sifive gpio IP block supports up to 32 GPIOs. Reflect that in the
interrupts property description and maxItems. Also add the standard
ngpios property to describe the number of GPIOs available on the
implementation.

Also add the "canaan,k210-gpiohs" compatible string to indicate the use
of this gpio controller in the Canaan Kendryte K210 SoC. If this
compatible string is used, do not define the clocks property as
required as the K210 SoC does not have a software controllable clock
for the Sifive gpio IP block.

Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 .../devicetree/bindings/gpio/sifive,gpio.yaml | 22 ++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml b/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml
index ab22056f8b44..211869d30212 100644
--- a/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/sifive,gpio.yaml
@@ -16,6 +16,7 @@ properties:
       - enum:
           - sifive,fu540-c000-gpio
           - sifive,fu740-c000-gpio
+          - canaan,k210-gpiohs
       - const: sifive,gpio0
 
   reg:
@@ -23,9 +24,9 @@ properties:
 
   interrupts:
     description:
-      interrupt mapping one per GPIO. Maximum 16 GPIOs.
+      interrupt mapping one per GPIO. Maximum 32 GPIOs.
     minItems: 1
-    maxItems: 16
+    maxItems: 32
 
   interrupt-controller: true
 
@@ -38,6 +39,11 @@ properties:
   "#gpio-cells":
     const: 2
 
+  ngpios:
+    description:
+      The number of GPIO pins available for the controller implementation.
+      It is 16 for the SiFive SoCs and 32 for the Canaan K210 SoC.
+
   gpio-controller: true
 
 required:
@@ -46,10 +52,20 @@ required:
   - interrupts
   - interrupt-controller
   - "#interrupt-cells"
-  - clocks
   - "#gpio-cells"
   - gpio-controller
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - sifive,fu540-c000-gpio
+          - sifive,fu740-c000-gpio
+then:
+  required:
+    - clocks
+
 additionalProperties: false
 
 examples:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 08/16] dt-bindings: add resets property to dw-apb-timer
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (5 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 07/16] dt-bindings: fix sifive gpio properties Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree Damien Le Moal
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Daniel Lezcano,
	Rob Herring, devicetree

The Synopsis DesignWare APB timer driver
(drivers/clocksource/dw_apb_timer_of.c) indirectly uses the resets
property of its node as it executes the function of_reset_control_get().
Make sure that this property is documented in
timer/snps,dw-apb-timer.yaml to avoid make dtbs_check warnings.

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
index d65faf289a83..d33c9205a909 100644
--- a/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
+++ b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
@@ -24,6 +24,9 @@ properties:
   interrupts:
     maxItems: 1
 
+  resets:
+    maxItems: 1
+
   clocks:
     minItems: 1
     items:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (6 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 08/16] dt-bindings: add resets property to dw-apb-timer Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05 20:25   ` Rob Herring
  2021-02-05  6:58 ` [PATCH v16 10/16] riscv: Add SiPeed MAIX BiT board " Damien Le Moal
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Update the Canaan Kendryte K210 base device tree k210.dtsi to define
all peripherals of the SoC, their clocks and reset lines. The device
tree file k210.dts is renamed to k210_generic.dts and becomes the
default value selection of the SOC_CANAAN_K210_DTB_BUILTIN_SOURCE
configuration option. No device beside the serial console is defined by
this device tree. This makes this generic device tree suitable for use
with a builtin initramfs with all known K210 based boards.

These changes result in the K210_CLK_ACLK clock ID to be unused and
removed from the dt-bindings k210-clk.h header file.

Most updates to the k210.dtsi file come from Sean Anderson's work on
U-Boot support for the K210.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 arch/riscv/Kconfig.socs                     |   2 +-
 arch/riscv/boot/dts/canaan/k210.dts         |  23 -
 arch/riscv/boot/dts/canaan/k210.dtsi        | 535 +++++++++++++++++++-
 arch/riscv/boot/dts/canaan/k210_generic.dts |  46 ++
 include/dt-bindings/clock/k210-clk.h        |   1 -
 5 files changed, 554 insertions(+), 53 deletions(-)
 delete mode 100644 arch/riscv/boot/dts/canaan/k210.dts
 create mode 100644 arch/riscv/boot/dts/canaan/k210_generic.dts

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 6402746c68f3..7efcece8896c 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -51,7 +51,7 @@ config SOC_CANAAN_K210_DTB_SOURCE
 	string "Source file for the Canaan Kendryte K210 builtin DTB"
 	depends on SOC_CANAAN
 	depends on SOC_CANAAN_K210_DTB_BUILTIN
-	default "k210"
+	default "k210_generic"
 	help
 	  Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
 	  for the DTS file that will be used to produce the DTB linked into the
diff --git a/arch/riscv/boot/dts/canaan/k210.dts b/arch/riscv/boot/dts/canaan/k210.dts
deleted file mode 100644
index 0d1f28fce6b2..000000000000
--- a/arch/riscv/boot/dts/canaan/k210.dts
+++ /dev/null
@@ -1,23 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright (C) 2020 Western Digital Corporation or its affiliates.
- */
-
-/dts-v1/;
-
-#include "k210.dtsi"
-
-/ {
-	model = "Kendryte K210 generic";
-	compatible = "kendryte,k210";
-
-	chosen {
-		bootargs = "earlycon console=ttySIF0";
-		stdout-path = "serial0";
-	};
-};
-
-&uarths0 {
-	status = "okay";
-};
-
diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
index 354b263195a3..63c1f4c98d6c 100644
--- a/arch/riscv/boot/dts/canaan/k210.dtsi
+++ b/arch/riscv/boot/dts/canaan/k210.dtsi
@@ -1,9 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright (C) 2019 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
  * Copyright (C) 2020 Western Digital Corporation or its affiliates.
  */
 #include <dt-bindings/clock/k210-clk.h>
+#include <dt-bindings/pinctrl/k210-fpioa.h>
+#include <dt-bindings/reset/k210-rst.h>
 
 / {
 	/*
@@ -12,14 +14,33 @@ / {
 	 */
 	#address-cells = <1>;
 	#size-cells = <1>;
-	compatible = "kendryte,k210";
+	compatible = "canaan,kendryte-k210";
 
 	aliases {
+		cpu0 = &cpu0;
+		cpu1 = &cpu1;
+		dma0 = &dmac0;
+		gpio0 = &gpio0;
+		gpio1 = &gpio1_0;
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
+		i2c2 = &i2c2;
+		pinctrl0 = &fpioa;
 		serial0 = &uarths0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+		serial3 = &uart3;
+		spi0 = &spi0;
+		spi1 = &spi1;
+		spi2 = &spi2;
+		spi3 = &spi3;
+		timer0 = &timer0;
+		timer1 = &timer1;
+		timer2 = &timer2;
 	};
 
 	/*
-	 * The K210 has an sv39 MMU following the priviledge specification v1.9.
+	 * The K210 has an sv39 MMU following the privileged specification v1.9.
 	 * Since this is a non-ratified draft specification, the kernel does not
 	 * support it and the K210 support enabled only for the !MMU case.
 	 * Be consistent with this by setting the CPUs MMU type to "none".
@@ -30,14 +51,14 @@ cpus {
 		timebase-frequency = <7800000>;
 		cpu0: cpu@0 {
 			device_type = "cpu";
+			compatible = "canaan,k210", "riscv";
 			reg = <0>;
-			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
 			riscv,isa = "rv64imafdc";
-			mmu-type = "none";
-			i-cache-size = <0x8000>;
+			mmu-type = "riscv,none";
 			i-cache-block-size = <64>;
-			d-cache-size = <0x8000>;
+			i-cache-size = <0x8000>;
 			d-cache-block-size = <64>;
+			d-cache-size = <0x8000>;
 			cpu0_intc: interrupt-controller {
 				#interrupt-cells = <1>;
 				interrupt-controller;
@@ -46,14 +67,14 @@ cpu0_intc: interrupt-controller {
 		};
 		cpu1: cpu@1 {
 			device_type = "cpu";
+			compatible = "canaan,k210", "riscv";
 			reg = <1>;
-			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
 			riscv,isa = "rv64imafdc";
-			mmu-type = "none";
-			i-cache-size = <0x8000>;
+			mmu-type = "riscv,none";
 			i-cache-block-size = <64>;
-			d-cache-size = <0x8000>;
+			i-cache-size = <0x8000>;
 			d-cache-block-size = <64>;
+			d-cache-size = <0x8000>;
 			cpu1_intc: interrupt-controller {
 				#interrupt-cells = <1>;
 				interrupt-controller;
@@ -64,10 +85,15 @@ cpu1_intc: interrupt-controller {
 
 	sram: memory@80000000 {
 		device_type = "memory";
+		compatible = "canaan,k210-sram";
 		reg = <0x80000000 0x400000>,
 		      <0x80400000 0x200000>,
 		      <0x80600000 0x200000>;
 		reg-names = "sram0", "sram1", "aisram";
+		clocks = <&sysclk K210_CLK_SRAM0>,
+			 <&sysclk K210_CLK_SRAM1>,
+			 <&sysclk K210_CLK_AI>;
+		clock-names = "sram0", "sram1", "aisram";
 	};
 
 	clocks {
@@ -81,40 +107,493 @@ in0: oscillator {
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-		compatible = "kendryte,k210-soc", "simple-bus";
+		compatible = "simple-bus";
 		ranges;
 		interrupt-parent = <&plic0>;
 
-		sysctl: sysctl@50440000 {
-			compatible = "kendryte,k210-sysctl", "simple-mfd";
-			reg = <0x50440000 0x1000>;
-			#clock-cells = <1>;
+		debug0: debug@0 {
+			compatible = "canaan,k210-debug", "riscv,debug";
+			reg = <0x0 0x1000>;
+			status = "disabled";
+		};
+
+		rom0: nvmem@1000 {
+			reg = <0x1000 0x1000>;
+			read-only;
+			status = "disabled";
 		};
 
 		clint0: clint@2000000 {
-			#interrupt-cells = <1>;
-			compatible = "riscv,clint0";
+			compatible = "canaan,k210-clint", "sifive,clint0";
 			reg = <0x2000000 0xC000>;
-			interrupts-extended =  <&cpu0_intc 3 &cpu0_intc 7
-						&cpu1_intc 3 &cpu1_intc 7>;
+			interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
+					      &cpu1_intc 3 &cpu1_intc 7>;
 		};
 
-		plic0: interrupt-controller@c000000 {
+		plic0: interrupt-controller@C000000 {
 			#interrupt-cells = <1>;
-			interrupt-controller;
-			compatible = "kendryte,k210-plic0", "riscv,plic0";
+			#address-cells = <0>;
+			compatible = "canaan,k210-plic", "sifive,plic-1.0.0";
 			reg = <0xC000000 0x4000000>;
-			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 0xffffffff>,
-					      <&cpu1_intc 11>, <&cpu1_intc 0xffffffff>;
+			interrupt-controller;
+			interrupts-extended = <&cpu0_intc 11 &cpu1_intc 11>;
 			riscv,ndev = <65>;
-			riscv,max-priority = <7>;
 		};
 
 		uarths0: serial@38000000 {
-			compatible = "kendryte,k210-uarths", "sifive,uart0";
+			compatible = "canaan,k210-uarths", "sifive,uart0";
 			reg = <0x38000000 0x1000>;
 			interrupts = <33>;
-			clocks = <&sysctl K210_CLK_CPU>;
+			clocks = <&sysclk K210_CLK_CPU>;
+			status = "disabled";
+		};
+
+		gpio0: gpio-controller@38001000 {
+			#interrupt-cells = <2>;
+			#gpio-cells = <2>;
+			compatible = "canaan,k210-gpiohs", "sifive,gpio0";
+			reg = <0x38001000 0x1000>;
+			interrupt-controller;
+			interrupts = <34 35 36 37 38 39 40 41
+				      42 43 44 45 46 47 48 49
+				      50 51 52 53 54 55 56 57
+				      58 59 60 61 62 63 64 65>;
+			gpio-controller;
+			ngpios = <32>;
+			status = "disabled";
+		};
+
+		kpu0: kpu@40800000 {
+			compatible = "canaan,k210-kpu";
+			reg = <0x40800000 0xc00000>;
+			interrupts = <25>;
+			clocks = <&sysclk K210_CLK_AI>;
+			status = "disabled";
+		};
+
+		fft0: fft@42000000 {
+			compatible = "canaan,k210-fft";
+			reg = <0x42000000 0x400000>;
+			interrupts = <26>;
+			clocks = <&sysclk K210_CLK_FFT>;
+			resets = <&sysrst K210_RST_FFT>;
+			status = "disabled";
+		};
+
+		dmac0: dma-controller@50000000 {
+			compatible = "snps,axi-dma-1.01a";
+			reg = <0x50000000 0x1000>;
+			interrupts = <27 28 29 30 31 32>;
+			clocks = <&sysclk K210_CLK_DMA>, <&sysclk K210_CLK_DMA>;
+			clock-names = "core-clk", "cfgr-clk";
+			resets = <&sysrst K210_RST_DMA>;
+			dma-channels = <6>;
+			snps,dma-masters = <2>;
+			snps,priority = <0 1 2 3 4 5>;
+			snps,data-width = <5>;
+			snps,block-size = <0x200000 0x200000 0x200000
+					   0x200000 0x200000 0x200000>;
+			snps,axi-max-burst-len = <256>;
+			status = "disabled";
+		};
+
+		apb0: bus@50200000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "simple-pm-bus";
+			ranges;
+			clocks = <&sysclk K210_CLK_APB0>;
+
+			gpio1: gpio@50200000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "snps,dw-apb-gpio";
+				reg = <0x50200000 0x80>;
+				clocks = <&sysclk K210_CLK_APB0>,
+					 <&sysclk K210_CLK_GPIO>;
+				clock-names = "bus", "db";
+				resets = <&sysrst K210_RST_GPIO>;
+				status = "disabled";
+
+				gpio1_0: gpio-port@0 {
+					#gpio-cells = <2>;
+					#interrupt-cells = <2>;
+					compatible = "snps,dw-apb-gpio-port";
+					reg = <0>;
+					interrupt-controller;
+					interrupts = <23>;
+					gpio-controller;
+					ngpios = <8>;
+				};
+			};
+
+			uart1: serial@50210000 {
+				compatible = "snps,dw-apb-uart";
+				reg = <0x50210000 0x100>;
+				interrupts = <11>;
+				clocks = <&sysclk K210_CLK_UART1>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "baudclk", "apb_pclk";
+				resets = <&sysrst K210_RST_UART1>;
+				reg-io-width = <4>;
+				reg-shift = <2>;
+				dcd-override;
+				dsr-override;
+				cts-override;
+				ri-override;
+				status = "disabled";
+			};
+
+			uart2: serial@50220000 {
+				compatible = "snps,dw-apb-uart";
+				reg = <0x50220000 0x100>;
+				interrupts = <12>;
+				clocks = <&sysclk K210_CLK_UART2>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "baudclk", "apb_pclk";
+				resets = <&sysrst K210_RST_UART2>;
+				reg-io-width = <4>;
+				reg-shift = <2>;
+				dcd-override;
+				dsr-override;
+				cts-override;
+				ri-override;
+				status = "disabled";
+			};
+
+			uart3: serial@50230000 {
+				compatible = "snps,dw-apb-uart";
+				reg = <0x50230000 0x100>;
+				interrupts = <13>;
+				clocks = <&sysclk K210_CLK_UART3>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "baudclk", "apb_pclk";
+				resets = <&sysrst K210_RST_UART3>;
+				reg-io-width = <4>;
+				reg-shift = <2>;
+				dcd-override;
+				dsr-override;
+				cts-override;
+				ri-override;
+				status = "disabled";
+			};
+
+			spi2: spi@50240000 {
+				compatible = "canaan,k210-spi";
+				spi-slave;
+				reg = <0x50240000 0x100>;
+				interrupts = <3>;
+				clocks = <&sysclk K210_CLK_SPI2>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "ssi_clk", "pclk";
+				resets = <&sysrst K210_RST_SPI2>;
+				spi-max-frequency = <25000000>;
+				status = "disabled";
+			};
+
+			i2s0: i2s@50250000 {
+				compatible = "snps,designware-i2s";
+				reg = <0x50250000 0x200>;
+				interrupts = <5>;
+				clocks = <&sysclk K210_CLK_I2S0>;
+				clock-names = "i2sclk";
+				resets = <&sysrst K210_RST_I2S0>;
+				status = "disabled";
+			};
+
+			apu0: sound@520250200 {
+				compatible = "canaan,k210-apu";
+				reg = <0x50250200 0x200>;
+				status = "disabled";
+			};
+
+			i2s1: i2s@50260000 {
+				compatible = "snps,designware-i2s";
+				reg = <0x50260000 0x200>;
+				interrupts = <6>;
+				clocks = <&sysclk K210_CLK_I2S1>;
+				clock-names = "i2sclk";
+				resets = <&sysrst K210_RST_I2S1>;
+				status = "disabled";
+			};
+
+			i2s2: i2s@50270000 {
+				compatible = "snps,designware-i2s";
+				reg = <0x50270000 0x200>;
+				interrupts = <7>;
+				clocks = <&sysclk K210_CLK_I2S2>;
+				clock-names = "i2sclk";
+				resets = <&sysrst K210_RST_I2S2>;
+				status = "disabled";
+			};
+
+			i2c0: i2c@50280000 {
+				compatible = "snps,designware-i2c";
+				reg = <0x50280000 0x100>;
+				interrupts = <8>;
+				clocks = <&sysclk K210_CLK_I2C0>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "ref", "pclk";
+				resets = <&sysrst K210_RST_I2C0>;
+				status = "disabled";
+			};
+
+			i2c1: i2c@50290000 {
+				compatible = "snps,designware-i2c";
+				reg = <0x50290000 0x100>;
+				interrupts = <9>;
+				clocks = <&sysclk K210_CLK_I2C1>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "ref", "pclk";
+				resets = <&sysrst K210_RST_I2C1>;
+				status = "disabled";
+			};
+
+			i2c2: i2c@502A0000 {
+				compatible = "snps,designware-i2c";
+				reg = <0x502A0000 0x100>;
+				interrupts = <10>;
+				clocks = <&sysclk K210_CLK_I2C2>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "ref", "pclk";
+				resets = <&sysrst K210_RST_I2C2>;
+				status = "disabled";
+			};
+
+			fpioa: pinmux@502B0000 {
+				compatible = "canaan,k210-fpioa";
+				reg = <0x502B0000 0x100>;
+				clocks = <&sysclk K210_CLK_FPIOA>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "ref", "pclk";
+				resets = <&sysrst K210_RST_FPIOA>;
+				canaan,k210-sysctl-power = <&sysctl 108>;
+				status = "disabled";
+			};
+
+			sha256: sha256@502C0000 {
+				compatible = "canaan,k210-sha256";
+				reg = <0x502C0000 0x100>;
+				clocks = <&sysclk K210_CLK_SHA>;
+				resets = <&sysrst K210_RST_SHA>;
+				status = "disabled";
+			};
+
+			timer0: timer@502D0000 {
+				compatible = "snps,dw-apb-timer";
+				reg = <0x502D0000 0x100>;
+				interrupts = <14 15>;
+				clocks = <&sysclk K210_CLK_TIMER0>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "timer", "pclk";
+				resets = <&sysrst K210_RST_TIMER0>;
+				status = "disabled";
+			};
+
+			timer1: timer@502E0000 {
+				compatible = "snps,dw-apb-timer";
+				reg = <0x502E0000 0x100>;
+				interrupts = <16 17>;
+				clocks = <&sysclk K210_CLK_TIMER1>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "timer", "pclk";
+				resets = <&sysrst K210_RST_TIMER1>;
+				status = "disabled";
+			};
+
+			timer2: timer@502F0000 {
+				compatible = "snps,dw-apb-timer";
+				reg = <0x502F0000 0x100>;
+				interrupts = <18 19>;
+				clocks = <&sysclk K210_CLK_TIMER2>,
+					 <&sysclk K210_CLK_APB0>;
+				clock-names = "timer", "pclk";
+				resets = <&sysrst K210_RST_TIMER2>;
+				status = "disabled";
+			};
+		};
+
+		apb1: bus@50400000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "simple-pm-bus";
+			ranges;
+			clocks = <&sysclk K210_CLK_APB1>;
+
+			wdt0: watchdog@50400000 {
+				compatible = "snps,dw-wdt";
+				reg = <0x50400000 0x100>;
+				interrupts = <21>;
+				clocks = <&sysclk K210_CLK_WDT0>,
+					 <&sysclk K210_CLK_APB1>;
+				clock-names = "tclk", "pclk";
+				resets = <&sysrst K210_RST_WDT0>;
+				status = "disabled";
+			};
+
+			wdt1: watchdog@50410000 {
+				compatible = "snps,dw-wdt";
+				reg = <0x50410000 0x100>;
+				interrupts = <22>;
+				clocks = <&sysclk K210_CLK_WDT1>,
+					 <&sysclk K210_CLK_APB1>;
+				clock-names = "tclk", "pclk";
+				resets = <&sysrst K210_RST_WDT1>;
+				status = "disabled";
+			};
+
+			otp0: nvmem@50420000 {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				compatible = "canaan,k210-otp";
+				reg = <0x50420000 0x100>,
+				      <0x88000000 0x20000>;
+				reg-names = "reg", "mem";
+				clocks = <&sysclk K210_CLK_ROM>;
+				resets = <&sysrst K210_RST_ROM>;
+				read-only;
+				status = "disabled";
+
+				/* Bootloader */
+				firmware@00000 {
+					reg = <0x00000 0xC200>;
+				};
+
+				/*
+				 * config string as described in RISC-V
+				 * privileged spec 1.9
+				 */
+				config-1-9@1c000 {
+					reg = <0x1C000 0x1000>;
+				};
+
+				/*
+				 * Device tree containing only registers,
+				 * interrupts, and cpus
+				 */
+				fdt@1d000 {
+					reg = <0x1D000 0x2000>;
+				};
+
+				/* CPU/ROM credits */
+				credits@1f000 {
+					reg = <0x1F000 0x1000>;
+				};
+			};
+
+			dvp0: camera@50430000 {
+				compatible = "canaan,k210-dvp";
+				reg = <0x50430000 0x100>;
+				interrupts = <24>;
+				clocks = <&sysclk K210_CLK_DVP>;
+				resets = <&sysrst K210_RST_DVP>;
+				canaan,k210-misc-offset = <&sysctl 84>;
+				status = "disabled";
+			};
+
+			sysctl: syscon@50440000 {
+				compatible = "canaan,k210-sysctl",
+					     "syscon", "simple-mfd";
+				reg = <0x50440000 0x100>;
+				clocks = <&sysclk K210_CLK_APB1>;
+				clock-names = "pclk";
+
+				sysclk: clock-controller {
+					#clock-cells = <1>;
+					compatible = "canaan,k210-clk";
+					clocks = <&in0>;
+				};
+
+				sysrst: reset-controller {
+					compatible = "canaan,k210-rst";
+					#reset-cells = <1>;
+				};
+
+				reboot: syscon-reboot {
+					compatible = "syscon-reboot";
+					regmap = <&sysctl>;
+					offset = <48>;
+					mask = <1>;
+					value = <1>;
+				};
+			};
+
+			aes0: aes@50450000 {
+				compatible = "canaan,k210-aes";
+				reg = <0x50450000 0x100>;
+				clocks = <&sysclk K210_CLK_AES>;
+				resets = <&sysrst K210_RST_AES>;
+				status = "disabled";
+			};
+
+			rtc: rtc@50460000 {
+				compatible = "canaan,k210-rtc";
+				reg = <0x50460000 0x100>;
+				clocks = <&in0>;
+				resets = <&sysrst K210_RST_RTC>;
+				interrupts = <20>;
+				status = "disabled";
+			};
+		};
+
+		apb2: bus@52000000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "simple-pm-bus";
+			ranges;
+			clocks = <&sysclk K210_CLK_APB2>;
+
+			spi0: spi@52000000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "canaan,k210-spi";
+				reg = <0x52000000 0x100>;
+				interrupts = <1>;
+				clocks = <&sysclk K210_CLK_SPI0>,
+					 <&sysclk K210_CLK_APB2>;
+				clock-names = "ssi_clk", "pclk";
+				resets = <&sysrst K210_RST_SPI0>;
+				reset-names = "spi";
+				spi-max-frequency = <25000000>;
+				num-cs = <4>;
+				reg-io-width = <4>;
+				status = "disabled";
+			};
+
+			spi1: spi@53000000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "canaan,k210-spi";
+				reg = <0x53000000 0x100>;
+				interrupts = <2>;
+				clocks = <&sysclk K210_CLK_SPI1>,
+					 <&sysclk K210_CLK_APB2>;
+				clock-names = "ssi_clk", "pclk";
+				resets = <&sysrst K210_RST_SPI1>;
+				reset-names = "spi";
+				spi-max-frequency = <25000000>;
+				num-cs = <4>;
+				reg-io-width = <4>;
+				status = "disabled";
+			};
+
+			spi3: spi@54000000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "snps,dwc-ssi-1.01a";
+				reg = <0x54000000 0x200>;
+				interrupts = <4>;
+				clocks = <&sysclk K210_CLK_SPI3>,
+					 <&sysclk K210_CLK_APB2>;
+				clock-names = "ssi_clk", "pclk";
+				resets = <&sysrst K210_RST_SPI3>;
+				reset-names = "spi";
+				/* Could possibly go up to 200 MHz */
+				spi-max-frequency = <100000000>;
+				num-cs = <4>;
+				reg-io-width = <4>;
+				status = "disabled";
+			};
 		};
 	};
 };
diff --git a/arch/riscv/boot/dts/canaan/k210_generic.dts b/arch/riscv/boot/dts/canaan/k210_generic.dts
new file mode 100644
index 000000000000..396c8ca4d24d
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/k210_generic.dts
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ */
+
+/dts-v1/;
+
+#include "k210.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "Kendryte K210 generic";
+	compatible = "canaan,kendryte-k210";
+
+	chosen {
+		bootargs = "earlycon console=ttySIF0";
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&fpioa {
+	pinctrl-0 = <&jtag_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	jtag_pins: jtag-pinmux {
+		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
+			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
+			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
+			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
+	};
+
+	uarths_pins: uarths-pinmux {
+		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
+			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
+	};
+};
+
+&uarths0 {
+	pinctrl-0 = <&uarths_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
diff --git a/include/dt-bindings/clock/k210-clk.h b/include/dt-bindings/clock/k210-clk.h
index a48176ad3c23..b2de702cbf75 100644
--- a/include/dt-bindings/clock/k210-clk.h
+++ b/include/dt-bindings/clock/k210-clk.h
@@ -9,7 +9,6 @@
 /*
  * Kendryte K210 SoC clock identifiers (arbitrary values).
  */
-#define K210_CLK_ACLK	0
 #define K210_CLK_CPU	0
 #define K210_CLK_SRAM0	1
 #define K210_CLK_SRAM1	2
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 10/16] riscv: Add SiPeed MAIX BiT board device tree
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (7 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 11/16] riscv: Add SiPeed MAIX DOCK " Damien Le Moal
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Add the device tree sipeed_maix_bit.dts for the SiPeed MAIX BiT and
MAIX BiTm boards. This device tree enables LEDs, gpio, i2c and spi/mmc
SD card devices.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 .../riscv/boot/dts/canaan/sipeed_maix_bit.dts | 234 ++++++++++++++++++
 1 file changed, 234 insertions(+)
 create mode 100644 arch/riscv/boot/dts/canaan/sipeed_maix_bit.dts

diff --git a/arch/riscv/boot/dts/canaan/sipeed_maix_bit.dts b/arch/riscv/boot/dts/canaan/sipeed_maix_bit.dts
new file mode 100644
index 000000000000..11e491410f00
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/sipeed_maix_bit.dts
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ */
+
+/dts-v1/;
+
+#include "k210.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	model = "SiPeed MAIX BiT";
+	compatible = "sipeed,maix-bit", "sipeed,maix-bitm",
+		     "canaan,kendryte-k210";
+
+	chosen {
+		bootargs = "earlycon console=ttySIF0";
+		stdout-path = "serial0:115200n8";
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		led0 {
+			color = <LED_COLOR_ID_GREEN>;
+			label = "green";
+			gpios = <&gpio1_0 4 GPIO_ACTIVE_LOW>;
+		};
+
+		led1 {
+			color = <LED_COLOR_ID_RED>;
+			label = "red";
+			gpios = <&gpio1_0 5 GPIO_ACTIVE_LOW>;
+		};
+
+		led2 {
+			color = <LED_COLOR_ID_BLUE>;
+			label = "blue";
+			gpios = <&gpio1_0 6 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		boot {
+			label = "BOOT";
+			linux,code = <BTN_0>;
+			gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		status = "disabled";
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0 0>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&mic>;
+		};
+	};
+
+	mic: mic {
+		#sound-dai-cells = <0>;
+		compatible = "memsensing,msm261s4030h0";
+		status = "disabled";
+	};
+};
+
+&fpioa {
+	pinctrl-names = "default";
+	pinctrl-0 = <&jtag_pinctrl>;
+	status = "okay";
+
+	jtag_pinctrl: jtag-pinmux {
+		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
+			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
+			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
+			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
+	};
+
+	uarths_pinctrl: uarths-pinmux {
+		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
+			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
+	};
+
+	gpio_pinctrl: gpio-pinmux {
+		pinmux = <K210_FPIOA(8, K210_PCF_GPIO0)>,
+			 <K210_FPIOA(9, K210_PCF_GPIO1)>,
+			 <K210_FPIOA(10, K210_PCF_GPIO2)>,
+			 <K210_FPIOA(11, K210_PCF_GPIO3)>,
+			 <K210_FPIOA(12, K210_PCF_GPIO4)>,
+			 <K210_FPIOA(13, K210_PCF_GPIO5)>,
+			 <K210_FPIOA(14, K210_PCF_GPIO6)>,
+			 <K210_FPIOA(15, K210_PCF_GPIO7)>;
+	};
+
+	gpiohs_pinctrl: gpiohs-pinmux {
+		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,
+			 <K210_FPIOA(17, K210_PCF_GPIOHS1)>,
+			 <K210_FPIOA(21, K210_PCF_GPIOHS5)>,
+			 <K210_FPIOA(22, K210_PCF_GPIOHS6)>,
+			 <K210_FPIOA(23, K210_PCF_GPIOHS7)>,
+			 <K210_FPIOA(24, K210_PCF_GPIOHS8)>,
+			 <K210_FPIOA(25, K210_PCF_GPIOHS9)>,
+			 <K210_FPIOA(32, K210_PCF_GPIOHS16)>,
+			 <K210_FPIOA(33, K210_PCF_GPIOHS17)>,
+			 <K210_FPIOA(34, K210_PCF_GPIOHS18)>,
+			 <K210_FPIOA(35, K210_PCF_GPIOHS19)>;
+	};
+
+	i2s0_pinctrl: i2s0-pinmux {
+		pinmux = <K210_FPIOA(18, K210_PCF_I2S0_SCLK)>,
+			 <K210_FPIOA(19, K210_PCF_I2S0_WS)>,
+			 <K210_FPIOA(20, K210_PCF_I2S0_IN_D0)>;
+	};
+
+	dvp_pinctrl: dvp-pinmux {
+		pinmux = <K210_FPIOA(40, K210_PCF_SCCB_SDA)>,
+			 <K210_FPIOA(41, K210_PCF_SCCB_SCLK)>,
+			 <K210_FPIOA(42, K210_PCF_DVP_RST)>,
+			 <K210_FPIOA(43, K210_PCF_DVP_VSYNC)>,
+			 <K210_FPIOA(44, K210_PCF_DVP_PWDN)>,
+			 <K210_FPIOA(45, K210_PCF_DVP_HSYNC)>,
+			 <K210_FPIOA(46, K210_PCF_DVP_XCLK)>,
+			 <K210_FPIOA(47, K210_PCF_DVP_PCLK)>;
+	};
+
+	spi0_pinctrl: spi0-pinmux {
+		pinmux = <K210_FPIOA(36, K210_PCF_GPIOHS20)>,  /* cs */
+			 <K210_FPIOA(37, K210_PCF_GPIOHS21)>,  /* rst */
+			 <K210_FPIOA(38, K210_PCF_GPIOHS22)>,  /* dc */
+			 <K210_FPIOA(39, K210_PCF_SPI0_SCLK)>; /* wr */
+	};
+
+	spi1_pinctrl: spi1-pinmux {
+		pinmux = <K210_FPIOA(26, K210_PCF_SPI1_D1)>,
+			 <K210_FPIOA(27, K210_PCF_SPI1_SCLK)>,
+			 <K210_FPIOA(28, K210_PCF_SPI1_D0)>,
+			 <K210_FPIOA(29, K210_PCF_GPIOHS13)>; /* cs */
+	};
+
+	i2c1_pinctrl: i2c1-pinmux {
+		pinmux = <K210_FPIOA(30, K210_PCF_I2C1_SCLK)>,
+			 <K210_FPIOA(31, K210_PCF_I2C1_SDA)>;
+	};
+};
+
+&uarths0 {
+	pinctrl-0 = <&uarths_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio0 {
+	pinctrl-0 = <&gpiohs_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio1 {
+	pinctrl-0 = <&gpio_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&i2s0 {
+	#sound-dai-cells = <1>;
+	pinctrl-0 = <&i2s0_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&i2c1 {
+	pinctrl-0 = <&i2c1_pinctrl>;
+	pinctrl-names = "default";
+	clock-frequency = <400000>;
+	status = "okay";
+};
+
+&dvp0 {
+	pinctrl-0 = <&dvp_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&spi0 {
+	pinctrl-0 = <&spi0_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 20 GPIO_ACTIVE_HIGH>;
+
+	panel@0 {
+		compatible = "sitronix,st7789v";
+		reg = <0>;
+		reset-gpios = <&gpio0 21 GPIO_ACTIVE_LOW>;
+		dc-gpios = <&gpio0 22 GPIO_ACTIVE_HIGH>;
+		spi-max-frequency = <15000000>;
+		spi-cs-high;
+		status = "disabled";
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&spi1_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
+	status = "okay";
+
+	slot@0 {
+		compatible = "mmc-spi-slot";
+		reg = <0>;
+		voltage-ranges = <3300 3300>;
+		spi-max-frequency = <25000000>;
+		broken-cd;
+	};
+};
+
+&spi3 {
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+		m25p,fast-read;
+		broken-flash-reset;
+	};
+};
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 11/16] riscv: Add SiPeed MAIX DOCK board device tree
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (8 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 10/16] riscv: Add SiPeed MAIX BiT board " Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 12/16] riscv: Add SiPeed MAIX GO " Damien Le Moal
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Add the device tree sipeed_maix_dock.dts for the SiPeed MAIX DOCK m1
and m1w boards. This device tree enables LEDs, gpio, i2c and spi/mmc
SD card devices.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 .../boot/dts/canaan/sipeed_maix_dock.dts      | 236 ++++++++++++++++++
 1 file changed, 236 insertions(+)
 create mode 100644 arch/riscv/boot/dts/canaan/sipeed_maix_dock.dts

diff --git a/arch/riscv/boot/dts/canaan/sipeed_maix_dock.dts b/arch/riscv/boot/dts/canaan/sipeed_maix_dock.dts
new file mode 100644
index 000000000000..fae0149a8740
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/sipeed_maix_dock.dts
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ */
+
+/dts-v1/;
+
+#include "k210.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	model = "SiPeed MAIX Dock";
+	compatible = "sipeed,maix-dock-m1", "sipeed,maix-dock-m1w",
+		     "canaan,kendryte-k210";
+
+	chosen {
+		bootargs = "earlycon console=ttySIF0";
+		stdout-path = "serial0:115200n8";
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		/*
+		 * Note: the board wiring drawing documents green on
+		 * gpio #4, red on gpio #5 and blue on gpio #6. However,
+		 * the board is actually wired differently as defined here.
+		 */
+		led0 {
+			color = <LED_COLOR_ID_BLUE>;
+			label = "blue";
+			gpios = <&gpio1_0 4 GPIO_ACTIVE_LOW>;
+		};
+
+		led1 {
+			color = <LED_COLOR_ID_GREEN>;
+			label = "green";
+			gpios = <&gpio1_0 5 GPIO_ACTIVE_LOW>;
+		};
+
+		led2 {
+			color = <LED_COLOR_ID_RED>;
+			label = "red";
+			gpios = <&gpio1_0 6 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		boot {
+			label = "BOOT";
+			linux,code = <BTN_0>;
+			gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		status = "disabled";
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0 0>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&mic>;
+		};
+	};
+
+	mic: mic {
+		#sound-dai-cells = <0>;
+		compatible = "memsensing,msm261s4030h0";
+		status = "disabled";
+	};
+};
+
+&fpioa {
+	pinctrl-0 = <&jtag_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	jtag_pinctrl: jtag-pinmux {
+		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
+			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
+			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
+			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
+	};
+
+	uarths_pinctrl: uarths-pinmux {
+		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
+			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
+	};
+
+	gpio_pinctrl: gpio-pinmux {
+		pinmux = <K210_FPIOA(8, K210_PCF_GPIO0)>,
+			 <K210_FPIOA(11, K210_PCF_GPIO3)>,
+			 <K210_FPIOA(12, K210_PCF_GPIO4)>,
+			 <K210_FPIOA(13, K210_PCF_GPIO5)>,
+			 <K210_FPIOA(14, K210_PCF_GPIO6)>,
+			 <K210_FPIOA(15, K210_PCF_GPIO7)>;
+	};
+
+	gpiohs_pinctrl: gpiohs-pinmux {
+		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,
+			 <K210_FPIOA(17, K210_PCF_GPIOHS1)>,
+			 <K210_FPIOA(21, K210_PCF_GPIOHS5)>,
+			 <K210_FPIOA(22, K210_PCF_GPIOHS6)>,
+			 <K210_FPIOA(23, K210_PCF_GPIOHS7)>,
+			 <K210_FPIOA(24, K210_PCF_GPIOHS8)>,
+			 <K210_FPIOA(25, K210_PCF_GPIOHS9)>,
+			 <K210_FPIOA(32, K210_PCF_GPIOHS16)>,
+			 <K210_FPIOA(33, K210_PCF_GPIOHS17)>,
+			 <K210_FPIOA(34, K210_PCF_GPIOHS18)>,
+			 <K210_FPIOA(35, K210_PCF_GPIOHS19)>;
+	};
+
+	i2s0_pinctrl: i2s0-pinmux {
+		pinmux = <K210_FPIOA(18, K210_PCF_I2S0_SCLK)>,
+			 <K210_FPIOA(19, K210_PCF_I2S0_WS)>,
+			 <K210_FPIOA(20, K210_PCF_I2S0_IN_D0)>;
+	};
+
+	dvp_pinctrl: dvp-pinmux {
+		pinmux = <K210_FPIOA(40, K210_PCF_SCCB_SDA)>,
+			 <K210_FPIOA(41, K210_PCF_SCCB_SCLK)>,
+			 <K210_FPIOA(42, K210_PCF_DVP_RST)>,
+			 <K210_FPIOA(43, K210_PCF_DVP_VSYNC)>,
+			 <K210_FPIOA(44, K210_PCF_DVP_PWDN)>,
+			 <K210_FPIOA(45, K210_PCF_DVP_HSYNC)>,
+			 <K210_FPIOA(46, K210_PCF_DVP_XCLK)>,
+			 <K210_FPIOA(47, K210_PCF_DVP_PCLK)>;
+	};
+
+	spi0_pinctrl: spi0-pinmux {
+		pinmux = <K210_FPIOA(36, K210_PCF_GPIOHS20)>,  /* cs */
+			 <K210_FPIOA(37, K210_PCF_GPIOHS21)>,  /* rst */
+			 <K210_FPIOA(38, K210_PCF_GPIOHS22)>,  /* dc */
+			 <K210_FPIOA(39, K210_PCF_SPI0_SCLK)>; /* wr */
+	};
+
+	spi1_pinctrl: spi1-pinmux {
+		pinmux = <K210_FPIOA(26, K210_PCF_SPI1_D1)>,
+			 <K210_FPIOA(27, K210_PCF_SPI1_SCLK)>,
+			 <K210_FPIOA(28, K210_PCF_SPI1_D0)>,
+			 <K210_FPIOA(29, K210_PCF_GPIOHS13)>; /* cs */
+	};
+
+	i2c1_pinctrl: i2c1-pinmux {
+		pinmux = <K210_FPIOA(9, K210_PCF_I2C1_SCLK)>,
+			 <K210_FPIOA(10, K210_PCF_I2C1_SDA)>;
+	};
+};
+
+&uarths0 {
+	pinctrl-0 = <&uarths_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio0 {
+	pinctrl-0 = <&gpiohs_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio1 {
+	pinctrl-0 = <&gpio_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&i2s0 {
+	#sound-dai-cells = <1>;
+	pinctrl-0 = <&i2s0_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&i2c1 {
+	pinctrl-0 = <&i2c1_pinctrl>;
+	pinctrl-names = "default";
+	clock-frequency = <400000>;
+	status = "okay";
+};
+
+&dvp0 {
+	pinctrl-0 = <&dvp_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&spi0 {
+	pinctrl-0 = <&spi0_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 20 GPIO_ACTIVE_HIGH>;
+
+	panel@0 {
+		compatible = "sitronix,st7789v";
+		reg = <0>;
+		reset-gpios = <&gpio0 21 GPIO_ACTIVE_LOW>;
+		dc-gpios = <&gpio0 22 0>;
+		spi-max-frequency = <15000000>;
+		status = "disabled";
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&spi1_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
+	status = "okay";
+
+	slot@0 {
+		compatible = "mmc-spi-slot";
+		reg = <0>;
+		voltage-ranges = <3300 3300>;
+		spi-max-frequency = <25000000>;
+		broken-cd;
+	};
+};
+
+&spi3 {
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+		m25p,fast-read;
+		broken-flash-reset;
+	};
+};
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 12/16] riscv: Add SiPeed MAIX GO board device tree
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (9 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 11/16] riscv: Add SiPeed MAIX DOCK " Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 13/16] riscv: Add SiPeed MAIXDUINO " Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 14/16] riscv: Add Kendryte KD233 " Damien Le Moal
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Add the device tree sipeed_maix_go.dts for the SiPeed MAIX GO board.
This device tree enables buttons, LEDs, gpio, i2c and spi/mmc SD card
devices.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 arch/riscv/boot/dts/canaan/sipeed_maix_go.dts | 244 ++++++++++++++++++
 1 file changed, 244 insertions(+)
 create mode 100644 arch/riscv/boot/dts/canaan/sipeed_maix_go.dts

diff --git a/arch/riscv/boot/dts/canaan/sipeed_maix_go.dts b/arch/riscv/boot/dts/canaan/sipeed_maix_go.dts
new file mode 100644
index 000000000000..373fbaa3ab94
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/sipeed_maix_go.dts
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ */
+
+/dts-v1/;
+
+#include "k210.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	model = "SiPeed MAIX GO";
+	compatible = "sipeed,maix-go", "canaan,kendryte-k210";
+
+	chosen {
+		bootargs = "earlycon console=ttySIF0";
+		stdout-path = "serial0:115200n8";
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		led0 {
+			color = <LED_COLOR_ID_GREEN>;
+			label = "green";
+			gpios = <&gpio1_0 4 GPIO_ACTIVE_LOW>;
+		};
+
+		led1 {
+			color = <LED_COLOR_ID_RED>;
+			label = "red";
+			gpios = <&gpio1_0 5 GPIO_ACTIVE_LOW>;
+		};
+
+		led2 {
+			color = <LED_COLOR_ID_BLUE>;
+			label = "blue";
+			gpios = <&gpio1_0 6 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		up {
+			label = "UP";
+			linux,code = <BTN_1>;
+			gpios = <&gpio1_0 7 GPIO_ACTIVE_LOW>;
+		};
+
+		press {
+			label = "PRESS";
+			linux,code = <BTN_0>;
+			gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+		};
+
+		down {
+			label = "DOWN";
+			linux,code = <BTN_2>;
+			gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		status = "disabled";
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0 0>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&mic>;
+		};
+	};
+
+	mic: mic {
+		#sound-dai-cells = <0>;
+		compatible = "memsensing,msm261s4030h0";
+		status = "disabled";
+	};
+};
+
+&fpioa {
+	pinctrl-0 = <&jtag_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	jtag_pinctrl: jtag-pinmux {
+		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
+			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
+			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
+			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
+	};
+
+	uarths_pinctrl: uarths-pinmux {
+		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
+			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
+	};
+
+	gpio_pinctrl: gpio-pinmux {
+		pinmux = <K210_FPIOA(8, K210_PCF_GPIO0)>,
+			 <K210_FPIOA(9, K210_PCF_GPIO1)>,
+			 <K210_FPIOA(10, K210_PCF_GPIO2)>,
+			 <K210_FPIOA(11, K210_PCF_GPIO3)>,
+			 <K210_FPIOA(12, K210_PCF_GPIO4)>,
+			 <K210_FPIOA(13, K210_PCF_GPIO5)>,
+			 <K210_FPIOA(14, K210_PCF_GPIO6)>,
+			 <K210_FPIOA(15, K210_PCF_GPIO7)>;
+	};
+
+	gpiohs_pinctrl: gpiohs-pinmux {
+		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,
+			 <K210_FPIOA(17, K210_PCF_GPIOHS1)>,
+			 <K210_FPIOA(21, K210_PCF_GPIOHS5)>,
+			 <K210_FPIOA(22, K210_PCF_GPIOHS6)>,
+			 <K210_FPIOA(23, K210_PCF_GPIOHS7)>,
+			 <K210_FPIOA(24, K210_PCF_GPIOHS8)>,
+			 <K210_FPIOA(25, K210_PCF_GPIOHS9)>,
+			 <K210_FPIOA(32, K210_PCF_GPIOHS16)>,
+			 <K210_FPIOA(33, K210_PCF_GPIOHS17)>,
+			 <K210_FPIOA(34, K210_PCF_GPIOHS18)>,
+			 <K210_FPIOA(35, K210_PCF_GPIOHS19)>;
+	};
+
+	i2s0_pinctrl: i2s0-pinmux {
+		pinmux = <K210_FPIOA(18, K210_PCF_I2S0_SCLK)>,
+			 <K210_FPIOA(19, K210_PCF_I2S0_WS)>,
+			 <K210_FPIOA(20, K210_PCF_I2S0_IN_D0)>;
+	};
+
+	dvp_pinctrl: dvp-pinmux {
+		pinmux = <K210_FPIOA(40, K210_PCF_SCCB_SDA)>,
+			 <K210_FPIOA(41, K210_PCF_SCCB_SCLK)>,
+			 <K210_FPIOA(42, K210_PCF_DVP_RST)>,
+			 <K210_FPIOA(43, K210_PCF_DVP_VSYNC)>,
+			 <K210_FPIOA(44, K210_PCF_DVP_PWDN)>,
+			 <K210_FPIOA(45, K210_PCF_DVP_HSYNC)>,
+			 <K210_FPIOA(46, K210_PCF_DVP_XCLK)>,
+			 <K210_FPIOA(47, K210_PCF_DVP_PCLK)>;
+	};
+
+	spi0_pinctrl: spi0-pinmux {
+		pinmux = <K210_FPIOA(36, K210_PCF_GPIOHS20)>,  /* cs */
+			 <K210_FPIOA(37, K210_PCF_GPIOHS21)>,  /* rst */
+			 <K210_FPIOA(38, K210_PCF_GPIOHS22)>,  /* dc */
+			 <K210_FPIOA(39, K210_PCF_SPI0_SCLK)>; /* wr */
+	};
+
+	spi1_pinctrl: spi1-pinmux {
+		pinmux = <K210_FPIOA(26, K210_PCF_SPI1_D1)>,
+			 <K210_FPIOA(27, K210_PCF_SPI1_SCLK)>,
+			 <K210_FPIOA(28, K210_PCF_SPI1_D0)>,
+			 <K210_FPIOA(29, K210_PCF_GPIOHS13)>; /* cs */
+	};
+
+	i2c1_pinctrl: i2c1-pinmux {
+		pinmux = <K210_FPIOA(30, K210_PCF_I2C1_SCLK)>,
+			 <K210_FPIOA(31, K210_PCF_I2C1_SDA)>;
+	};
+};
+
+&uarths0 {
+	pinctrl-0 = <&uarths_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio0 {
+	pinctrl-0 = <&gpiohs_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio1 {
+	pinctrl-0 = <&gpio_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&i2s0 {
+	#sound-dai-cells = <1>;
+	pinctrl-0 = <&i2s0_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&i2c1 {
+	pinctrl-0 = <&i2c1_pinctrl>;
+	pinctrl-names = "default";
+	clock-frequency = <400000>;
+	status = "okay";
+};
+
+&dvp0 {
+	pinctrl-0 = <&dvp_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&spi0 {
+	pinctrl-0 = <&spi0_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 20 GPIO_ACTIVE_HIGH>;
+
+	panel@0 {
+		compatible = "sitronix,st7789v";
+		reg = <0>;
+		reset-gpios = <&gpio0 21 GPIO_ACTIVE_LOW>;
+		dc-gpios = <&gpio0 22 GPIO_ACTIVE_HIGH>;
+		spi-max-frequency = <15000000>;
+		status = "disabled";
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&spi1_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
+	status = "okay";
+
+	slot@0 {
+		compatible = "mmc-spi-slot";
+		reg = <0>;
+		voltage-ranges = <3300 3300>;
+		spi-max-frequency = <25000000>;
+		broken-cd;
+	};
+};
+
+&spi3 {
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+		m25p,fast-read;
+		broken-flash-reset;
+	};
+};
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 13/16] riscv: Add SiPeed MAIXDUINO board device tree
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (10 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 12/16] riscv: Add SiPeed MAIX GO " Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  2021-02-05  6:58 ` [PATCH v16 14/16] riscv: Add Kendryte KD233 " Damien Le Moal
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Add the device tree sipeed_maixduino.dts for the SiPeed MAIXDUINO board.
This device tree enables LEDs and spi/mmc SD card device. Additionally,
gpios and i2c are also enabled and mapped to the board header pins as
indicated on the board itself.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 .../boot/dts/canaan/sipeed_maixduino.dts      | 209 ++++++++++++++++++
 1 file changed, 209 insertions(+)
 create mode 100644 arch/riscv/boot/dts/canaan/sipeed_maixduino.dts

diff --git a/arch/riscv/boot/dts/canaan/sipeed_maixduino.dts b/arch/riscv/boot/dts/canaan/sipeed_maixduino.dts
new file mode 100644
index 000000000000..804edc45eeeb
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/sipeed_maixduino.dts
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ */
+
+/dts-v1/;
+
+#include "k210.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "SiPeed MAIXDUINO";
+	compatible = "sipeed,maixduino", "canaan,kendryte-k210";
+
+	chosen {
+		bootargs = "earlycon console=ttySIF0";
+		stdout-path = "serial0:115200n8";
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		boot {
+			label = "BOOT";
+			linux,code = <BTN_0>;
+			gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		status = "disabled";
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0 0>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&mic>;
+		};
+	};
+
+	mic: mic {
+		#sound-dai-cells = <0>;
+		compatible = "memsensing,msm261s4030h0";
+		status = "disabled";
+	};
+
+	vcc_3v3: regulator-3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+};
+
+&fpioa {
+	status = "okay";
+
+	uarths_pinctrl: uarths-pinmux {
+		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>, /* Header "0" */
+			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>; /* Header "1" */
+	};
+
+	gpio_pinctrl: gpio-pinmux {
+		pinmux = <K210_FPIOA(8, K210_PCF_GPIO0)>,
+			 <K210_FPIOA(9, K210_PCF_GPIO1)>;
+	};
+
+	gpiohs_pinctrl: gpiohs-pinmux {
+		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,  /* BOOT */
+			 <K210_FPIOA(21, K210_PCF_GPIOHS2)>,  /* Header "2" */
+			 <K210_FPIOA(22, K210_PCF_GPIOHS3)>,  /* Header "3" */
+			 <K210_FPIOA(23, K210_PCF_GPIOHS4)>,  /* Header "4" */
+			 <K210_FPIOA(24, K210_PCF_GPIOHS5)>,  /* Header "5" */
+			 <K210_FPIOA(32, K210_PCF_GPIOHS6)>,  /* Header "6" */
+			 <K210_FPIOA(15, K210_PCF_GPIOHS7)>,  /* Header "7" */
+			 <K210_FPIOA(14, K210_PCF_GPIOHS8)>,  /* Header "8" */
+			 <K210_FPIOA(13, K210_PCF_GPIOHS9)>,  /* Header "9" */
+			 <K210_FPIOA(12, K210_PCF_GPIOHS10)>, /* Header "10" */
+			 <K210_FPIOA(11, K210_PCF_GPIOHS11)>, /* Header "11" */
+			 <K210_FPIOA(10, K210_PCF_GPIOHS12)>, /* Header "12" */
+			 <K210_FPIOA(3,  K210_PCF_GPIOHS13)>; /* Header "13" */
+	};
+
+	i2s0_pinctrl: i2s0-pinmux {
+		pinmux = <K210_FPIOA(18, K210_PCF_I2S0_SCLK)>,
+			 <K210_FPIOA(19, K210_PCF_I2S0_WS)>,
+			 <K210_FPIOA(20, K210_PCF_I2S0_IN_D0)>;
+	};
+
+	spi1_pinctrl: spi1-pinmux {
+		pinmux = <K210_FPIOA(26, K210_PCF_SPI1_D1)>,
+			 <K210_FPIOA(27, K210_PCF_SPI1_SCLK)>,
+			 <K210_FPIOA(28, K210_PCF_SPI1_D0)>,
+			 <K210_FPIOA(29, K210_PCF_GPIO2)>; /* cs */
+	};
+
+	i2c1_pinctrl: i2c1-pinmux {
+		pinmux = <K210_FPIOA(30, K210_PCF_I2C1_SCLK)>, /* Header "scl" */
+			 <K210_FPIOA(31, K210_PCF_I2C1_SDA)>;  /* Header "sda" */
+	};
+
+	i2s1_pinctrl: i2s1-pinmux {
+		pinmux = <K210_FPIOA(33, K210_PCF_I2S1_WS)>,
+			 <K210_FPIOA(34, K210_PCF_I2S1_IN_D0)>,
+			 <K210_FPIOA(35, K210_PCF_I2S1_SCLK)>;
+	};
+
+	spi0_pinctrl: spi0-pinmux {
+		pinmux = <K210_FPIOA(36, K210_PCF_GPIOHS20)>,  /* cs */
+			 <K210_FPIOA(37, K210_PCF_GPIOHS21)>,  /* rst */
+			 <K210_FPIOA(38, K210_PCF_GPIOHS22)>,  /* dc */
+			 <K210_FPIOA(39, K210_PCF_SPI0_SCLK)>; /* wr */
+	};
+
+	dvp_pinctrl: dvp-pinmux {
+		pinmux = <K210_FPIOA(40, K210_PCF_SCCB_SDA)>,
+			 <K210_FPIOA(41, K210_PCF_SCCB_SCLK)>,
+			 <K210_FPIOA(42, K210_PCF_DVP_RST)>,
+			 <K210_FPIOA(43, K210_PCF_DVP_VSYNC)>,
+			 <K210_FPIOA(44, K210_PCF_DVP_PWDN)>,
+			 <K210_FPIOA(45, K210_PCF_DVP_HSYNC)>,
+			 <K210_FPIOA(46, K210_PCF_DVP_XCLK)>,
+			 <K210_FPIOA(47, K210_PCF_DVP_PCLK)>;
+	};
+};
+
+&uarths0 {
+	pinctrl-0 = <&uarths_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio0 {
+	pinctrl-0 = <&gpiohs_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio1 {
+	pinctrl-0 = <&gpio_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&i2s0 {
+	#sound-dai-cells = <1>;
+	pinctrl-0 = <&i2s0_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&i2c1 {
+	pinctrl-0 = <&i2c1_pinctrl>;
+	pinctrl-names = "default";
+	clock-frequency = <400000>;
+	status = "okay";
+};
+
+&dvp0 {
+	pinctrl-0 = <&dvp_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&spi0 {
+	pinctrl-0 = <&spi0_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 20 GPIO_ACTIVE_HIGH>;
+
+	panel@0 {
+		compatible = "sitronix,st7789v";
+		reg = <0>;
+		reset-gpios = <&gpio0 21 GPIO_ACTIVE_LOW>;
+		dc-gpios = <&gpio0 22 0>;
+		spi-max-frequency = <15000000>;
+		power-supply = <&vcc_3v3>;
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&spi1_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio1_0 2 GPIO_ACTIVE_LOW>;
+	status = "okay";
+
+	slot@0 {
+		compatible = "mmc-spi-slot";
+		reg = <0>;
+		voltage-ranges = <3300 3300>;
+		spi-max-frequency = <25000000>;
+		broken-cd;
+	};
+};
+
+&spi3 {
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+		m25p,fast-read;
+		broken-flash-reset;
+	};
+};
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v16 14/16] riscv: Add Kendryte KD233 board device tree
       [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
                   ` (11 preceding siblings ...)
  2021-02-05  6:58 ` [PATCH v16 13/16] riscv: Add SiPeed MAIXDUINO " Damien Le Moal
@ 2021-02-05  6:58 ` Damien Le Moal
  12 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05  6:58 UTC (permalink / raw)
  To: Palmer Dabbelt, linux-riscv
  Cc: Atish Patra, Anup Patel, Sean Anderson, Rob Herring, devicetree

Add the device tree canaan_kd233.dts for the Canaan Kendryte KD233
development board.  This device tree enables LEDs, some gpios and
spi/mmc SD card device. The WS2812B RGB LED and the 10 positions rotary
dip switch present on the board are left undefined.

Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/boot/dts/canaan/canaan_kd233.dts | 177 ++++++++++++++++++++
 1 file changed, 177 insertions(+)
 create mode 100644 arch/riscv/boot/dts/canaan/canaan_kd233.dts

diff --git a/arch/riscv/boot/dts/canaan/canaan_kd233.dts b/arch/riscv/boot/dts/canaan/canaan_kd233.dts
new file mode 100644
index 000000000000..d9af66144938
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/canaan_kd233.dts
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ */
+
+/dts-v1/;
+
+#include "k210.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "Kendryte KD233";
+	compatible = "canaan,kendryte-kd233", "canaan,kendryte-k210";
+
+	chosen {
+		bootargs = "earlycon console=ttySIF0";
+		stdout-path = "serial0:115200n8";
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		led0 {
+			gpios = <&gpio0 8 GPIO_ACTIVE_LOW>;
+		};
+
+		led1 {
+			gpios = <&gpio0 9 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		key0 {
+			label = "KEY0";
+			linux,code = <BTN_0>;
+			gpios = <&gpio0 10 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		status = "disabled";
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0 0>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&mic>;
+		};
+	};
+
+	mic: mic {
+		#sound-dai-cells = <0>;
+		compatible = "memsensing,msm261s4030h0";
+		status = "disabled";
+	};
+};
+
+&fpioa {
+	pinctrl-0 = <&jtag_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	jtag_pinctrl: jtag-pinmux {
+		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
+			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
+			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
+			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
+	};
+
+	uarths_pinctrl: uarths-pinmux {
+		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
+			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
+	};
+
+	spi0_pinctrl: spi0-pinmux {
+		pinmux = <K210_FPIOA(6, K210_PCF_GPIOHS20)>,  /* cs */
+			 <K210_FPIOA(7, K210_PCF_SPI0_SCLK)>, /* wr */
+			 <K210_FPIOA(8, K210_PCF_GPIOHS21)>;  /* dc */
+	};
+
+	dvp_pinctrl: dvp-pinmux {
+		pinmux = <K210_FPIOA(9, K210_PCF_SCCB_SCLK)>,
+			 <K210_FPIOA(10, K210_PCF_SCCB_SDA)>,
+			 <K210_FPIOA(11, K210_PCF_DVP_RST)>,
+			 <K210_FPIOA(12, K210_PCF_DVP_VSYNC)>,
+			 <K210_FPIOA(13, K210_PCF_DVP_PWDN)>,
+			 <K210_FPIOA(14, K210_PCF_DVP_XCLK)>,
+			 <K210_FPIOA(15, K210_PCF_DVP_PCLK)>,
+			 <K210_FPIOA(17, K210_PCF_DVP_HSYNC)>;
+	};
+
+	gpiohs_pinctrl: gpiohs-pinmux {
+		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,
+			 <K210_FPIOA(20, K210_PCF_GPIOHS4)>, /* Rot. dip sw line 8 */
+			 <K210_FPIOA(21, K210_PCF_GPIOHS5)>, /* Rot. dip sw line 4 */
+			 <K210_FPIOA(22, K210_PCF_GPIOHS6)>, /* Rot. dip sw line 2 */
+			 <K210_FPIOA(23, K210_PCF_GPIOHS7)>, /* Rot. dip sw line 1 */
+			 <K210_FPIOA(24, K210_PCF_GPIOHS8)>,
+			 <K210_FPIOA(25, K210_PCF_GPIOHS9)>,
+			 <K210_FPIOA(26, K210_PCF_GPIOHS10)>;
+	};
+
+	spi1_pinctrl: spi1-pinmux {
+		pinmux = <K210_FPIOA(29, K210_PCF_SPI1_SCLK)>,
+			 <K210_FPIOA(30, K210_PCF_SPI1_D0)>,
+			 <K210_FPIOA(31, K210_PCF_SPI1_D1)>,
+			 <K210_FPIOA(32, K210_PCF_GPIOHS16)>; /* cs */
+	};
+
+	i2s0_pinctrl: i2s0-pinmux {
+		pinmux = <K210_FPIOA(33, K210_PCF_I2S0_IN_D0)>,
+			 <K210_FPIOA(34, K210_PCF_I2S0_WS)>,
+			 <K210_FPIOA(35, K210_PCF_I2S0_SCLK)>;
+	};
+};
+
+&uarths0 {
+	pinctrl-0 = <&uarths_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio0 {
+	pinctrl-0 = <&gpiohs_pinctrl>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&i2s0 {
+	#sound-dai-cells = <1>;
+	pinctrl-0 = <&i2s0_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&dvp0 {
+	pinctrl-0 = <&dvp_pinctrl>;
+	pinctrl-names = "default";
+};
+
+&spi0 {
+	pinctrl-0 = <&spi0_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 20 GPIO_ACTIVE_HIGH>;
+
+	panel@0 {
+		compatible = "ilitek,ili9341";
+		reg = <0>;
+		dc-gpios = <&gpio0 21 GPIO_ACTIVE_HIGH>;
+		spi-max-frequency = <15000000>;
+		status = "disabled";
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&spi1_pinctrl>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 16 GPIO_ACTIVE_LOW>;
+	status = "okay";
+
+	slot@0 {
+		compatible = "mmc-spi-slot";
+		reg = <0>;
+		voltage-ranges = <3300 3300>;
+		spi-max-frequency = <25000000>;
+		broken-cd;
+	};
+};
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 03/16] dt-bindings: update risc-v cpu properties
  2021-02-05  6:58 ` [PATCH v16 03/16] dt-bindings: update risc-v cpu properties Damien Le Moal
@ 2021-02-05 19:47   ` Rob Herring
  0 siblings, 0 replies; 27+ messages in thread
From: Rob Herring @ 2021-02-05 19:47 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: devicetree, Paul Walmsley, Palmer Dabbelt, Sean Anderson,
	Anup Patel, Atish Patra, linux-riscv

On Fri, 05 Feb 2021 15:58:14 +0900, Damien Le Moal wrote:
> The Canaan Kendryte K210 SoC CPU cores are based on a rocket chip
> version using a draft verion of the RISC-V ISA specifications. To avoid
> any confusion with CPU cores using stable specifications, add the
> compatible string "canaan,k210" for this SoC CPU cores.
> 
> Also add the "riscv,none" value to the mmu-type property to allow a DT
> to indicate that the CPU being described does not have an MMU or that
> it has an MMU that is not usable (which is the case for the K210 SoC).
> 
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> Reviewed-by: Atish Patra <atish.patra@wdc.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 04/16] dt-bindings: update sifive plic compatible string
  2021-02-05  6:58 ` [PATCH v16 04/16] dt-bindings: update sifive plic compatible string Damien Le Moal
@ 2021-02-05 19:47   ` Rob Herring
  0 siblings, 0 replies; 27+ messages in thread
From: Rob Herring @ 2021-02-05 19:47 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Anup Patel, devicetree, Paul Walmsley, Palmer Dabbelt,
	linux-riscv, Sean Anderson, Atish Patra

On Fri, 05 Feb 2021 15:58:15 +0900, Damien Le Moal wrote:
> Add the compatible string "canaan,k210-plic" to the Sifive plic bindings
> to indicate the use of the "sifive,plic-1.0.0" IP block in the Canaan
> Kendryte K210 SoC. The description is also updated to reflect this
> change, that is, that SoCs from other vendors may also use this plic
> implementation.
> 
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  .../interrupt-controller/sifive,plic-1.0.0.yaml     | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 05/16] dt-bindings: update sifive clint compatible string
  2021-02-05  6:58 ` [PATCH v16 05/16] dt-bindings: update sifive clint " Damien Le Moal
@ 2021-02-05 19:48   ` Rob Herring
  0 siblings, 0 replies; 27+ messages in thread
From: Rob Herring @ 2021-02-05 19:48 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Atish Patra, linux-riscv, devicetree, Sean Anderson, Anup Patel,
	Palmer Dabbelt

On Fri, 05 Feb 2021 15:58:16 +0900, Damien Le Moal wrote:
> Add the "canaan,k210-clint" compatible string to the Sifive clint
> bindings to indicate the use of the "sifive,clint0" IP block in the
> Canaan Kendryte K210 SoC. The description of the compatible string
> property is also updated to reflect this addition.
> 
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  .../devicetree/bindings/timer/sifive,clint.yaml      | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-05  6:58 ` [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree Damien Le Moal
@ 2021-02-05 20:25   ` Rob Herring
  2021-02-05 22:52     ` Sean Anderson
  2021-02-06  0:13     ` Damien Le Moal
  0 siblings, 2 replies; 27+ messages in thread
From: Rob Herring @ 2021-02-05 20:25 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Palmer Dabbelt, linux-riscv, Atish Patra, Anup Patel,
	Sean Anderson, devicetree

On Fri, Feb 05, 2021 at 03:58:20PM +0900, Damien Le Moal wrote:
> Update the Canaan Kendryte K210 base device tree k210.dtsi to define
> all peripherals of the SoC, their clocks and reset lines. The device
> tree file k210.dts is renamed to k210_generic.dts and becomes the
> default value selection of the SOC_CANAAN_K210_DTB_BUILTIN_SOURCE
> configuration option. No device beside the serial console is defined by
> this device tree. This makes this generic device tree suitable for use
> with a builtin initramfs with all known K210 based boards.
> 
> These changes result in the K210_CLK_ACLK clock ID to be unused and
> removed from the dt-bindings k210-clk.h header file.
> 
> Most updates to the k210.dtsi file come from Sean Anderson's work on
> U-Boot support for the K210.
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  arch/riscv/Kconfig.socs                     |   2 +-
>  arch/riscv/boot/dts/canaan/k210.dts         |  23 -
>  arch/riscv/boot/dts/canaan/k210.dtsi        | 535 +++++++++++++++++++-
>  arch/riscv/boot/dts/canaan/k210_generic.dts |  46 ++
>  include/dt-bindings/clock/k210-clk.h        |   1 -
>  5 files changed, 554 insertions(+), 53 deletions(-)
>  delete mode 100644 arch/riscv/boot/dts/canaan/k210.dts
>  create mode 100644 arch/riscv/boot/dts/canaan/k210_generic.dts
> 
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 6402746c68f3..7efcece8896c 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -51,7 +51,7 @@ config SOC_CANAAN_K210_DTB_SOURCE
>  	string "Source file for the Canaan Kendryte K210 builtin DTB"
>  	depends on SOC_CANAAN
>  	depends on SOC_CANAAN_K210_DTB_BUILTIN
> -	default "k210"
> +	default "k210_generic"
>  	help
>  	  Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
>  	  for the DTS file that will be used to produce the DTB linked into the
> diff --git a/arch/riscv/boot/dts/canaan/k210.dts b/arch/riscv/boot/dts/canaan/k210.dts
> deleted file mode 100644
> index 0d1f28fce6b2..000000000000
> --- a/arch/riscv/boot/dts/canaan/k210.dts
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> - */
> -
> -/dts-v1/;
> -
> -#include "k210.dtsi"
> -
> -/ {
> -	model = "Kendryte K210 generic";
> -	compatible = "kendryte,k210";
> -
> -	chosen {
> -		bootargs = "earlycon console=ttySIF0";
> -		stdout-path = "serial0";
> -	};
> -};
> -
> -&uarths0 {
> -	status = "okay";
> -};
> -
> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
> index 354b263195a3..63c1f4c98d6c 100644
> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
> @@ -1,9 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * Copyright (C) 2019 Sean Anderson <seanga2@gmail.com>
> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
>   * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>   */
>  #include <dt-bindings/clock/k210-clk.h>
> +#include <dt-bindings/pinctrl/k210-fpioa.h>
> +#include <dt-bindings/reset/k210-rst.h>
>  
>  / {
>  	/*
> @@ -12,14 +14,33 @@ / {
>  	 */
>  	#address-cells = <1>;
>  	#size-cells = <1>;
> -	compatible = "kendryte,k210";
> +	compatible = "canaan,kendryte-k210";
>  
>  	aliases {
> +		cpu0 = &cpu0;
> +		cpu1 = &cpu1;
> +		dma0 = &dmac0;
> +		gpio0 = &gpio0;
> +		gpio1 = &gpio1_0;
> +		i2c0 = &i2c0;
> +		i2c1 = &i2c1;
> +		i2c2 = &i2c2;
> +		pinctrl0 = &fpioa;
>  		serial0 = &uarths0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		serial3 = &uart3;
> +		spi0 = &spi0;
> +		spi1 = &spi1;
> +		spi2 = &spi2;
> +		spi3 = &spi3;
> +		timer0 = &timer0;
> +		timer1 = &timer1;
> +		timer2 = &timer2;

Don't add random aliases. Really, only the serialN ones should be here. 
cpu, dma, pinctrl, timer are definitely non-standard.

>  	};
>  
>  	/*
> -	 * The K210 has an sv39 MMU following the priviledge specification v1.9.
> +	 * The K210 has an sv39 MMU following the privileged specification v1.9.
>  	 * Since this is a non-ratified draft specification, the kernel does not
>  	 * support it and the K210 support enabled only for the !MMU case.
>  	 * Be consistent with this by setting the CPUs MMU type to "none".
> @@ -30,14 +51,14 @@ cpus {
>  		timebase-frequency = <7800000>;
>  		cpu0: cpu@0 {
>  			device_type = "cpu";
> +			compatible = "canaan,k210", "riscv";
>  			reg = <0>;
> -			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>  			riscv,isa = "rv64imafdc";
> -			mmu-type = "none";
> -			i-cache-size = <0x8000>;
> +			mmu-type = "riscv,none";
>  			i-cache-block-size = <64>;
> -			d-cache-size = <0x8000>;
> +			i-cache-size = <0x8000>;
>  			d-cache-block-size = <64>;
> +			d-cache-size = <0x8000>;
>  			cpu0_intc: interrupt-controller {
>  				#interrupt-cells = <1>;
>  				interrupt-controller;
> @@ -46,14 +67,14 @@ cpu0_intc: interrupt-controller {
>  		};
>  		cpu1: cpu@1 {
>  			device_type = "cpu";
> +			compatible = "canaan,k210", "riscv";
>  			reg = <1>;
> -			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>  			riscv,isa = "rv64imafdc";
> -			mmu-type = "none";
> -			i-cache-size = <0x8000>;
> +			mmu-type = "riscv,none";
>  			i-cache-block-size = <64>;
> -			d-cache-size = <0x8000>;
> +			i-cache-size = <0x8000>;
>  			d-cache-block-size = <64>;
> +			d-cache-size = <0x8000>;
>  			cpu1_intc: interrupt-controller {
>  				#interrupt-cells = <1>;
>  				interrupt-controller;
> @@ -64,10 +85,15 @@ cpu1_intc: interrupt-controller {
>  
>  	sram: memory@80000000 {
>  		device_type = "memory";
> +		compatible = "canaan,k210-sram";
>  		reg = <0x80000000 0x400000>,
>  		      <0x80400000 0x200000>,
>  		      <0x80600000 0x200000>;
>  		reg-names = "sram0", "sram1", "aisram";
> +		clocks = <&sysclk K210_CLK_SRAM0>,
> +			 <&sysclk K210_CLK_SRAM1>,
> +			 <&sysclk K210_CLK_AI>;
> +		clock-names = "sram0", "sram1", "aisram";
>  	};
>  
>  	clocks {
> @@ -81,40 +107,493 @@ in0: oscillator {
>  	soc {
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> -		compatible = "kendryte,k210-soc", "simple-bus";
> +		compatible = "simple-bus";
>  		ranges;
>  		interrupt-parent = <&plic0>;
>  
> -		sysctl: sysctl@50440000 {
> -			compatible = "kendryte,k210-sysctl", "simple-mfd";
> -			reg = <0x50440000 0x1000>;
> -			#clock-cells = <1>;
> +		debug0: debug@0 {
> +			compatible = "canaan,k210-debug", "riscv,debug";

Not documented.

> +			reg = <0x0 0x1000>;
> +			status = "disabled";
> +		};
> +
> +		rom0: nvmem@1000 {
> +			reg = <0x1000 0x1000>;
> +			read-only;
> +			status = "disabled";
>  		};
>  
>  		clint0: clint@2000000 {

interrupt-controller@...

> -			#interrupt-cells = <1>;
> -			compatible = "riscv,clint0";
> +			compatible = "canaan,k210-clint", "sifive,clint0";
>  			reg = <0x2000000 0xC000>;
> -			interrupts-extended =  <&cpu0_intc 3 &cpu0_intc 7
> -						&cpu1_intc 3 &cpu1_intc 7>;
> +			interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> +					      &cpu1_intc 3 &cpu1_intc 7>;
>  		};
>  
> -		plic0: interrupt-controller@c000000 {
> +		plic0: interrupt-controller@C000000 {

No, lowercase hex in unit-addresses was correct.

>  			#interrupt-cells = <1>;
> -			interrupt-controller;
> -			compatible = "kendryte,k210-plic0", "riscv,plic0";
> +			#address-cells = <0>;
> +			compatible = "canaan,k210-plic", "sifive,plic-1.0.0";
>  			reg = <0xC000000 0x4000000>;
> -			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 0xffffffff>,
> -					      <&cpu1_intc 11>, <&cpu1_intc 0xffffffff>;
> +			interrupt-controller;
> +			interrupts-extended = <&cpu0_intc 11 &cpu1_intc 11>;
>  			riscv,ndev = <65>;
> -			riscv,max-priority = <7>;
>  		};
>  
>  		uarths0: serial@38000000 {
> -			compatible = "kendryte,k210-uarths", "sifive,uart0";
> +			compatible = "canaan,k210-uarths", "sifive,uart0";
>  			reg = <0x38000000 0x1000>;
>  			interrupts = <33>;
> -			clocks = <&sysctl K210_CLK_CPU>;
> +			clocks = <&sysclk K210_CLK_CPU>;
> +			status = "disabled";
> +		};
> +
> +		gpio0: gpio-controller@38001000 {
> +			#interrupt-cells = <2>;
> +			#gpio-cells = <2>;
> +			compatible = "canaan,k210-gpiohs", "sifive,gpio0";
> +			reg = <0x38001000 0x1000>;
> +			interrupt-controller;
> +			interrupts = <34 35 36 37 38 39 40 41
> +				      42 43 44 45 46 47 48 49
> +				      50 51 52 53 54 55 56 57
> +				      58 59 60 61 62 63 64 65>;
> +			gpio-controller;
> +			ngpios = <32>;
> +			status = "disabled";
> +		};
> +
> +		kpu0: kpu@40800000 {
> +			compatible = "canaan,k210-kpu";
> +			reg = <0x40800000 0xc00000>;
> +			interrupts = <25>;
> +			clocks = <&sysclk K210_CLK_AI>;
> +			status = "disabled";
> +		};
> +
> +		fft0: fft@42000000 {
> +			compatible = "canaan,k210-fft";
> +			reg = <0x42000000 0x400000>;
> +			interrupts = <26>;
> +			clocks = <&sysclk K210_CLK_FFT>;
> +			resets = <&sysrst K210_RST_FFT>;
> +			status = "disabled";
> +		};
> +
> +		dmac0: dma-controller@50000000 {
> +			compatible = "snps,axi-dma-1.01a";
> +			reg = <0x50000000 0x1000>;
> +			interrupts = <27 28 29 30 31 32>;
> +			clocks = <&sysclk K210_CLK_DMA>, <&sysclk K210_CLK_DMA>;
> +			clock-names = "core-clk", "cfgr-clk";
> +			resets = <&sysrst K210_RST_DMA>;
> +			dma-channels = <6>;
> +			snps,dma-masters = <2>;
> +			snps,priority = <0 1 2 3 4 5>;
> +			snps,data-width = <5>;
> +			snps,block-size = <0x200000 0x200000 0x200000
> +					   0x200000 0x200000 0x200000>;
> +			snps,axi-max-burst-len = <256>;
> +			status = "disabled";
> +		};
> +
> +		apb0: bus@50200000 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "simple-pm-bus";
> +			ranges;
> +			clocks = <&sysclk K210_CLK_APB0>;
> +
> +			gpio1: gpio@50200000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "snps,dw-apb-gpio";
> +				reg = <0x50200000 0x80>;
> +				clocks = <&sysclk K210_CLK_APB0>,
> +					 <&sysclk K210_CLK_GPIO>;
> +				clock-names = "bus", "db";
> +				resets = <&sysrst K210_RST_GPIO>;
> +				status = "disabled";
> +
> +				gpio1_0: gpio-port@0 {
> +					#gpio-cells = <2>;
> +					#interrupt-cells = <2>;
> +					compatible = "snps,dw-apb-gpio-port";
> +					reg = <0>;
> +					interrupt-controller;
> +					interrupts = <23>;
> +					gpio-controller;
> +					ngpios = <8>;
> +				};
> +			};
> +
> +			uart1: serial@50210000 {
> +				compatible = "snps,dw-apb-uart";
> +				reg = <0x50210000 0x100>;
> +				interrupts = <11>;
> +				clocks = <&sysclk K210_CLK_UART1>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "baudclk", "apb_pclk";
> +				resets = <&sysrst K210_RST_UART1>;
> +				reg-io-width = <4>;
> +				reg-shift = <2>;
> +				dcd-override;
> +				dsr-override;
> +				cts-override;
> +				ri-override;
> +				status = "disabled";
> +			};
> +
> +			uart2: serial@50220000 {
> +				compatible = "snps,dw-apb-uart";
> +				reg = <0x50220000 0x100>;
> +				interrupts = <12>;
> +				clocks = <&sysclk K210_CLK_UART2>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "baudclk", "apb_pclk";
> +				resets = <&sysrst K210_RST_UART2>;
> +				reg-io-width = <4>;
> +				reg-shift = <2>;
> +				dcd-override;
> +				dsr-override;
> +				cts-override;
> +				ri-override;
> +				status = "disabled";
> +			};
> +
> +			uart3: serial@50230000 {
> +				compatible = "snps,dw-apb-uart";
> +				reg = <0x50230000 0x100>;
> +				interrupts = <13>;
> +				clocks = <&sysclk K210_CLK_UART3>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "baudclk", "apb_pclk";
> +				resets = <&sysrst K210_RST_UART3>;
> +				reg-io-width = <4>;
> +				reg-shift = <2>;
> +				dcd-override;
> +				dsr-override;
> +				cts-override;
> +				ri-override;
> +				status = "disabled";
> +			};
> +
> +			spi2: spi@50240000 {
> +				compatible = "canaan,k210-spi";
> +				spi-slave;
> +				reg = <0x50240000 0x100>;
> +				interrupts = <3>;
> +				clocks = <&sysclk K210_CLK_SPI2>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "ssi_clk", "pclk";
> +				resets = <&sysrst K210_RST_SPI2>;
> +				spi-max-frequency = <25000000>;
> +				status = "disabled";
> +			};
> +
> +			i2s0: i2s@50250000 {
> +				compatible = "snps,designware-i2s";
> +				reg = <0x50250000 0x200>;
> +				interrupts = <5>;
> +				clocks = <&sysclk K210_CLK_I2S0>;
> +				clock-names = "i2sclk";
> +				resets = <&sysrst K210_RST_I2S0>;
> +				status = "disabled";
> +			};
> +
> +			apu0: sound@520250200 {
> +				compatible = "canaan,k210-apu";
> +				reg = <0x50250200 0x200>;

The unit-address and 'reg' value don't match.

> +				status = "disabled";
> +			};
> +
> +			i2s1: i2s@50260000 {
> +				compatible = "snps,designware-i2s";
> +				reg = <0x50260000 0x200>;
> +				interrupts = <6>;
> +				clocks = <&sysclk K210_CLK_I2S1>;
> +				clock-names = "i2sclk";
> +				resets = <&sysrst K210_RST_I2S1>;
> +				status = "disabled";
> +			};
> +
> +			i2s2: i2s@50270000 {
> +				compatible = "snps,designware-i2s";
> +				reg = <0x50270000 0x200>;
> +				interrupts = <7>;
> +				clocks = <&sysclk K210_CLK_I2S2>;
> +				clock-names = "i2sclk";
> +				resets = <&sysrst K210_RST_I2S2>;
> +				status = "disabled";
> +			};
> +
> +			i2c0: i2c@50280000 {
> +				compatible = "snps,designware-i2c";
> +				reg = <0x50280000 0x100>;
> +				interrupts = <8>;
> +				clocks = <&sysclk K210_CLK_I2C0>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "ref", "pclk";
> +				resets = <&sysrst K210_RST_I2C0>;
> +				status = "disabled";
> +			};
> +
> +			i2c1: i2c@50290000 {
> +				compatible = "snps,designware-i2c";
> +				reg = <0x50290000 0x100>;
> +				interrupts = <9>;
> +				clocks = <&sysclk K210_CLK_I2C1>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "ref", "pclk";
> +				resets = <&sysrst K210_RST_I2C1>;
> +				status = "disabled";
> +			};
> +
> +			i2c2: i2c@502A0000 {

Again, lowercase hex.

> +				compatible = "snps,designware-i2c";
> +				reg = <0x502A0000 0x100>;
> +				interrupts = <10>;
> +				clocks = <&sysclk K210_CLK_I2C2>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "ref", "pclk";
> +				resets = <&sysrst K210_RST_I2C2>;
> +				status = "disabled";
> +			};
> +
> +			fpioa: pinmux@502B0000 {
> +				compatible = "canaan,k210-fpioa";
> +				reg = <0x502B0000 0x100>;
> +				clocks = <&sysclk K210_CLK_FPIOA>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "ref", "pclk";
> +				resets = <&sysrst K210_RST_FPIOA>;
> +				canaan,k210-sysctl-power = <&sysctl 108>;
> +				status = "disabled";
> +			};
> +
> +			sha256: sha256@502C0000 {
> +				compatible = "canaan,k210-sha256";
> +				reg = <0x502C0000 0x100>;
> +				clocks = <&sysclk K210_CLK_SHA>;
> +				resets = <&sysrst K210_RST_SHA>;
> +				status = "disabled";
> +			};
> +
> +			timer0: timer@502D0000 {
> +				compatible = "snps,dw-apb-timer";
> +				reg = <0x502D0000 0x100>;
> +				interrupts = <14 15>;
> +				clocks = <&sysclk K210_CLK_TIMER0>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "timer", "pclk";
> +				resets = <&sysrst K210_RST_TIMER0>;
> +				status = "disabled";
> +			};
> +
> +			timer1: timer@502E0000 {
> +				compatible = "snps,dw-apb-timer";
> +				reg = <0x502E0000 0x100>;
> +				interrupts = <16 17>;
> +				clocks = <&sysclk K210_CLK_TIMER1>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "timer", "pclk";
> +				resets = <&sysrst K210_RST_TIMER1>;
> +				status = "disabled";
> +			};
> +
> +			timer2: timer@502F0000 {
> +				compatible = "snps,dw-apb-timer";
> +				reg = <0x502F0000 0x100>;
> +				interrupts = <18 19>;
> +				clocks = <&sysclk K210_CLK_TIMER2>,
> +					 <&sysclk K210_CLK_APB0>;
> +				clock-names = "timer", "pclk";
> +				resets = <&sysrst K210_RST_TIMER2>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		apb1: bus@50400000 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "simple-pm-bus";
> +			ranges;
> +			clocks = <&sysclk K210_CLK_APB1>;
> +
> +			wdt0: watchdog@50400000 {
> +				compatible = "snps,dw-wdt";
> +				reg = <0x50400000 0x100>;
> +				interrupts = <21>;
> +				clocks = <&sysclk K210_CLK_WDT0>,
> +					 <&sysclk K210_CLK_APB1>;
> +				clock-names = "tclk", "pclk";
> +				resets = <&sysrst K210_RST_WDT0>;
> +				status = "disabled";
> +			};
> +
> +			wdt1: watchdog@50410000 {
> +				compatible = "snps,dw-wdt";
> +				reg = <0x50410000 0x100>;
> +				interrupts = <22>;
> +				clocks = <&sysclk K210_CLK_WDT1>,
> +					 <&sysclk K210_CLK_APB1>;
> +				clock-names = "tclk", "pclk";
> +				resets = <&sysrst K210_RST_WDT1>;
> +				status = "disabled";
> +			};
> +
> +			otp0: nvmem@50420000 {
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				compatible = "canaan,k210-otp";
> +				reg = <0x50420000 0x100>,
> +				      <0x88000000 0x20000>;
> +				reg-names = "reg", "mem";
> +				clocks = <&sysclk K210_CLK_ROM>;
> +				resets = <&sysrst K210_RST_ROM>;
> +				read-only;
> +				status = "disabled";

Your disabled nodes seem a bit excessive. A device should really only be 
disabled if it's a board level decision to use or not. I'd assume the 
OTP is always there and usable.

> +
> +				/* Bootloader */
> +				firmware@00000 {

Drop leading 0s.

Is this memory mapped? If so, you are missing 'ranges' in the parent to 
make it translateable.

> +					reg = <0x00000 0xC200>;
> +				};
> +
> +				/*
> +				 * config string as described in RISC-V
> +				 * privileged spec 1.9
> +				 */
> +				config-1-9@1c000 {
> +					reg = <0x1C000 0x1000>;
> +				};
> +
> +				/*
> +				 * Device tree containing only registers,
> +				 * interrupts, and cpus
> +				 */
> +				fdt@1d000 {
> +					reg = <0x1D000 0x2000>;
> +				};
> +
> +				/* CPU/ROM credits */
> +				credits@1f000 {
> +					reg = <0x1F000 0x1000>;
> +				};
> +			};
> +
> +			dvp0: camera@50430000 {
> +				compatible = "canaan,k210-dvp";

No documented. Seems to be several of them.

> +				reg = <0x50430000 0x100>;
> +				interrupts = <24>;
> +				clocks = <&sysclk K210_CLK_DVP>;
> +				resets = <&sysrst K210_RST_DVP>;
> +				canaan,k210-misc-offset = <&sysctl 84>;
> +				status = "disabled";
> +			};
> +
> +			sysctl: syscon@50440000 {
> +				compatible = "canaan,k210-sysctl",
> +					     "syscon", "simple-mfd";
> +				reg = <0x50440000 0x100>;
> +				clocks = <&sysclk K210_CLK_APB1>;
> +				clock-names = "pclk";
> +
> +				sysclk: clock-controller {
> +					#clock-cells = <1>;
> +					compatible = "canaan,k210-clk";
> +					clocks = <&in0>;
> +				};
> +
> +				sysrst: reset-controller {
> +					compatible = "canaan,k210-rst";
> +					#reset-cells = <1>;
> +				};
> +
> +				reboot: syscon-reboot {
> +					compatible = "syscon-reboot";
> +					regmap = <&sysctl>;
> +					offset = <48>;
> +					mask = <1>;
> +					value = <1>;
> +				};
> +			};
> +
> +			aes0: aes@50450000 {
> +				compatible = "canaan,k210-aes";
> +				reg = <0x50450000 0x100>;
> +				clocks = <&sysclk K210_CLK_AES>;
> +				resets = <&sysrst K210_RST_AES>;
> +				status = "disabled";
> +			};
> +
> +			rtc: rtc@50460000 {
> +				compatible = "canaan,k210-rtc";
> +				reg = <0x50460000 0x100>;
> +				clocks = <&in0>;
> +				resets = <&sysrst K210_RST_RTC>;
> +				interrupts = <20>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		apb2: bus@52000000 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "simple-pm-bus";
> +			ranges;
> +			clocks = <&sysclk K210_CLK_APB2>;
> +
> +			spi0: spi@52000000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "canaan,k210-spi";
> +				reg = <0x52000000 0x100>;
> +				interrupts = <1>;
> +				clocks = <&sysclk K210_CLK_SPI0>,
> +					 <&sysclk K210_CLK_APB2>;
> +				clock-names = "ssi_clk", "pclk";
> +				resets = <&sysrst K210_RST_SPI0>;
> +				reset-names = "spi";
> +				spi-max-frequency = <25000000>;
> +				num-cs = <4>;
> +				reg-io-width = <4>;
> +				status = "disabled";
> +			};
> +
> +			spi1: spi@53000000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "canaan,k210-spi";
> +				reg = <0x53000000 0x100>;
> +				interrupts = <2>;
> +				clocks = <&sysclk K210_CLK_SPI1>,
> +					 <&sysclk K210_CLK_APB2>;
> +				clock-names = "ssi_clk", "pclk";
> +				resets = <&sysrst K210_RST_SPI1>;
> +				reset-names = "spi";
> +				spi-max-frequency = <25000000>;
> +				num-cs = <4>;
> +				reg-io-width = <4>;
> +				status = "disabled";
> +			};
> +
> +			spi3: spi@54000000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "snps,dwc-ssi-1.01a";
> +				reg = <0x54000000 0x200>;
> +				interrupts = <4>;
> +				clocks = <&sysclk K210_CLK_SPI3>,
> +					 <&sysclk K210_CLK_APB2>;
> +				clock-names = "ssi_clk", "pclk";
> +				resets = <&sysrst K210_RST_SPI3>;
> +				reset-names = "spi";
> +				/* Could possibly go up to 200 MHz */
> +				spi-max-frequency = <100000000>;
> +				num-cs = <4>;
> +				reg-io-width = <4>;
> +				status = "disabled";
> +			};
>  		};
>  	};
>  };
> diff --git a/arch/riscv/boot/dts/canaan/k210_generic.dts b/arch/riscv/boot/dts/canaan/k210_generic.dts
> new file mode 100644
> index 000000000000..396c8ca4d24d
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k210_generic.dts
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> + */
> +
> +/dts-v1/;
> +
> +#include "k210.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> +	model = "Kendryte K210 generic";
> +	compatible = "canaan,kendryte-k210";
> +
> +	chosen {
> +		bootargs = "earlycon console=ttySIF0";
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&fpioa {
> +	pinctrl-0 = <&jtag_pins>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +
> +	jtag_pins: jtag-pinmux {
> +		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
> +			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
> +			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
> +			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
> +	};
> +
> +	uarths_pins: uarths-pinmux {
> +		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
> +			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
> +	};
> +};
> +
> +&uarths0 {
> +	pinctrl-0 = <&uarths_pins>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +};
> diff --git a/include/dt-bindings/clock/k210-clk.h b/include/dt-bindings/clock/k210-clk.h
> index a48176ad3c23..b2de702cbf75 100644
> --- a/include/dt-bindings/clock/k210-clk.h
> +++ b/include/dt-bindings/clock/k210-clk.h
> @@ -9,7 +9,6 @@
>  /*
>   * Kendryte K210 SoC clock identifiers (arbitrary values).
>   */
> -#define K210_CLK_ACLK	0
>  #define K210_CLK_CPU	0
>  #define K210_CLK_SRAM0	1
>  #define K210_CLK_SRAM1	2
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-05 20:25   ` Rob Herring
@ 2021-02-05 22:52     ` Sean Anderson
  2021-02-05 23:49       ` Damien Le Moal
  2021-02-08 19:49       ` Rob Herring
  2021-02-06  0:13     ` Damien Le Moal
  1 sibling, 2 replies; 27+ messages in thread
From: Sean Anderson @ 2021-02-05 22:52 UTC (permalink / raw)
  To: Rob Herring, Damien Le Moal
  Cc: Palmer Dabbelt, linux-riscv, Atish Patra, Anup Patel, devicetree

On 2/5/21 3:25 PM, Rob Herring wrote:
> On Fri, Feb 05, 2021 at 03:58:20PM +0900, Damien Le Moal wrote:
>> Update the Canaan Kendryte K210 base device tree k210.dtsi to define
>> all peripherals of the SoC, their clocks and reset lines. The device
>> tree file k210.dts is renamed to k210_generic.dts and becomes the
>> default value selection of the SOC_CANAAN_K210_DTB_BUILTIN_SOURCE
>> configuration option. No device beside the serial console is defined by
>> this device tree. This makes this generic device tree suitable for use
>> with a builtin initramfs with all known K210 based boards.
>>
>> These changes result in the K210_CLK_ACLK clock ID to be unused and
>> removed from the dt-bindings k210-clk.h header file.
>>
>> Most updates to the k210.dtsi file come from Sean Anderson's work on
>> U-Boot support for the K210.
>>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>> ---
>>   arch/riscv/Kconfig.socs                     |   2 +-
>>   arch/riscv/boot/dts/canaan/k210.dts         |  23 -
>>   arch/riscv/boot/dts/canaan/k210.dtsi        | 535 +++++++++++++++++++-
>>   arch/riscv/boot/dts/canaan/k210_generic.dts |  46 ++
>>   include/dt-bindings/clock/k210-clk.h        |   1 -
>>   5 files changed, 554 insertions(+), 53 deletions(-)
>>   delete mode 100644 arch/riscv/boot/dts/canaan/k210.dts
>>   create mode 100644 arch/riscv/boot/dts/canaan/k210_generic.dts
>>
>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>> index 6402746c68f3..7efcece8896c 100644
>> --- a/arch/riscv/Kconfig.socs
>> +++ b/arch/riscv/Kconfig.socs
>> @@ -51,7 +51,7 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>   	string "Source file for the Canaan Kendryte K210 builtin DTB"
>>   	depends on SOC_CANAAN
>>   	depends on SOC_CANAAN_K210_DTB_BUILTIN
>> -	default "k210"
>> +	default "k210_generic"
>>   	help
>>   	  Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
>>   	  for the DTS file that will be used to produce the DTB linked into the
>> diff --git a/arch/riscv/boot/dts/canaan/k210.dts b/arch/riscv/boot/dts/canaan/k210.dts
>> deleted file mode 100644
>> index 0d1f28fce6b2..000000000000
>> --- a/arch/riscv/boot/dts/canaan/k210.dts
>> +++ /dev/null
>> @@ -1,23 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0+
>> -/*
>> - * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>> - */
>> -
>> -/dts-v1/;
>> -
>> -#include "k210.dtsi"
>> -
>> -/ {
>> -	model = "Kendryte K210 generic";
>> -	compatible = "kendryte,k210";
>> -
>> -	chosen {
>> -		bootargs = "earlycon console=ttySIF0";
>> -		stdout-path = "serial0";
>> -	};
>> -};
>> -
>> -&uarths0 {
>> -	status = "okay";
>> -};
>> -
>> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
>> index 354b263195a3..63c1f4c98d6c 100644
>> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
>> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
>> @@ -1,9 +1,11 @@
>>   // SPDX-License-Identifier: GPL-2.0+
>>   /*
>> - * Copyright (C) 2019 Sean Anderson <seanga2@gmail.com>
>> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
>>    * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>>    */
>>   #include <dt-bindings/clock/k210-clk.h>
>> +#include <dt-bindings/pinctrl/k210-fpioa.h>
>> +#include <dt-bindings/reset/k210-rst.h>
>>   
>>   / {
>>   	/*
>> @@ -12,14 +14,33 @@ / {
>>   	 */
>>   	#address-cells = <1>;
>>   	#size-cells = <1>;
>> -	compatible = "kendryte,k210";
>> +	compatible = "canaan,kendryte-k210";
>>   
>>   	aliases {
>> +		cpu0 = &cpu0;
>> +		cpu1 = &cpu1;
>> +		dma0 = &dmac0;
>> +		gpio0 = &gpio0;
>> +		gpio1 = &gpio1_0;
>> +		i2c0 = &i2c0;
>> +		i2c1 = &i2c1;
>> +		i2c2 = &i2c2;
>> +		pinctrl0 = &fpioa;
>>   		serial0 = &uarths0;
>> +		serial1 = &uart1;
>> +		serial2 = &uart2;
>> +		serial3 = &uart3;
>> +		spi0 = &spi0;
>> +		spi1 = &spi1;
>> +		spi2 = &spi2;
>> +		spi3 = &spi3;
>> +		timer0 = &timer0;
>> +		timer1 = &timer1;
>> +		timer2 = &timer2;
> 
> Don't add random aliases. Really, only the serialN ones should be here.
> cpu, dma, pinctrl, timer are definitely non-standard.


All of these except for cpu and dma are already found in the kernel.
They were primarily added for U-Boot.

> 
>>   	};
>>   
>>   	/*
>> -	 * The K210 has an sv39 MMU following the priviledge specification v1.9.
>> +	 * The K210 has an sv39 MMU following the privileged specification v1.9.
>>   	 * Since this is a non-ratified draft specification, the kernel does not
>>   	 * support it and the K210 support enabled only for the !MMU case.
>>   	 * Be consistent with this by setting the CPUs MMU type to "none".
>> @@ -30,14 +51,14 @@ cpus {
>>   		timebase-frequency = <7800000>;
>>   		cpu0: cpu@0 {
>>   			device_type = "cpu";
>> +			compatible = "canaan,k210", "riscv";
>>   			reg = <0>;
>> -			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>>   			riscv,isa = "rv64imafdc";
>> -			mmu-type = "none";
>> -			i-cache-size = <0x8000>;
>> +			mmu-type = "riscv,none";
>>   			i-cache-block-size = <64>;
>> -			d-cache-size = <0x8000>;
>> +			i-cache-size = <0x8000>;
>>   			d-cache-block-size = <64>;
>> +			d-cache-size = <0x8000>;
>>   			cpu0_intc: interrupt-controller {
>>   				#interrupt-cells = <1>;
>>   				interrupt-controller;
>> @@ -46,14 +67,14 @@ cpu0_intc: interrupt-controller {
>>   		};
>>   		cpu1: cpu@1 {
>>   			device_type = "cpu";
>> +			compatible = "canaan,k210", "riscv";
>>   			reg = <1>;
>> -			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>>   			riscv,isa = "rv64imafdc";
>> -			mmu-type = "none";
>> -			i-cache-size = <0x8000>;
>> +			mmu-type = "riscv,none";
>>   			i-cache-block-size = <64>;
>> -			d-cache-size = <0x8000>;
>> +			i-cache-size = <0x8000>;
>>   			d-cache-block-size = <64>;
>> +			d-cache-size = <0x8000>;
>>   			cpu1_intc: interrupt-controller {
>>   				#interrupt-cells = <1>;
>>   				interrupt-controller;
>> @@ -64,10 +85,15 @@ cpu1_intc: interrupt-controller {
>>   
>>   	sram: memory@80000000 {
>>   		device_type = "memory";
>> +		compatible = "canaan,k210-sram";
>>   		reg = <0x80000000 0x400000>,
>>   		      <0x80400000 0x200000>,
>>   		      <0x80600000 0x200000>;
>>   		reg-names = "sram0", "sram1", "aisram";
>> +		clocks = <&sysclk K210_CLK_SRAM0>,
>> +			 <&sysclk K210_CLK_SRAM1>,
>> +			 <&sysclk K210_CLK_AI>;
>> +		clock-names = "sram0", "sram1", "aisram";
>>   	};
>>   
>>   	clocks {
>> @@ -81,40 +107,493 @@ in0: oscillator {
>>   	soc {
>>   		#address-cells = <1>;
>>   		#size-cells = <1>;
>> -		compatible = "kendryte,k210-soc", "simple-bus";
>> +		compatible = "simple-bus";
>>   		ranges;
>>   		interrupt-parent = <&plic0>;
>>   
>> -		sysctl: sysctl@50440000 {
>> -			compatible = "kendryte,k210-sysctl", "simple-mfd";
>> -			reg = <0x50440000 0x1000>;
>> -			#clock-cells = <1>;
>> +		debug0: debug@0 {
>> +			compatible = "canaan,k210-debug", "riscv,debug";
> 
> Not documented.

On 1/14/21 7:06 PM, Sean Anderson wrote:
> Here it's because "riscv,debug" doesn't exist. This is the "debug"
> device as described in the debug spec. AFAIK Linux never needs to
> configure this device. It could probably be removed. 

No objections here.

> 
>> +			reg = <0x0 0x1000>;
>> +			status = "disabled";
>> +		};
>> +
>> +		rom0: nvmem@1000 {
>> +			reg = <0x1000 0x1000>;
>> +			read-only;
>> +			status = "disabled";
>>   		};
>>   
>>   		clint0: clint@2000000 {
> 
> interrupt-controller@...

Yes, this should be changed.

> 
>> -			#interrupt-cells = <1>;
>> -			compatible = "riscv,clint0";
>> +			compatible = "canaan,k210-clint", "sifive,clint0";
>>   			reg = <0x2000000 0xC000>;
>> -			interrupts-extended =  <&cpu0_intc 3 &cpu0_intc 7
>> -						&cpu1_intc 3 &cpu1_intc 7>;
>> +			interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>> +					      &cpu1_intc 3 &cpu1_intc 7>;
>>   		};
>>   
>> -		plic0: interrupt-controller@c000000 {
>> +		plic0: interrupt-controller@C000000 {
> 
> No, lowercase hex in unit-addresses was correct.

Do you have any authoritative source for this? When I was creating this
tree, I didn't see anything about what case the addresses should be in
(and a quick grep for lower-case in Documentation/devicetree doesn't
have any relevant results).

> 
>>   			#interrupt-cells = <1>;
>> -			interrupt-controller;
>> -			compatible = "kendryte,k210-plic0", "riscv,plic0";
>> +			#address-cells = <0>;
>> +			compatible = "canaan,k210-plic", "sifive,plic-1.0.0";
>>   			reg = <0xC000000 0x4000000>;
>> -			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 0xffffffff>,
>> -					      <&cpu1_intc 11>, <&cpu1_intc 0xffffffff>;
>> +			interrupt-controller;
>> +			interrupts-extended = <&cpu0_intc 11 &cpu1_intc 11>;
>>   			riscv,ndev = <65>;
>> -			riscv,max-priority = <7>;
>>   		};
>>   
>>   		uarths0: serial@38000000 {
>> -			compatible = "kendryte,k210-uarths", "sifive,uart0";
>> +			compatible = "canaan,k210-uarths", "sifive,uart0";
>>   			reg = <0x38000000 0x1000>;
>>   			interrupts = <33>;
>> -			clocks = <&sysctl K210_CLK_CPU>;
>> +			clocks = <&sysclk K210_CLK_CPU>;
>> +			status = "disabled";
>> +		};
>> +
>> +		gpio0: gpio-controller@38001000 {
>> +			#interrupt-cells = <2>;
>> +			#gpio-cells = <2>;
>> +			compatible = "canaan,k210-gpiohs", "sifive,gpio0";
>> +			reg = <0x38001000 0x1000>;
>> +			interrupt-controller;
>> +			interrupts = <34 35 36 37 38 39 40 41
>> +				      42 43 44 45 46 47 48 49
>> +				      50 51 52 53 54 55 56 57
>> +				      58 59 60 61 62 63 64 65>;
>> +			gpio-controller;
>> +			ngpios = <32>;
>> +			status = "disabled";
>> +		};
>> +
>> +		kpu0: kpu@40800000 {
>> +			compatible = "canaan,k210-kpu";
>> +			reg = <0x40800000 0xc00000>;
>> +			interrupts = <25>;
>> +			clocks = <&sysclk K210_CLK_AI>;
>> +			status = "disabled";
>> +		};
>> +
>> +		fft0: fft@42000000 {
>> +			compatible = "canaan,k210-fft";
>> +			reg = <0x42000000 0x400000>;
>> +			interrupts = <26>;
>> +			clocks = <&sysclk K210_CLK_FFT>;
>> +			resets = <&sysrst K210_RST_FFT>;
>> +			status = "disabled";
>> +		};
>> +
>> +		dmac0: dma-controller@50000000 {
>> +			compatible = "snps,axi-dma-1.01a";
>> +			reg = <0x50000000 0x1000>;
>> +			interrupts = <27 28 29 30 31 32>;
>> +			clocks = <&sysclk K210_CLK_DMA>, <&sysclk K210_CLK_DMA>;
>> +			clock-names = "core-clk", "cfgr-clk";
>> +			resets = <&sysrst K210_RST_DMA>;
>> +			dma-channels = <6>;
>> +			snps,dma-masters = <2>;
>> +			snps,priority = <0 1 2 3 4 5>;
>> +			snps,data-width = <5>;
>> +			snps,block-size = <0x200000 0x200000 0x200000
>> +					   0x200000 0x200000 0x200000>;
>> +			snps,axi-max-burst-len = <256>;
>> +			status = "disabled";
>> +		};
>> +
>> +		apb0: bus@50200000 {
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			compatible = "simple-pm-bus";
>> +			ranges;
>> +			clocks = <&sysclk K210_CLK_APB0>;
>> +
>> +			gpio1: gpio@50200000 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				compatible = "snps,dw-apb-gpio";
>> +				reg = <0x50200000 0x80>;
>> +				clocks = <&sysclk K210_CLK_APB0>,
>> +					 <&sysclk K210_CLK_GPIO>;
>> +				clock-names = "bus", "db";
>> +				resets = <&sysrst K210_RST_GPIO>;
>> +				status = "disabled";
>> +
>> +				gpio1_0: gpio-port@0 {
>> +					#gpio-cells = <2>;
>> +					#interrupt-cells = <2>;
>> +					compatible = "snps,dw-apb-gpio-port";
>> +					reg = <0>;
>> +					interrupt-controller;
>> +					interrupts = <23>;
>> +					gpio-controller;
>> +					ngpios = <8>;
>> +				};
>> +			};
>> +
>> +			uart1: serial@50210000 {
>> +				compatible = "snps,dw-apb-uart";
>> +				reg = <0x50210000 0x100>;
>> +				interrupts = <11>;
>> +				clocks = <&sysclk K210_CLK_UART1>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "baudclk", "apb_pclk";
>> +				resets = <&sysrst K210_RST_UART1>;
>> +				reg-io-width = <4>;
>> +				reg-shift = <2>;
>> +				dcd-override;
>> +				dsr-override;
>> +				cts-override;
>> +				ri-override;
>> +				status = "disabled";
>> +			};
>> +
>> +			uart2: serial@50220000 {
>> +				compatible = "snps,dw-apb-uart";
>> +				reg = <0x50220000 0x100>;
>> +				interrupts = <12>;
>> +				clocks = <&sysclk K210_CLK_UART2>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "baudclk", "apb_pclk";
>> +				resets = <&sysrst K210_RST_UART2>;
>> +				reg-io-width = <4>;
>> +				reg-shift = <2>;
>> +				dcd-override;
>> +				dsr-override;
>> +				cts-override;
>> +				ri-override;
>> +				status = "disabled";
>> +			};
>> +
>> +			uart3: serial@50230000 {
>> +				compatible = "snps,dw-apb-uart";
>> +				reg = <0x50230000 0x100>;
>> +				interrupts = <13>;
>> +				clocks = <&sysclk K210_CLK_UART3>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "baudclk", "apb_pclk";
>> +				resets = <&sysrst K210_RST_UART3>;
>> +				reg-io-width = <4>;
>> +				reg-shift = <2>;
>> +				dcd-override;
>> +				dsr-override;
>> +				cts-override;
>> +				ri-override;
>> +				status = "disabled";
>> +			};
>> +
>> +			spi2: spi@50240000 {
>> +				compatible = "canaan,k210-spi";
>> +				spi-slave;
>> +				reg = <0x50240000 0x100>;
>> +				interrupts = <3>;
>> +				clocks = <&sysclk K210_CLK_SPI2>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "ssi_clk", "pclk";
>> +				resets = <&sysrst K210_RST_SPI2>;
>> +				spi-max-frequency = <25000000>;
>> +				status = "disabled";
>> +			};
>> +
>> +			i2s0: i2s@50250000 {
>> +				compatible = "snps,designware-i2s";
>> +				reg = <0x50250000 0x200>;
>> +				interrupts = <5>;
>> +				clocks = <&sysclk K210_CLK_I2S0>;
>> +				clock-names = "i2sclk";
>> +				resets = <&sysrst K210_RST_I2S0>;
>> +				status = "disabled";
>> +			};
>> +
>> +			apu0: sound@520250200 {
>> +				compatible = "canaan,k210-apu";
>> +				reg = <0x50250200 0x200>;
> 
> The unit-address and 'reg' value don't match.

Good catch. The unit address should be 50250200.

> 
>> +				status = "disabled";
>> +			};
>> +
>> +			i2s1: i2s@50260000 {
>> +				compatible = "snps,designware-i2s";
>> +				reg = <0x50260000 0x200>;
>> +				interrupts = <6>;
>> +				clocks = <&sysclk K210_CLK_I2S1>;
>> +				clock-names = "i2sclk";
>> +				resets = <&sysrst K210_RST_I2S1>;
>> +				status = "disabled";
>> +			};
>> +
>> +			i2s2: i2s@50270000 {
>> +				compatible = "snps,designware-i2s";
>> +				reg = <0x50270000 0x200>;
>> +				interrupts = <7>;
>> +				clocks = <&sysclk K210_CLK_I2S2>;
>> +				clock-names = "i2sclk";
>> +				resets = <&sysrst K210_RST_I2S2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			i2c0: i2c@50280000 {
>> +				compatible = "snps,designware-i2c";
>> +				reg = <0x50280000 0x100>;
>> +				interrupts = <8>;
>> +				clocks = <&sysclk K210_CLK_I2C0>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "ref", "pclk";
>> +				resets = <&sysrst K210_RST_I2C0>;
>> +				status = "disabled";
>> +			};
>> +
>> +			i2c1: i2c@50290000 {
>> +				compatible = "snps,designware-i2c";
>> +				reg = <0x50290000 0x100>;
>> +				interrupts = <9>;
>> +				clocks = <&sysclk K210_CLK_I2C1>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "ref", "pclk";
>> +				resets = <&sysrst K210_RST_I2C1>;
>> +				status = "disabled";
>> +			};
>> +
>> +			i2c2: i2c@502A0000 {
> 
> Again, lowercase hex.
> 
>> +				compatible = "snps,designware-i2c";
>> +				reg = <0x502A0000 0x100>;
>> +				interrupts = <10>;
>> +				clocks = <&sysclk K210_CLK_I2C2>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "ref", "pclk";
>> +				resets = <&sysrst K210_RST_I2C2>;
>> +				status = "disabled";
>> +			};
>> +
>> +			fpioa: pinmux@502B0000 {
>> +				compatible = "canaan,k210-fpioa";
>> +				reg = <0x502B0000 0x100>;
>> +				clocks = <&sysclk K210_CLK_FPIOA>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "ref", "pclk";
>> +				resets = <&sysrst K210_RST_FPIOA>;
>> +				canaan,k210-sysctl-power = <&sysctl 108>;
>> +				status = "disabled";
>> +			};
>> +
>> +			sha256: sha256@502C0000 {
>> +				compatible = "canaan,k210-sha256";
>> +				reg = <0x502C0000 0x100>;
>> +				clocks = <&sysclk K210_CLK_SHA>;
>> +				resets = <&sysrst K210_RST_SHA>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer0: timer@502D0000 {
>> +				compatible = "snps,dw-apb-timer";
>> +				reg = <0x502D0000 0x100>;
>> +				interrupts = <14 15>;
>> +				clocks = <&sysclk K210_CLK_TIMER0>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "timer", "pclk";
>> +				resets = <&sysrst K210_RST_TIMER0>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer1: timer@502E0000 {
>> +				compatible = "snps,dw-apb-timer";
>> +				reg = <0x502E0000 0x100>;
>> +				interrupts = <16 17>;
>> +				clocks = <&sysclk K210_CLK_TIMER1>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "timer", "pclk";
>> +				resets = <&sysrst K210_RST_TIMER1>;
>> +				status = "disabled";
>> +			};
>> +
>> +			timer2: timer@502F0000 {
>> +				compatible = "snps,dw-apb-timer";
>> +				reg = <0x502F0000 0x100>;
>> +				interrupts = <18 19>;
>> +				clocks = <&sysclk K210_CLK_TIMER2>,
>> +					 <&sysclk K210_CLK_APB0>;
>> +				clock-names = "timer", "pclk";
>> +				resets = <&sysrst K210_RST_TIMER2>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		apb1: bus@50400000 {
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			compatible = "simple-pm-bus";
>> +			ranges;
>> +			clocks = <&sysclk K210_CLK_APB1>;
>> +
>> +			wdt0: watchdog@50400000 {
>> +				compatible = "snps,dw-wdt";
>> +				reg = <0x50400000 0x100>;
>> +				interrupts = <21>;
>> +				clocks = <&sysclk K210_CLK_WDT0>,
>> +					 <&sysclk K210_CLK_APB1>;
>> +				clock-names = "tclk", "pclk";
>> +				resets = <&sysrst K210_RST_WDT0>;
>> +				status = "disabled";
>> +			};
>> +
>> +			wdt1: watchdog@50410000 {
>> +				compatible = "snps,dw-wdt";
>> +				reg = <0x50410000 0x100>;
>> +				interrupts = <22>;
>> +				clocks = <&sysclk K210_CLK_WDT1>,
>> +					 <&sysclk K210_CLK_APB1>;
>> +				clock-names = "tclk", "pclk";
>> +				resets = <&sysrst K210_RST_WDT1>;
>> +				status = "disabled";
>> +			};
>> +
>> +			otp0: nvmem@50420000 {
>> +				#address-cells = <1>;
>> +				#size-cells = <1>;
>> +				compatible = "canaan,k210-otp";
>> +				reg = <0x50420000 0x100>,
>> +				      <0x88000000 0x20000>;
>> +				reg-names = "reg", "mem";
>> +				clocks = <&sysclk K210_CLK_ROM>;
>> +				resets = <&sysrst K210_RST_ROM>;
>> +				read-only;
>> +				status = "disabled";
> 
> Your disabled nodes seem a bit excessive. A device should really only be
> disabled if it's a board level decision to use or not. I'd assume the
> OTP is always there and usable.

It's disabled because there is no driver for it (yet). Though see below
for the reasoning behind this.

>> +
>> +				/* Bootloader */
>> +				firmware@00000 {
> 
> Drop leading 0s.
> 
> Is this memory mapped? If so, you are missing 'ranges' in the parent to
> make it translateable.

Yes, there should be ranges.

Though I am not sure that the ROM is controllable from this OTP...

Perhaps it should be its own node.

> 
>> +					reg = <0x00000 0xC200>;
>> +				};
>> +
>> +				/*
>> +				 * config string as described in RISC-V
>> +				 * privileged spec 1.9
>> +				 */
>> +				config-1-9@1c000 {
>> +					reg = <0x1C000 0x1000>;
>> +				};
>> +
>> +				/*
>> +				 * Device tree containing only registers,
>> +				 * interrupts, and cpus
>> +				 */
>> +				fdt@1d000 {
>> +					reg = <0x1D000 0x2000>;
>> +				};
>> +
>> +				/* CPU/ROM credits */
>> +				credits@1f000 {
>> +					reg = <0x1F000 0x1000>;
>> +				};
>> +			};
>> +
>> +			dvp0: camera@50430000 {
>> +				compatible = "canaan,k210-dvp";
> 
> No documented. Seems to be several of them.

Correct. At some point there may be drivers. But there is no
authoritative information (memory map, list of registers, list of
interrupts) outside of source code for this board.

> 
>> +				reg = <0x50430000 0x100>;
>> +				interrupts = <24>;
>> +				clocks = <&sysclk K210_CLK_DVP>;
>> +				resets = <&sysrst K210_RST_DVP>;
>> +				canaan,k210-misc-offset = <&sysctl 84>;
>> +				status = "disabled";
>> +			};
>> +
>> +			sysctl: syscon@50440000 {
>> +				compatible = "canaan,k210-sysctl",
>> +					     "syscon", "simple-mfd";
>> +				reg = <0x50440000 0x100>;
>> +				clocks = <&sysclk K210_CLK_APB1>;
>> +				clock-names = "pclk";
>> +
>> +				sysclk: clock-controller {
>> +					#clock-cells = <1>;
>> +					compatible = "canaan,k210-clk";
>> +					clocks = <&in0>;
>> +				};
>> +
>> +				sysrst: reset-controller {
>> +					compatible = "canaan,k210-rst";
>> +					#reset-cells = <1>;
>> +				};
>> +
>> +				reboot: syscon-reboot {
>> +					compatible = "syscon-reboot";
>> +					regmap = <&sysctl>;
>> +					offset = <48>;
>> +					mask = <1>;
>> +					value = <1>;
>> +				};
>> +			};
>> +
>> +			aes0: aes@50450000 {
>> +				compatible = "canaan,k210-aes";
>> +				reg = <0x50450000 0x100>;
>> +				clocks = <&sysclk K210_CLK_AES>;
>> +				resets = <&sysrst K210_RST_AES>;
>> +				status = "disabled";
>> +			};
>> +
>> +			rtc: rtc@50460000 {
>> +				compatible = "canaan,k210-rtc";
>> +				reg = <0x50460000 0x100>;
>> +				clocks = <&in0>;
>> +				resets = <&sysrst K210_RST_RTC>;
>> +				interrupts = <20>;
>> +				status = "disabled";
>> +			};
>> +		};
>> +
>> +		apb2: bus@52000000 {
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			compatible = "simple-pm-bus";
>> +			ranges;
>> +			clocks = <&sysclk K210_CLK_APB2>;
>> +
>> +			spi0: spi@52000000 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				compatible = "canaan,k210-spi";
>> +				reg = <0x52000000 0x100>;
>> +				interrupts = <1>;
>> +				clocks = <&sysclk K210_CLK_SPI0>,
>> +					 <&sysclk K210_CLK_APB2>;
>> +				clock-names = "ssi_clk", "pclk";
>> +				resets = <&sysrst K210_RST_SPI0>;
>> +				reset-names = "spi";
>> +				spi-max-frequency = <25000000>;
>> +				num-cs = <4>;
>> +				reg-io-width = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			spi1: spi@53000000 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				compatible = "canaan,k210-spi";
>> +				reg = <0x53000000 0x100>;
>> +				interrupts = <2>;
>> +				clocks = <&sysclk K210_CLK_SPI1>,
>> +					 <&sysclk K210_CLK_APB2>;
>> +				clock-names = "ssi_clk", "pclk";
>> +				resets = <&sysrst K210_RST_SPI1>;
>> +				reset-names = "spi";
>> +				spi-max-frequency = <25000000>;
>> +				num-cs = <4>;
>> +				reg-io-width = <4>;
>> +				status = "disabled";
>> +			};
>> +
>> +			spi3: spi@54000000 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				compatible = "snps,dwc-ssi-1.01a";
>> +				reg = <0x54000000 0x200>;
>> +				interrupts = <4>;
>> +				clocks = <&sysclk K210_CLK_SPI3>,
>> +					 <&sysclk K210_CLK_APB2>;
>> +				clock-names = "ssi_clk", "pclk";
>> +				resets = <&sysrst K210_RST_SPI3>;
>> +				reset-names = "spi";
>> +				/* Could possibly go up to 200 MHz */
>> +				spi-max-frequency = <100000000>;
>> +				num-cs = <4>;
>> +				reg-io-width = <4>;
>> +				status = "disabled";
>> +			};
>>   		};
>>   	};
>>   };
>> diff --git a/arch/riscv/boot/dts/canaan/k210_generic.dts b/arch/riscv/boot/dts/canaan/k210_generic.dts
>> new file mode 100644
>> index 000000000000..396c8ca4d24d
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/canaan/k210_generic.dts
>> @@ -0,0 +1,46 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
>> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "k210.dtsi"
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +
>> +/ {
>> +	model = "Kendryte K210 generic";
>> +	compatible = "canaan,kendryte-k210";
>> +
>> +	chosen {
>> +		bootargs = "earlycon console=ttySIF0";
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +};
>> +
>> +&fpioa {
>> +	pinctrl-0 = <&jtag_pins>;
>> +	pinctrl-names = "default";
>> +	status = "okay";
>> +
>> +	jtag_pins: jtag-pinmux {
>> +		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
>> +			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
>> +			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
>> +			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
>> +	};
>> +
>> +	uarths_pins: uarths-pinmux {
>> +		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
>> +			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
>> +	};
>> +};
>> +
>> +&uarths0 {
>> +	pinctrl-0 = <&uarths_pins>;
>> +	pinctrl-names = "default";
>> +	status = "okay";
>> +};
>> diff --git a/include/dt-bindings/clock/k210-clk.h b/include/dt-bindings/clock/k210-clk.h
>> index a48176ad3c23..b2de702cbf75 100644
>> --- a/include/dt-bindings/clock/k210-clk.h
>> +++ b/include/dt-bindings/clock/k210-clk.h
>> @@ -9,7 +9,6 @@
>>   /*
>>    * Kendryte K210 SoC clock identifiers (arbitrary values).
>>    */
>> -#define K210_CLK_ACLK	0
>>   #define K210_CLK_CPU	0
>>   #define K210_CLK_SRAM0	1
>>   #define K210_CLK_SRAM1	2
>> -- 
>> 2.29.2
>>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-05 22:52     ` Sean Anderson
@ 2021-02-05 23:49       ` Damien Le Moal
  2021-02-08 19:49       ` Rob Herring
  1 sibling, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-05 23:49 UTC (permalink / raw)
  To: Sean Anderson, Rob Herring
  Cc: Palmer Dabbelt, linux-riscv, Atish Patra, Anup Patel, devicetree

On 2021/02/06 7:52, Sean Anderson wrote:
> On 2/5/21 3:25 PM, Rob Herring wrote:
>> On Fri, Feb 05, 2021 at 03:58:20PM +0900, Damien Le Moal wrote:
>>> Update the Canaan Kendryte K210 base device tree k210.dtsi to define
>>> all peripherals of the SoC, their clocks and reset lines. The device
>>> tree file k210.dts is renamed to k210_generic.dts and becomes the
>>> default value selection of the SOC_CANAAN_K210_DTB_BUILTIN_SOURCE
>>> configuration option. No device beside the serial console is defined by
>>> this device tree. This makes this generic device tree suitable for use
>>> with a builtin initramfs with all known K210 based boards.
>>>
>>> These changes result in the K210_CLK_ACLK clock ID to be unused and
>>> removed from the dt-bindings k210-clk.h header file.
>>>
>>> Most updates to the k210.dtsi file come from Sean Anderson's work on
>>> U-Boot support for the K210.
>>>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>>> ---
>>>   arch/riscv/Kconfig.socs                     |   2 +-
>>>   arch/riscv/boot/dts/canaan/k210.dts         |  23 -
>>>   arch/riscv/boot/dts/canaan/k210.dtsi        | 535 +++++++++++++++++++-
>>>   arch/riscv/boot/dts/canaan/k210_generic.dts |  46 ++
>>>   include/dt-bindings/clock/k210-clk.h        |   1 -
>>>   5 files changed, 554 insertions(+), 53 deletions(-)
>>>   delete mode 100644 arch/riscv/boot/dts/canaan/k210.dts
>>>   create mode 100644 arch/riscv/boot/dts/canaan/k210_generic.dts
>>>
>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>> index 6402746c68f3..7efcece8896c 100644
>>> --- a/arch/riscv/Kconfig.socs
>>> +++ b/arch/riscv/Kconfig.socs
>>> @@ -51,7 +51,7 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>   	string "Source file for the Canaan Kendryte K210 builtin DTB"
>>>   	depends on SOC_CANAAN
>>>   	depends on SOC_CANAAN_K210_DTB_BUILTIN
>>> -	default "k210"
>>> +	default "k210_generic"
>>>   	help
>>>   	  Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
>>>   	  for the DTS file that will be used to produce the DTB linked into the
>>> diff --git a/arch/riscv/boot/dts/canaan/k210.dts b/arch/riscv/boot/dts/canaan/k210.dts
>>> deleted file mode 100644
>>> index 0d1f28fce6b2..000000000000
>>> --- a/arch/riscv/boot/dts/canaan/k210.dts
>>> +++ /dev/null
>>> @@ -1,23 +0,0 @@
>>> -// SPDX-License-Identifier: GPL-2.0+
>>> -/*
>>> - * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>>> - */
>>> -
>>> -/dts-v1/;
>>> -
>>> -#include "k210.dtsi"
>>> -
>>> -/ {
>>> -	model = "Kendryte K210 generic";
>>> -	compatible = "kendryte,k210";
>>> -
>>> -	chosen {
>>> -		bootargs = "earlycon console=ttySIF0";
>>> -		stdout-path = "serial0";
>>> -	};
>>> -};
>>> -
>>> -&uarths0 {
>>> -	status = "okay";
>>> -};
>>> -
>>> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
>>> index 354b263195a3..63c1f4c98d6c 100644
>>> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
>>> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
>>> @@ -1,9 +1,11 @@
>>>   // SPDX-License-Identifier: GPL-2.0+
>>>   /*
>>> - * Copyright (C) 2019 Sean Anderson <seanga2@gmail.com>
>>> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
>>>    * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>>>    */
>>>   #include <dt-bindings/clock/k210-clk.h>
>>> +#include <dt-bindings/pinctrl/k210-fpioa.h>
>>> +#include <dt-bindings/reset/k210-rst.h>
>>>   
>>>   / {
>>>   	/*
>>> @@ -12,14 +14,33 @@ / {
>>>   	 */
>>>   	#address-cells = <1>;
>>>   	#size-cells = <1>;
>>> -	compatible = "kendryte,k210";
>>> +	compatible = "canaan,kendryte-k210";
>>>   
>>>   	aliases {
>>> +		cpu0 = &cpu0;
>>> +		cpu1 = &cpu1;
>>> +		dma0 = &dmac0;
>>> +		gpio0 = &gpio0;
>>> +		gpio1 = &gpio1_0;
>>> +		i2c0 = &i2c0;
>>> +		i2c1 = &i2c1;
>>> +		i2c2 = &i2c2;
>>> +		pinctrl0 = &fpioa;
>>>   		serial0 = &uarths0;
>>> +		serial1 = &uart1;
>>> +		serial2 = &uart2;
>>> +		serial3 = &uart3;
>>> +		spi0 = &spi0;
>>> +		spi1 = &spi1;
>>> +		spi2 = &spi2;
>>> +		spi3 = &spi3;
>>> +		timer0 = &timer0;
>>> +		timer1 = &timer1;
>>> +		timer2 = &timer2;
>>
>> Don't add random aliases. Really, only the serialN ones should be here.
>> cpu, dma, pinctrl, timer are definitely non-standard.
> 
> 
> All of these except for cpu and dma are already found in the kernel.
> They were primarily added for U-Boot.

They are unused in the DTS. make dtbs_check passes keeping only the serial
aliases, just check. Can I remove them ? Will that be a problem for U-Boot ?

> 
>>
>>>   	};
>>>   
>>>   	/*
>>> -	 * The K210 has an sv39 MMU following the priviledge specification v1.9.
>>> +	 * The K210 has an sv39 MMU following the privileged specification v1.9.
>>>   	 * Since this is a non-ratified draft specification, the kernel does not
>>>   	 * support it and the K210 support enabled only for the !MMU case.
>>>   	 * Be consistent with this by setting the CPUs MMU type to "none".
>>> @@ -30,14 +51,14 @@ cpus {
>>>   		timebase-frequency = <7800000>;
>>>   		cpu0: cpu@0 {
>>>   			device_type = "cpu";
>>> +			compatible = "canaan,k210", "riscv";
>>>   			reg = <0>;
>>> -			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>>>   			riscv,isa = "rv64imafdc";
>>> -			mmu-type = "none";
>>> -			i-cache-size = <0x8000>;
>>> +			mmu-type = "riscv,none";
>>>   			i-cache-block-size = <64>;
>>> -			d-cache-size = <0x8000>;
>>> +			i-cache-size = <0x8000>;
>>>   			d-cache-block-size = <64>;
>>> +			d-cache-size = <0x8000>;
>>>   			cpu0_intc: interrupt-controller {
>>>   				#interrupt-cells = <1>;
>>>   				interrupt-controller;
>>> @@ -46,14 +67,14 @@ cpu0_intc: interrupt-controller {
>>>   		};
>>>   		cpu1: cpu@1 {
>>>   			device_type = "cpu";
>>> +			compatible = "canaan,k210", "riscv";
>>>   			reg = <1>;
>>> -			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>>>   			riscv,isa = "rv64imafdc";
>>> -			mmu-type = "none";
>>> -			i-cache-size = <0x8000>;
>>> +			mmu-type = "riscv,none";
>>>   			i-cache-block-size = <64>;
>>> -			d-cache-size = <0x8000>;
>>> +			i-cache-size = <0x8000>;
>>>   			d-cache-block-size = <64>;
>>> +			d-cache-size = <0x8000>;
>>>   			cpu1_intc: interrupt-controller {
>>>   				#interrupt-cells = <1>;
>>>   				interrupt-controller;
>>> @@ -64,10 +85,15 @@ cpu1_intc: interrupt-controller {
>>>   
>>>   	sram: memory@80000000 {
>>>   		device_type = "memory";
>>> +		compatible = "canaan,k210-sram";
>>>   		reg = <0x80000000 0x400000>,
>>>   		      <0x80400000 0x200000>,
>>>   		      <0x80600000 0x200000>;
>>>   		reg-names = "sram0", "sram1", "aisram";
>>> +		clocks = <&sysclk K210_CLK_SRAM0>,
>>> +			 <&sysclk K210_CLK_SRAM1>,
>>> +			 <&sysclk K210_CLK_AI>;
>>> +		clock-names = "sram0", "sram1", "aisram";
>>>   	};
>>>   
>>>   	clocks {
>>> @@ -81,40 +107,493 @@ in0: oscillator {
>>>   	soc {
>>>   		#address-cells = <1>;
>>>   		#size-cells = <1>;
>>> -		compatible = "kendryte,k210-soc", "simple-bus";
>>> +		compatible = "simple-bus";
>>>   		ranges;
>>>   		interrupt-parent = <&plic0>;
>>>   
>>> -		sysctl: sysctl@50440000 {
>>> -			compatible = "kendryte,k210-sysctl", "simple-mfd";
>>> -			reg = <0x50440000 0x1000>;
>>> -			#clock-cells = <1>;
>>> +		debug0: debug@0 {
>>> +			compatible = "canaan,k210-debug", "riscv,debug";
>>
>> Not documented.
> 
> On 1/14/21 7:06 PM, Sean Anderson wrote:
>> Here it's because "riscv,debug" doesn't exist. This is the "debug"
>> device as described in the debug spec. AFAIK Linux never needs to
>> configure this device. It could probably be removed. 
> 
> No objections here.

OK. Will drop this one.

> 
>>
>>> +			reg = <0x0 0x1000>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		rom0: nvmem@1000 {
>>> +			reg = <0x1000 0x1000>;
>>> +			read-only;
>>> +			status = "disabled";
>>>   		};
>>>   
>>>   		clint0: clint@2000000 {
>>
>> interrupt-controller@...
> 
> Yes, this should be changed.

The bindings doc say it should be timer@... Will change to that.

> 
>>
>>> -			#interrupt-cells = <1>;
>>> -			compatible = "riscv,clint0";
>>> +			compatible = "canaan,k210-clint", "sifive,clint0";
>>>   			reg = <0x2000000 0xC000>;
>>> -			interrupts-extended =  <&cpu0_intc 3 &cpu0_intc 7
>>> -						&cpu1_intc 3 &cpu1_intc 7>;
>>> +			interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>>> +					      &cpu1_intc 3 &cpu1_intc 7>;
>>>   		};
>>>   
>>> -		plic0: interrupt-controller@c000000 {
>>> +		plic0: interrupt-controller@C000000 {
>>
>> No, lowercase hex in unit-addresses was correct.
> 
> Do you have any authoritative source for this? When I was creating this
> tree, I didn't see anything about what case the addresses should be in
> (and a quick grep for lower-case in Documentation/devicetree doesn't
> have any relevant results).
> 
>>
>>>   			#interrupt-cells = <1>;
>>> -			interrupt-controller;
>>> -			compatible = "kendryte,k210-plic0", "riscv,plic0";
>>> +			#address-cells = <0>;
>>> +			compatible = "canaan,k210-plic", "sifive,plic-1.0.0";
>>>   			reg = <0xC000000 0x4000000>;
>>> -			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 0xffffffff>,
>>> -					      <&cpu1_intc 11>, <&cpu1_intc 0xffffffff>;
>>> +			interrupt-controller;
>>> +			interrupts-extended = <&cpu0_intc 11 &cpu1_intc 11>;
>>>   			riscv,ndev = <65>;
>>> -			riscv,max-priority = <7>;
>>>   		};
>>>   
>>>   		uarths0: serial@38000000 {
>>> -			compatible = "kendryte,k210-uarths", "sifive,uart0";
>>> +			compatible = "canaan,k210-uarths", "sifive,uart0";
>>>   			reg = <0x38000000 0x1000>;
>>>   			interrupts = <33>;
>>> -			clocks = <&sysctl K210_CLK_CPU>;
>>> +			clocks = <&sysclk K210_CLK_CPU>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		gpio0: gpio-controller@38001000 {
>>> +			#interrupt-cells = <2>;
>>> +			#gpio-cells = <2>;
>>> +			compatible = "canaan,k210-gpiohs", "sifive,gpio0";
>>> +			reg = <0x38001000 0x1000>;
>>> +			interrupt-controller;
>>> +			interrupts = <34 35 36 37 38 39 40 41
>>> +				      42 43 44 45 46 47 48 49
>>> +				      50 51 52 53 54 55 56 57
>>> +				      58 59 60 61 62 63 64 65>;
>>> +			gpio-controller;
>>> +			ngpios = <32>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		kpu0: kpu@40800000 {
>>> +			compatible = "canaan,k210-kpu";
>>> +			reg = <0x40800000 0xc00000>;
>>> +			interrupts = <25>;
>>> +			clocks = <&sysclk K210_CLK_AI>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		fft0: fft@42000000 {
>>> +			compatible = "canaan,k210-fft";
>>> +			reg = <0x42000000 0x400000>;
>>> +			interrupts = <26>;
>>> +			clocks = <&sysclk K210_CLK_FFT>;
>>> +			resets = <&sysrst K210_RST_FFT>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		dmac0: dma-controller@50000000 {
>>> +			compatible = "snps,axi-dma-1.01a";
>>> +			reg = <0x50000000 0x1000>;
>>> +			interrupts = <27 28 29 30 31 32>;
>>> +			clocks = <&sysclk K210_CLK_DMA>, <&sysclk K210_CLK_DMA>;
>>> +			clock-names = "core-clk", "cfgr-clk";
>>> +			resets = <&sysrst K210_RST_DMA>;
>>> +			dma-channels = <6>;
>>> +			snps,dma-masters = <2>;
>>> +			snps,priority = <0 1 2 3 4 5>;
>>> +			snps,data-width = <5>;
>>> +			snps,block-size = <0x200000 0x200000 0x200000
>>> +					   0x200000 0x200000 0x200000>;
>>> +			snps,axi-max-burst-len = <256>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		apb0: bus@50200000 {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			compatible = "simple-pm-bus";
>>> +			ranges;
>>> +			clocks = <&sysclk K210_CLK_APB0>;
>>> +
>>> +			gpio1: gpio@50200000 {
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>>> +				compatible = "snps,dw-apb-gpio";
>>> +				reg = <0x50200000 0x80>;
>>> +				clocks = <&sysclk K210_CLK_APB0>,
>>> +					 <&sysclk K210_CLK_GPIO>;
>>> +				clock-names = "bus", "db";
>>> +				resets = <&sysrst K210_RST_GPIO>;
>>> +				status = "disabled";
>>> +
>>> +				gpio1_0: gpio-port@0 {
>>> +					#gpio-cells = <2>;
>>> +					#interrupt-cells = <2>;
>>> +					compatible = "snps,dw-apb-gpio-port";
>>> +					reg = <0>;
>>> +					interrupt-controller;
>>> +					interrupts = <23>;
>>> +					gpio-controller;
>>> +					ngpios = <8>;
>>> +				};
>>> +			};
>>> +
>>> +			uart1: serial@50210000 {
>>> +				compatible = "snps,dw-apb-uart";
>>> +				reg = <0x50210000 0x100>;
>>> +				interrupts = <11>;
>>> +				clocks = <&sysclk K210_CLK_UART1>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "baudclk", "apb_pclk";
>>> +				resets = <&sysrst K210_RST_UART1>;
>>> +				reg-io-width = <4>;
>>> +				reg-shift = <2>;
>>> +				dcd-override;
>>> +				dsr-override;
>>> +				cts-override;
>>> +				ri-override;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			uart2: serial@50220000 {
>>> +				compatible = "snps,dw-apb-uart";
>>> +				reg = <0x50220000 0x100>;
>>> +				interrupts = <12>;
>>> +				clocks = <&sysclk K210_CLK_UART2>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "baudclk", "apb_pclk";
>>> +				resets = <&sysrst K210_RST_UART2>;
>>> +				reg-io-width = <4>;
>>> +				reg-shift = <2>;
>>> +				dcd-override;
>>> +				dsr-override;
>>> +				cts-override;
>>> +				ri-override;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			uart3: serial@50230000 {
>>> +				compatible = "snps,dw-apb-uart";
>>> +				reg = <0x50230000 0x100>;
>>> +				interrupts = <13>;
>>> +				clocks = <&sysclk K210_CLK_UART3>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "baudclk", "apb_pclk";
>>> +				resets = <&sysrst K210_RST_UART3>;
>>> +				reg-io-width = <4>;
>>> +				reg-shift = <2>;
>>> +				dcd-override;
>>> +				dsr-override;
>>> +				cts-override;
>>> +				ri-override;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			spi2: spi@50240000 {
>>> +				compatible = "canaan,k210-spi";
>>> +				spi-slave;
>>> +				reg = <0x50240000 0x100>;
>>> +				interrupts = <3>;
>>> +				clocks = <&sysclk K210_CLK_SPI2>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "ssi_clk", "pclk";
>>> +				resets = <&sysrst K210_RST_SPI2>;
>>> +				spi-max-frequency = <25000000>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			i2s0: i2s@50250000 {
>>> +				compatible = "snps,designware-i2s";
>>> +				reg = <0x50250000 0x200>;
>>> +				interrupts = <5>;
>>> +				clocks = <&sysclk K210_CLK_I2S0>;
>>> +				clock-names = "i2sclk";
>>> +				resets = <&sysrst K210_RST_I2S0>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			apu0: sound@520250200 {
>>> +				compatible = "canaan,k210-apu";
>>> +				reg = <0x50250200 0x200>;
>>
>> The unit-address and 'reg' value don't match.
> 
> Good catch. The unit address should be 50250200.

Fixed.

> 
>>
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			i2s1: i2s@50260000 {
>>> +				compatible = "snps,designware-i2s";
>>> +				reg = <0x50260000 0x200>;
>>> +				interrupts = <6>;
>>> +				clocks = <&sysclk K210_CLK_I2S1>;
>>> +				clock-names = "i2sclk";
>>> +				resets = <&sysrst K210_RST_I2S1>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			i2s2: i2s@50270000 {
>>> +				compatible = "snps,designware-i2s";
>>> +				reg = <0x50270000 0x200>;
>>> +				interrupts = <7>;
>>> +				clocks = <&sysclk K210_CLK_I2S2>;
>>> +				clock-names = "i2sclk";
>>> +				resets = <&sysrst K210_RST_I2S2>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			i2c0: i2c@50280000 {
>>> +				compatible = "snps,designware-i2c";
>>> +				reg = <0x50280000 0x100>;
>>> +				interrupts = <8>;
>>> +				clocks = <&sysclk K210_CLK_I2C0>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "ref", "pclk";
>>> +				resets = <&sysrst K210_RST_I2C0>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			i2c1: i2c@50290000 {
>>> +				compatible = "snps,designware-i2c";
>>> +				reg = <0x50290000 0x100>;
>>> +				interrupts = <9>;
>>> +				clocks = <&sysclk K210_CLK_I2C1>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "ref", "pclk";
>>> +				resets = <&sysrst K210_RST_I2C1>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			i2c2: i2c@502A0000 {
>>
>> Again, lowercase hex.
>>
>>> +				compatible = "snps,designware-i2c";
>>> +				reg = <0x502A0000 0x100>;
>>> +				interrupts = <10>;
>>> +				clocks = <&sysclk K210_CLK_I2C2>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "ref", "pclk";
>>> +				resets = <&sysrst K210_RST_I2C2>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			fpioa: pinmux@502B0000 {
>>> +				compatible = "canaan,k210-fpioa";
>>> +				reg = <0x502B0000 0x100>;
>>> +				clocks = <&sysclk K210_CLK_FPIOA>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "ref", "pclk";
>>> +				resets = <&sysrst K210_RST_FPIOA>;
>>> +				canaan,k210-sysctl-power = <&sysctl 108>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			sha256: sha256@502C0000 {
>>> +				compatible = "canaan,k210-sha256";
>>> +				reg = <0x502C0000 0x100>;
>>> +				clocks = <&sysclk K210_CLK_SHA>;
>>> +				resets = <&sysrst K210_RST_SHA>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			timer0: timer@502D0000 {
>>> +				compatible = "snps,dw-apb-timer";
>>> +				reg = <0x502D0000 0x100>;
>>> +				interrupts = <14 15>;
>>> +				clocks = <&sysclk K210_CLK_TIMER0>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "timer", "pclk";
>>> +				resets = <&sysrst K210_RST_TIMER0>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			timer1: timer@502E0000 {
>>> +				compatible = "snps,dw-apb-timer";
>>> +				reg = <0x502E0000 0x100>;
>>> +				interrupts = <16 17>;
>>> +				clocks = <&sysclk K210_CLK_TIMER1>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "timer", "pclk";
>>> +				resets = <&sysrst K210_RST_TIMER1>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			timer2: timer@502F0000 {
>>> +				compatible = "snps,dw-apb-timer";
>>> +				reg = <0x502F0000 0x100>;
>>> +				interrupts = <18 19>;
>>> +				clocks = <&sysclk K210_CLK_TIMER2>,
>>> +					 <&sysclk K210_CLK_APB0>;
>>> +				clock-names = "timer", "pclk";
>>> +				resets = <&sysrst K210_RST_TIMER2>;
>>> +				status = "disabled";
>>> +			};
>>> +		};
>>> +
>>> +		apb1: bus@50400000 {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			compatible = "simple-pm-bus";
>>> +			ranges;
>>> +			clocks = <&sysclk K210_CLK_APB1>;
>>> +
>>> +			wdt0: watchdog@50400000 {
>>> +				compatible = "snps,dw-wdt";
>>> +				reg = <0x50400000 0x100>;
>>> +				interrupts = <21>;
>>> +				clocks = <&sysclk K210_CLK_WDT0>,
>>> +					 <&sysclk K210_CLK_APB1>;
>>> +				clock-names = "tclk", "pclk";
>>> +				resets = <&sysrst K210_RST_WDT0>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			wdt1: watchdog@50410000 {
>>> +				compatible = "snps,dw-wdt";
>>> +				reg = <0x50410000 0x100>;
>>> +				interrupts = <22>;
>>> +				clocks = <&sysclk K210_CLK_WDT1>,
>>> +					 <&sysclk K210_CLK_APB1>;
>>> +				clock-names = "tclk", "pclk";
>>> +				resets = <&sysrst K210_RST_WDT1>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			otp0: nvmem@50420000 {
>>> +				#address-cells = <1>;
>>> +				#size-cells = <1>;
>>> +				compatible = "canaan,k210-otp";
>>> +				reg = <0x50420000 0x100>,
>>> +				      <0x88000000 0x20000>;
>>> +				reg-names = "reg", "mem";
>>> +				clocks = <&sysclk K210_CLK_ROM>;
>>> +				resets = <&sysrst K210_RST_ROM>;
>>> +				read-only;
>>> +				status = "disabled";
>>
>> Your disabled nodes seem a bit excessive. A device should really only be
>> disabled if it's a board level decision to use or not. I'd assume the
>> OTP is always there and usable.
> 
> It's disabled because there is no driver for it (yet). Though see below
> for the reasoning behind this.
> 
>>> +
>>> +				/* Bootloader */
>>> +				firmware@00000 {
>>
>> Drop leading 0s.
>>
>> Is this memory mapped? If so, you are missing 'ranges' in the parent to
>> make it translateable.
> 
> Yes, there should be ranges.
> 
> Though I am not sure that the ROM is controllable from this OTP...
> 
> Perhaps it should be its own node.

Not sures what to do here... I added "ranges;" to the node. Is that enough ? Or
should the range for each child be defined ?

> 
>>
>>> +					reg = <0x00000 0xC200>;
>>> +				};
>>> +
>>> +				/*
>>> +				 * config string as described in RISC-V
>>> +				 * privileged spec 1.9
>>> +				 */
>>> +				config-1-9@1c000 {
>>> +					reg = <0x1C000 0x1000>;
>>> +				};
>>> +
>>> +				/*
>>> +				 * Device tree containing only registers,
>>> +				 * interrupts, and cpus
>>> +				 */
>>> +				fdt@1d000 {
>>> +					reg = <0x1D000 0x2000>;
>>> +				};
>>> +
>>> +				/* CPU/ROM credits */
>>> +				credits@1f000 {
>>> +					reg = <0x1F000 0x1000>;
>>> +				};
>>> +			};
>>> +
>>> +			dvp0: camera@50430000 {
>>> +				compatible = "canaan,k210-dvp";
>>
>> No documented. Seems to be several of them.
> 
> Correct. At some point there may be drivers. But there is no
> authoritative information (memory map, list of registers, list of
> interrupts) outside of source code for this board.
> 
>>
>>> +				reg = <0x50430000 0x100>;
>>> +				interrupts = <24>;
>>> +				clocks = <&sysclk K210_CLK_DVP>;
>>> +				resets = <&sysrst K210_RST_DVP>;
>>> +				canaan,k210-misc-offset = <&sysctl 84>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			sysctl: syscon@50440000 {
>>> +				compatible = "canaan,k210-sysctl",
>>> +					     "syscon", "simple-mfd";
>>> +				reg = <0x50440000 0x100>;
>>> +				clocks = <&sysclk K210_CLK_APB1>;
>>> +				clock-names = "pclk";
>>> +
>>> +				sysclk: clock-controller {
>>> +					#clock-cells = <1>;
>>> +					compatible = "canaan,k210-clk";
>>> +					clocks = <&in0>;
>>> +				};
>>> +
>>> +				sysrst: reset-controller {
>>> +					compatible = "canaan,k210-rst";
>>> +					#reset-cells = <1>;
>>> +				};
>>> +
>>> +				reboot: syscon-reboot {
>>> +					compatible = "syscon-reboot";
>>> +					regmap = <&sysctl>;
>>> +					offset = <48>;
>>> +					mask = <1>;
>>> +					value = <1>;
>>> +				};
>>> +			};
>>> +
>>> +			aes0: aes@50450000 {
>>> +				compatible = "canaan,k210-aes";
>>> +				reg = <0x50450000 0x100>;
>>> +				clocks = <&sysclk K210_CLK_AES>;
>>> +				resets = <&sysrst K210_RST_AES>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			rtc: rtc@50460000 {
>>> +				compatible = "canaan,k210-rtc";
>>> +				reg = <0x50460000 0x100>;
>>> +				clocks = <&in0>;
>>> +				resets = <&sysrst K210_RST_RTC>;
>>> +				interrupts = <20>;
>>> +				status = "disabled";
>>> +			};
>>> +		};
>>> +
>>> +		apb2: bus@52000000 {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			compatible = "simple-pm-bus";
>>> +			ranges;
>>> +			clocks = <&sysclk K210_CLK_APB2>;
>>> +
>>> +			spi0: spi@52000000 {
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>>> +				compatible = "canaan,k210-spi";
>>> +				reg = <0x52000000 0x100>;
>>> +				interrupts = <1>;
>>> +				clocks = <&sysclk K210_CLK_SPI0>,
>>> +					 <&sysclk K210_CLK_APB2>;
>>> +				clock-names = "ssi_clk", "pclk";
>>> +				resets = <&sysrst K210_RST_SPI0>;
>>> +				reset-names = "spi";
>>> +				spi-max-frequency = <25000000>;
>>> +				num-cs = <4>;
>>> +				reg-io-width = <4>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			spi1: spi@53000000 {
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>>> +				compatible = "canaan,k210-spi";
>>> +				reg = <0x53000000 0x100>;
>>> +				interrupts = <2>;
>>> +				clocks = <&sysclk K210_CLK_SPI1>,
>>> +					 <&sysclk K210_CLK_APB2>;
>>> +				clock-names = "ssi_clk", "pclk";
>>> +				resets = <&sysrst K210_RST_SPI1>;
>>> +				reset-names = "spi";
>>> +				spi-max-frequency = <25000000>;
>>> +				num-cs = <4>;
>>> +				reg-io-width = <4>;
>>> +				status = "disabled";
>>> +			};
>>> +
>>> +			spi3: spi@54000000 {
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>>> +				compatible = "snps,dwc-ssi-1.01a";
>>> +				reg = <0x54000000 0x200>;
>>> +				interrupts = <4>;
>>> +				clocks = <&sysclk K210_CLK_SPI3>,
>>> +					 <&sysclk K210_CLK_APB2>;
>>> +				clock-names = "ssi_clk", "pclk";
>>> +				resets = <&sysrst K210_RST_SPI3>;
>>> +				reset-names = "spi";
>>> +				/* Could possibly go up to 200 MHz */
>>> +				spi-max-frequency = <100000000>;
>>> +				num-cs = <4>;
>>> +				reg-io-width = <4>;
>>> +				status = "disabled";
>>> +			};
>>>   		};
>>>   	};
>>>   };
>>> diff --git a/arch/riscv/boot/dts/canaan/k210_generic.dts b/arch/riscv/boot/dts/canaan/k210_generic.dts
>>> new file mode 100644
>>> index 000000000000..396c8ca4d24d
>>> --- /dev/null
>>> +++ b/arch/riscv/boot/dts/canaan/k210_generic.dts
>>> @@ -0,0 +1,46 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
>>> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "k210.dtsi"
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/input/input.h>
>>> +
>>> +/ {
>>> +	model = "Kendryte K210 generic";
>>> +	compatible = "canaan,kendryte-k210";
>>> +
>>> +	chosen {
>>> +		bootargs = "earlycon console=ttySIF0";
>>> +		stdout-path = "serial0:115200n8";
>>> +	};
>>> +};
>>> +
>>> +&fpioa {
>>> +	pinctrl-0 = <&jtag_pins>;
>>> +	pinctrl-names = "default";
>>> +	status = "okay";
>>> +
>>> +	jtag_pins: jtag-pinmux {
>>> +		pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
>>> +			 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
>>> +			 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
>>> +			 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
>>> +	};
>>> +
>>> +	uarths_pins: uarths-pinmux {
>>> +		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
>>> +			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
>>> +	};
>>> +};
>>> +
>>> +&uarths0 {
>>> +	pinctrl-0 = <&uarths_pins>;
>>> +	pinctrl-names = "default";
>>> +	status = "okay";
>>> +};
>>> diff --git a/include/dt-bindings/clock/k210-clk.h b/include/dt-bindings/clock/k210-clk.h
>>> index a48176ad3c23..b2de702cbf75 100644
>>> --- a/include/dt-bindings/clock/k210-clk.h
>>> +++ b/include/dt-bindings/clock/k210-clk.h
>>> @@ -9,7 +9,6 @@
>>>   /*
>>>    * Kendryte K210 SoC clock identifiers (arbitrary values).
>>>    */
>>> -#define K210_CLK_ACLK	0
>>>   #define K210_CLK_CPU	0
>>>   #define K210_CLK_SRAM0	1
>>>   #define K210_CLK_SRAM1	2
>>> -- 
>>> 2.29.2
>>>
> 
> 


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-05 20:25   ` Rob Herring
  2021-02-05 22:52     ` Sean Anderson
@ 2021-02-06  0:13     ` Damien Le Moal
  2021-02-08 20:00       ` Rob Herring
  1 sibling, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2021-02-06  0:13 UTC (permalink / raw)
  To: robh; +Cc: palmer, linux-riscv, Atish Patra, seanga2, Anup Patel, devicetree

On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
[...]
> > +			otp0: nvmem@50420000 {
> > +				#address-cells = <1>;
> > +				#size-cells = <1>;
> > +				compatible = "canaan,k210-otp";
> > +				reg = <0x50420000 0x100>,
> > +				      <0x88000000 0x20000>;
> > +				reg-names = "reg", "mem";
> > +				clocks = <&sysclk K210_CLK_ROM>;
> > +				resets = <&sysrst K210_RST_ROM>;
> > +				read-only;
> > +				status = "disabled";
> 
> Your disabled nodes seem a bit excessive. A device should really only be 
> disabled if it's a board level decision to use or not. I'd assume the 
> OTP is always there and usable.

Please see below.

> 
> > +
> > +				/* Bootloader */
> > +				firmware@00000 {
> 
> Drop leading 0s.
> 
> Is this memory mapped? If so, you are missing 'ranges' in the parent to 
> make it translateable.
> 
> > +					reg = <0x00000 0xC200>;
> > +				};
> > +
> > +				/*
> > +				 * config string as described in RISC-V
> > +				 * privileged spec 1.9
> > +				 */
> > +				config-1-9@1c000 {
> > +					reg = <0x1C000 0x1000>;
> > +				};
> > +
> > +				/*
> > +				 * Device tree containing only registers,
> > +				 * interrupts, and cpus
> > +				 */
> > +				fdt@1d000 {
> > +					reg = <0x1D000 0x2000>;
> > +				};
> > +
> > +				/* CPU/ROM credits */
> > +				credits@1f000 {
> > +					reg = <0x1F000 0x1000>;
> > +				};
> > +			};
> > +
> > +			dvp0: camera@50430000 {
> > +				compatible = "canaan,k210-dvp";
> 
> No documented. Seems to be several of them.

There are no Linux drivers for these undocumented nodes. That is why I did not
add any documentation. make dtbs_check does not complain about that as long as
the nodes are marked disabled. I kept these nodes to have the DTS in sync with
U-Boot which has them. Keeping them also creates documentation for the SoC
since this device tree is more detailed than the SoC specsheet...

I removed "status = disabled;" from all nodes that have a Linux driver and kept
it for all nodes that don't have one.



-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-05 22:52     ` Sean Anderson
  2021-02-05 23:49       ` Damien Le Moal
@ 2021-02-08 19:49       ` Rob Herring
  2021-02-08 22:41         ` Damien Le Moal
  1 sibling, 1 reply; 27+ messages in thread
From: Rob Herring @ 2021-02-08 19:49 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Damien Le Moal, Palmer Dabbelt, linux-riscv, Atish Patra,
	Anup Patel, devicetree

On Fri, Feb 5, 2021 at 4:52 PM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 2/5/21 3:25 PM, Rob Herring wrote:
> > On Fri, Feb 05, 2021 at 03:58:20PM +0900, Damien Le Moal wrote:
> >> Update the Canaan Kendryte K210 base device tree k210.dtsi to define
> >> all peripherals of the SoC, their clocks and reset lines. The device
> >> tree file k210.dts is renamed to k210_generic.dts and becomes the
> >> default value selection of the SOC_CANAAN_K210_DTB_BUILTIN_SOURCE
> >> configuration option. No device beside the serial console is defined by
> >> this device tree. This makes this generic device tree suitable for use
> >> with a builtin initramfs with all known K210 based boards.
> >>
> >> These changes result in the K210_CLK_ACLK clock ID to be unused and
> >> removed from the dt-bindings k210-clk.h header file.
> >>
> >> Most updates to the k210.dtsi file come from Sean Anderson's work on
> >> U-Boot support for the K210.
> >>
> >> Cc: Rob Herring <robh@kernel.org>
> >> Cc: devicetree@vger.kernel.org
> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> >> ---
> >>   arch/riscv/Kconfig.socs                     |   2 +-
> >>   arch/riscv/boot/dts/canaan/k210.dts         |  23 -
> >>   arch/riscv/boot/dts/canaan/k210.dtsi        | 535 +++++++++++++++++++-
> >>   arch/riscv/boot/dts/canaan/k210_generic.dts |  46 ++
> >>   include/dt-bindings/clock/k210-clk.h        |   1 -
> >>   5 files changed, 554 insertions(+), 53 deletions(-)
> >>   delete mode 100644 arch/riscv/boot/dts/canaan/k210.dts
> >>   create mode 100644 arch/riscv/boot/dts/canaan/k210_generic.dts
> >>
> >> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> >> index 6402746c68f3..7efcece8896c 100644
> >> --- a/arch/riscv/Kconfig.socs
> >> +++ b/arch/riscv/Kconfig.socs
> >> @@ -51,7 +51,7 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >>      string "Source file for the Canaan Kendryte K210 builtin DTB"
> >>      depends on SOC_CANAAN
> >>      depends on SOC_CANAAN_K210_DTB_BUILTIN
> >> -    default "k210"
> >> +    default "k210_generic"
> >>      help
> >>        Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
> >>        for the DTS file that will be used to produce the DTB linked into the
> >> diff --git a/arch/riscv/boot/dts/canaan/k210.dts b/arch/riscv/boot/dts/canaan/k210.dts
> >> deleted file mode 100644
> >> index 0d1f28fce6b2..000000000000
> >> --- a/arch/riscv/boot/dts/canaan/k210.dts
> >> +++ /dev/null
> >> @@ -1,23 +0,0 @@
> >> -// SPDX-License-Identifier: GPL-2.0+
> >> -/*
> >> - * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> >> - */
> >> -
> >> -/dts-v1/;
> >> -
> >> -#include "k210.dtsi"
> >> -
> >> -/ {
> >> -    model = "Kendryte K210 generic";
> >> -    compatible = "kendryte,k210";
> >> -
> >> -    chosen {
> >> -            bootargs = "earlycon console=ttySIF0";
> >> -            stdout-path = "serial0";
> >> -    };
> >> -};
> >> -
> >> -&uarths0 {
> >> -    status = "okay";
> >> -};
> >> -
> >> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
> >> index 354b263195a3..63c1f4c98d6c 100644
> >> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
> >> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
> >> @@ -1,9 +1,11 @@
> >>   // SPDX-License-Identifier: GPL-2.0+
> >>   /*
> >> - * Copyright (C) 2019 Sean Anderson <seanga2@gmail.com>
> >> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
> >>    * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> >>    */
> >>   #include <dt-bindings/clock/k210-clk.h>
> >> +#include <dt-bindings/pinctrl/k210-fpioa.h>
> >> +#include <dt-bindings/reset/k210-rst.h>
> >>
> >>   / {
> >>      /*
> >> @@ -12,14 +14,33 @@ / {
> >>       */
> >>      #address-cells = <1>;
> >>      #size-cells = <1>;
> >> -    compatible = "kendryte,k210";
> >> +    compatible = "canaan,kendryte-k210";
> >>
> >>      aliases {
> >> +            cpu0 = &cpu0;
> >> +            cpu1 = &cpu1;
> >> +            dma0 = &dmac0;
> >> +            gpio0 = &gpio0;
> >> +            gpio1 = &gpio1_0;
> >> +            i2c0 = &i2c0;
> >> +            i2c1 = &i2c1;
> >> +            i2c2 = &i2c2;
> >> +            pinctrl0 = &fpioa;
> >>              serial0 = &uarths0;
> >> +            serial1 = &uart1;
> >> +            serial2 = &uart2;
> >> +            serial3 = &uart3;
> >> +            spi0 = &spi0;
> >> +            spi1 = &spi1;
> >> +            spi2 = &spi2;
> >> +            spi3 = &spi3;
> >> +            timer0 = &timer0;
> >> +            timer1 = &timer1;
> >> +            timer2 = &timer2;
> >
> > Don't add random aliases. Really, only the serialN ones should be here.
> > cpu, dma, pinctrl, timer are definitely non-standard.
>
>
> All of these except for cpu and dma are already found in the kernel.
> They were primarily added for U-Boot.
>
> >
> >>      };
> >>
> >>      /*
> >> -     * The K210 has an sv39 MMU following the priviledge specification v1.9.
> >> +     * The K210 has an sv39 MMU following the privileged specification v1.9.
> >>       * Since this is a non-ratified draft specification, the kernel does not
> >>       * support it and the K210 support enabled only for the !MMU case.
> >>       * Be consistent with this by setting the CPUs MMU type to "none".
> >> @@ -30,14 +51,14 @@ cpus {
> >>              timebase-frequency = <7800000>;
> >>              cpu0: cpu@0 {
> >>                      device_type = "cpu";
> >> +                    compatible = "canaan,k210", "riscv";
> >>                      reg = <0>;
> >> -                    compatible = "kendryte,k210", "sifive,rocket0", "riscv";
> >>                      riscv,isa = "rv64imafdc";
> >> -                    mmu-type = "none";
> >> -                    i-cache-size = <0x8000>;
> >> +                    mmu-type = "riscv,none";
> >>                      i-cache-block-size = <64>;
> >> -                    d-cache-size = <0x8000>;
> >> +                    i-cache-size = <0x8000>;
> >>                      d-cache-block-size = <64>;
> >> +                    d-cache-size = <0x8000>;
> >>                      cpu0_intc: interrupt-controller {
> >>                              #interrupt-cells = <1>;
> >>                              interrupt-controller;
> >> @@ -46,14 +67,14 @@ cpu0_intc: interrupt-controller {
> >>              };
> >>              cpu1: cpu@1 {
> >>                      device_type = "cpu";
> >> +                    compatible = "canaan,k210", "riscv";
> >>                      reg = <1>;
> >> -                    compatible = "kendryte,k210", "sifive,rocket0", "riscv";
> >>                      riscv,isa = "rv64imafdc";
> >> -                    mmu-type = "none";
> >> -                    i-cache-size = <0x8000>;
> >> +                    mmu-type = "riscv,none";
> >>                      i-cache-block-size = <64>;
> >> -                    d-cache-size = <0x8000>;
> >> +                    i-cache-size = <0x8000>;
> >>                      d-cache-block-size = <64>;
> >> +                    d-cache-size = <0x8000>;
> >>                      cpu1_intc: interrupt-controller {
> >>                              #interrupt-cells = <1>;
> >>                              interrupt-controller;
> >> @@ -64,10 +85,15 @@ cpu1_intc: interrupt-controller {
> >>
> >>      sram: memory@80000000 {
> >>              device_type = "memory";
> >> +            compatible = "canaan,k210-sram";
> >>              reg = <0x80000000 0x400000>,
> >>                    <0x80400000 0x200000>,
> >>                    <0x80600000 0x200000>;
> >>              reg-names = "sram0", "sram1", "aisram";
> >> +            clocks = <&sysclk K210_CLK_SRAM0>,
> >> +                     <&sysclk K210_CLK_SRAM1>,
> >> +                     <&sysclk K210_CLK_AI>;
> >> +            clock-names = "sram0", "sram1", "aisram";
> >>      };
> >>
> >>      clocks {
> >> @@ -81,40 +107,493 @@ in0: oscillator {
> >>      soc {
> >>              #address-cells = <1>;
> >>              #size-cells = <1>;
> >> -            compatible = "kendryte,k210-soc", "simple-bus";
> >> +            compatible = "simple-bus";
> >>              ranges;
> >>              interrupt-parent = <&plic0>;
> >>
> >> -            sysctl: sysctl@50440000 {
> >> -                    compatible = "kendryte,k210-sysctl", "simple-mfd";
> >> -                    reg = <0x50440000 0x1000>;
> >> -                    #clock-cells = <1>;
> >> +            debug0: debug@0 {
> >> +                    compatible = "canaan,k210-debug", "riscv,debug";
> >
> > Not documented.
>
> On 1/14/21 7:06 PM, Sean Anderson wrote:
> > Here it's because "riscv,debug" doesn't exist. This is the "debug"
> > device as described in the debug spec. AFAIK Linux never needs to
> > configure this device. It could probably be removed.
>
> No objections here.
>
> >
> >> +                    reg = <0x0 0x1000>;
> >> +                    status = "disabled";
> >> +            };
> >> +
> >> +            rom0: nvmem@1000 {
> >> +                    reg = <0x1000 0x1000>;
> >> +                    read-only;
> >> +                    status = "disabled";
> >>              };
> >>
> >>              clint0: clint@2000000 {
> >
> > interrupt-controller@...
>
> Yes, this should be changed.
>
> >
> >> -                    #interrupt-cells = <1>;
> >> -                    compatible = "riscv,clint0";
> >> +                    compatible = "canaan,k210-clint", "sifive,clint0";
> >>                      reg = <0x2000000 0xC000>;
> >> -                    interrupts-extended =  <&cpu0_intc 3 &cpu0_intc 7
> >> -                                            &cpu1_intc 3 &cpu1_intc 7>;
> >> +                    interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> >> +                                          &cpu1_intc 3 &cpu1_intc 7>;
> >>              };
> >>
> >> -            plic0: interrupt-controller@c000000 {
> >> +            plic0: interrupt-controller@C000000 {
> >
> > No, lowercase hex in unit-addresses was correct.
>
> Do you have any authoritative source for this? When I was creating this
> tree, I didn't see anything about what case the addresses should be in
> (and a quick grep for lower-case in Documentation/devicetree doesn't
> have any relevant results).

It's ultimately up to the bus definition, but the DT spec may say
something. I don't recall offhand. For most bus definitions we have a
schema enforcement and dtc has some checks. (Though I just noticed the
simple-bus schema allows uppercase which is an error. dtc does not.)
This case doesn't get caught because there's not a bus definition for
root node. IMO, it should be same as 'simple-bus', but there was some
debate about that when I added the dtc checks.

The other cases should have been flagged by simple-pm-bus.yaml. You
all did run 'make dtbs_check' on this, right?

> >> +                    otp0: nvmem@50420000 {
> >> +                            #address-cells = <1>;
> >> +                            #size-cells = <1>;
> >> +                            compatible = "canaan,k210-otp";
> >> +                            reg = <0x50420000 0x100>,
> >> +                                  <0x88000000 0x20000>;
> >> +                            reg-names = "reg", "mem";
> >> +                            clocks = <&sysclk K210_CLK_ROM>;
> >> +                            resets = <&sysrst K210_RST_ROM>;
> >> +                            read-only;
> >> +                            status = "disabled";
> >
> > Your disabled nodes seem a bit excessive. A device should really only be
> > disabled if it's a board level decision to use or not. I'd assume the
> > OTP is always there and usable.
>
> It's disabled because there is no driver for it (yet). Though see below
> for the reasoning behind this.

No driver is not really a reason to disable. Why force a DT change
when there is a driver?

> >> +
> >> +                            /* Bootloader */
> >> +                            firmware@00000 {
> >
> > Drop leading 0s.
> >
> > Is this memory mapped? If so, you are missing 'ranges' in the parent to
> > make it translateable.
>
> Yes, there should be ranges.
>
> Though I am not sure that the ROM is controllable from this OTP...
>
> Perhaps it should be its own node.
>
> >
> >> +                                    reg = <0x00000 0xC200>;
> >> +                            };
> >> +
> >> +                            /*
> >> +                             * config string as described in RISC-V
> >> +                             * privileged spec 1.9
> >> +                             */
> >> +                            config-1-9@1c000 {
> >> +                                    reg = <0x1C000 0x1000>;
> >> +                            };
> >> +
> >> +                            /*
> >> +                             * Device tree containing only registers,
> >> +                             * interrupts, and cpus
> >> +                             */
> >> +                            fdt@1d000 {
> >> +                                    reg = <0x1D000 0x2000>;
> >> +                            };
> >> +
> >> +                            /* CPU/ROM credits */
> >> +                            credits@1f000 {
> >> +                                    reg = <0x1F000 0x1000>;
> >> +                            };
> >> +                    };
> >> +
> >> +                    dvp0: camera@50430000 {
> >> +                            compatible = "canaan,k210-dvp";
> >
> > No documented. Seems to be several of them.
>
> Correct. At some point there may be drivers. But there is no
> authoritative information (memory map, list of registers, list of
> interrupts) outside of source code for this board.

Then you should just omit it.

Rob

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-06  0:13     ` Damien Le Moal
@ 2021-02-08 20:00       ` Rob Herring
  2021-02-08 22:53         ` Sean Anderson
  0 siblings, 1 reply; 27+ messages in thread
From: Rob Herring @ 2021-02-08 20:00 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: palmer, linux-riscv, Atish Patra, seanga2, Anup Patel, devicetree

On Fri, Feb 5, 2021 at 6:13 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>
> On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
> [...]
> > > +                   otp0: nvmem@50420000 {
> > > +                           #address-cells = <1>;
> > > +                           #size-cells = <1>;
> > > +                           compatible = "canaan,k210-otp";
> > > +                           reg = <0x50420000 0x100>,
> > > +                                 <0x88000000 0x20000>;
> > > +                           reg-names = "reg", "mem";
> > > +                           clocks = <&sysclk K210_CLK_ROM>;
> > > +                           resets = <&sysrst K210_RST_ROM>;
> > > +                           read-only;
> > > +                           status = "disabled";
> >
> > Your disabled nodes seem a bit excessive. A device should really only be
> > disabled if it's a board level decision to use or not. I'd assume the
> > OTP is always there and usable.
>
> Please see below.
>
> >
> > > +
> > > +                           /* Bootloader */
> > > +                           firmware@00000 {
> >
> > Drop leading 0s.
> >
> > Is this memory mapped? If so, you are missing 'ranges' in the parent to
> > make it translateable.
> >
> > > +                                   reg = <0x00000 0xC200>;
> > > +                           };
> > > +
> > > +                           /*
> > > +                            * config string as described in RISC-V
> > > +                            * privileged spec 1.9
> > > +                            */
> > > +                           config-1-9@1c000 {
> > > +                                   reg = <0x1C000 0x1000>;
> > > +                           };
> > > +
> > > +                           /*
> > > +                            * Device tree containing only registers,
> > > +                            * interrupts, and cpus
> > > +                            */
> > > +                           fdt@1d000 {
> > > +                                   reg = <0x1D000 0x2000>;
> > > +                           };
> > > +
> > > +                           /* CPU/ROM credits */
> > > +                           credits@1f000 {
> > > +                                   reg = <0x1F000 0x1000>;
> > > +                           };
> > > +                   };
> > > +
> > > +                   dvp0: camera@50430000 {
> > > +                           compatible = "canaan,k210-dvp";
> >
> > No documented. Seems to be several of them.
>
> There are no Linux drivers for these undocumented nodes. That is why I did not
> add any documentation.

Documentation is required when dts files OR Linux drivers use them.

> make dtbs_check does not complain about that as long as
> the nodes are marked disabled.

'disabled' should only turn off required properties missing checks.
Undocumented compatible strings checks are about to get turned on now
that I've made it work without false positives.

> I kept these nodes to have the DTS in sync with
> U-Boot which has them.

That's a worthwhile goal. Doesn't u-boot require documenting bindings?

> Keeping them also creates documentation for the SoC
> since this device tree is more detailed than the SoC specsheet...

It's already 'documented' in u-boot it seems...

Rob

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-08 19:49       ` Rob Herring
@ 2021-02-08 22:41         ` Damien Le Moal
  0 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-08 22:41 UTC (permalink / raw)
  To: Rob Herring, Sean Anderson
  Cc: Palmer Dabbelt, linux-riscv, Atish Patra, Anup Patel, devicetree

On 2021/02/09 4:50, Rob Herring wrote:
> On Fri, Feb 5, 2021 at 4:52 PM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> On 2/5/21 3:25 PM, Rob Herring wrote:
>>> On Fri, Feb 05, 2021 at 03:58:20PM +0900, Damien Le Moal wrote:
>>>> Update the Canaan Kendryte K210 base device tree k210.dtsi to define
>>>> all peripherals of the SoC, their clocks and reset lines. The device
>>>> tree file k210.dts is renamed to k210_generic.dts and becomes the
>>>> default value selection of the SOC_CANAAN_K210_DTB_BUILTIN_SOURCE
>>>> configuration option. No device beside the serial console is defined by
>>>> this device tree. This makes this generic device tree suitable for use
>>>> with a builtin initramfs with all known K210 based boards.
>>>>
>>>> These changes result in the K210_CLK_ACLK clock ID to be unused and
>>>> removed from the dt-bindings k210-clk.h header file.
>>>>
>>>> Most updates to the k210.dtsi file come from Sean Anderson's work on
>>>> U-Boot support for the K210.
>>>>
>>>> Cc: Rob Herring <robh@kernel.org>
>>>> Cc: devicetree@vger.kernel.org
>>>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>>>> ---
>>>>   arch/riscv/Kconfig.socs                     |   2 +-
>>>>   arch/riscv/boot/dts/canaan/k210.dts         |  23 -
>>>>   arch/riscv/boot/dts/canaan/k210.dtsi        | 535 +++++++++++++++++++-
>>>>   arch/riscv/boot/dts/canaan/k210_generic.dts |  46 ++
>>>>   include/dt-bindings/clock/k210-clk.h        |   1 -
>>>>   5 files changed, 554 insertions(+), 53 deletions(-)
>>>>   delete mode 100644 arch/riscv/boot/dts/canaan/k210.dts
>>>>   create mode 100644 arch/riscv/boot/dts/canaan/k210_generic.dts
>>>>
>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>> index 6402746c68f3..7efcece8896c 100644
>>>> --- a/arch/riscv/Kconfig.socs
>>>> +++ b/arch/riscv/Kconfig.socs
>>>> @@ -51,7 +51,7 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>>      string "Source file for the Canaan Kendryte K210 builtin DTB"
>>>>      depends on SOC_CANAAN
>>>>      depends on SOC_CANAAN_K210_DTB_BUILTIN
>>>> -    default "k210"
>>>> +    default "k210_generic"
>>>>      help
>>>>        Base name (without suffix, relative to arch/riscv/boot/dts/canaan)
>>>>        for the DTS file that will be used to produce the DTB linked into the
>>>> diff --git a/arch/riscv/boot/dts/canaan/k210.dts b/arch/riscv/boot/dts/canaan/k210.dts
>>>> deleted file mode 100644
>>>> index 0d1f28fce6b2..000000000000
>>>> --- a/arch/riscv/boot/dts/canaan/k210.dts
>>>> +++ /dev/null
>>>> @@ -1,23 +0,0 @@
>>>> -// SPDX-License-Identifier: GPL-2.0+
>>>> -/*
>>>> - * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>>>> - */
>>>> -
>>>> -/dts-v1/;
>>>> -
>>>> -#include "k210.dtsi"
>>>> -
>>>> -/ {
>>>> -    model = "Kendryte K210 generic";
>>>> -    compatible = "kendryte,k210";
>>>> -
>>>> -    chosen {
>>>> -            bootargs = "earlycon console=ttySIF0";
>>>> -            stdout-path = "serial0";
>>>> -    };
>>>> -};
>>>> -
>>>> -&uarths0 {
>>>> -    status = "okay";
>>>> -};
>>>> -
>>>> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
>>>> index 354b263195a3..63c1f4c98d6c 100644
>>>> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
>>>> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
>>>> @@ -1,9 +1,11 @@
>>>>   // SPDX-License-Identifier: GPL-2.0+
>>>>   /*
>>>> - * Copyright (C) 2019 Sean Anderson <seanga2@gmail.com>
>>>> + * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
>>>>    * Copyright (C) 2020 Western Digital Corporation or its affiliates.
>>>>    */
>>>>   #include <dt-bindings/clock/k210-clk.h>
>>>> +#include <dt-bindings/pinctrl/k210-fpioa.h>
>>>> +#include <dt-bindings/reset/k210-rst.h>
>>>>
>>>>   / {
>>>>      /*
>>>> @@ -12,14 +14,33 @@ / {
>>>>       */
>>>>      #address-cells = <1>;
>>>>      #size-cells = <1>;
>>>> -    compatible = "kendryte,k210";
>>>> +    compatible = "canaan,kendryte-k210";
>>>>
>>>>      aliases {
>>>> +            cpu0 = &cpu0;
>>>> +            cpu1 = &cpu1;
>>>> +            dma0 = &dmac0;
>>>> +            gpio0 = &gpio0;
>>>> +            gpio1 = &gpio1_0;
>>>> +            i2c0 = &i2c0;
>>>> +            i2c1 = &i2c1;
>>>> +            i2c2 = &i2c2;
>>>> +            pinctrl0 = &fpioa;
>>>>              serial0 = &uarths0;
>>>> +            serial1 = &uart1;
>>>> +            serial2 = &uart2;
>>>> +            serial3 = &uart3;
>>>> +            spi0 = &spi0;
>>>> +            spi1 = &spi1;
>>>> +            spi2 = &spi2;
>>>> +            spi3 = &spi3;
>>>> +            timer0 = &timer0;
>>>> +            timer1 = &timer1;
>>>> +            timer2 = &timer2;
>>>
>>> Don't add random aliases. Really, only the serialN ones should be here.
>>> cpu, dma, pinctrl, timer are definitely non-standard.
>>
>>
>> All of these except for cpu and dma are already found in the kernel.
>> They were primarily added for U-Boot.
>>
>>>
>>>>      };
>>>>
>>>>      /*
>>>> -     * The K210 has an sv39 MMU following the priviledge specification v1.9.
>>>> +     * The K210 has an sv39 MMU following the privileged specification v1.9.
>>>>       * Since this is a non-ratified draft specification, the kernel does not
>>>>       * support it and the K210 support enabled only for the !MMU case.
>>>>       * Be consistent with this by setting the CPUs MMU type to "none".
>>>> @@ -30,14 +51,14 @@ cpus {
>>>>              timebase-frequency = <7800000>;
>>>>              cpu0: cpu@0 {
>>>>                      device_type = "cpu";
>>>> +                    compatible = "canaan,k210", "riscv";
>>>>                      reg = <0>;
>>>> -                    compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>>>>                      riscv,isa = "rv64imafdc";
>>>> -                    mmu-type = "none";
>>>> -                    i-cache-size = <0x8000>;
>>>> +                    mmu-type = "riscv,none";
>>>>                      i-cache-block-size = <64>;
>>>> -                    d-cache-size = <0x8000>;
>>>> +                    i-cache-size = <0x8000>;
>>>>                      d-cache-block-size = <64>;
>>>> +                    d-cache-size = <0x8000>;
>>>>                      cpu0_intc: interrupt-controller {
>>>>                              #interrupt-cells = <1>;
>>>>                              interrupt-controller;
>>>> @@ -46,14 +67,14 @@ cpu0_intc: interrupt-controller {
>>>>              };
>>>>              cpu1: cpu@1 {
>>>>                      device_type = "cpu";
>>>> +                    compatible = "canaan,k210", "riscv";
>>>>                      reg = <1>;
>>>> -                    compatible = "kendryte,k210", "sifive,rocket0", "riscv";
>>>>                      riscv,isa = "rv64imafdc";
>>>> -                    mmu-type = "none";
>>>> -                    i-cache-size = <0x8000>;
>>>> +                    mmu-type = "riscv,none";
>>>>                      i-cache-block-size = <64>;
>>>> -                    d-cache-size = <0x8000>;
>>>> +                    i-cache-size = <0x8000>;
>>>>                      d-cache-block-size = <64>;
>>>> +                    d-cache-size = <0x8000>;
>>>>                      cpu1_intc: interrupt-controller {
>>>>                              #interrupt-cells = <1>;
>>>>                              interrupt-controller;
>>>> @@ -64,10 +85,15 @@ cpu1_intc: interrupt-controller {
>>>>
>>>>      sram: memory@80000000 {
>>>>              device_type = "memory";
>>>> +            compatible = "canaan,k210-sram";
>>>>              reg = <0x80000000 0x400000>,
>>>>                    <0x80400000 0x200000>,
>>>>                    <0x80600000 0x200000>;
>>>>              reg-names = "sram0", "sram1", "aisram";
>>>> +            clocks = <&sysclk K210_CLK_SRAM0>,
>>>> +                     <&sysclk K210_CLK_SRAM1>,
>>>> +                     <&sysclk K210_CLK_AI>;
>>>> +            clock-names = "sram0", "sram1", "aisram";
>>>>      };
>>>>
>>>>      clocks {
>>>> @@ -81,40 +107,493 @@ in0: oscillator {
>>>>      soc {
>>>>              #address-cells = <1>;
>>>>              #size-cells = <1>;
>>>> -            compatible = "kendryte,k210-soc", "simple-bus";
>>>> +            compatible = "simple-bus";
>>>>              ranges;
>>>>              interrupt-parent = <&plic0>;
>>>>
>>>> -            sysctl: sysctl@50440000 {
>>>> -                    compatible = "kendryte,k210-sysctl", "simple-mfd";
>>>> -                    reg = <0x50440000 0x1000>;
>>>> -                    #clock-cells = <1>;
>>>> +            debug0: debug@0 {
>>>> +                    compatible = "canaan,k210-debug", "riscv,debug";
>>>
>>> Not documented.
>>
>> On 1/14/21 7:06 PM, Sean Anderson wrote:
>>> Here it's because "riscv,debug" doesn't exist. This is the "debug"
>>> device as described in the debug spec. AFAIK Linux never needs to
>>> configure this device. It could probably be removed.
>>
>> No objections here.
>>
>>>
>>>> +                    reg = <0x0 0x1000>;
>>>> +                    status = "disabled";
>>>> +            };
>>>> +
>>>> +            rom0: nvmem@1000 {
>>>> +                    reg = <0x1000 0x1000>;
>>>> +                    read-only;
>>>> +                    status = "disabled";
>>>>              };
>>>>
>>>>              clint0: clint@2000000 {
>>>
>>> interrupt-controller@...
>>
>> Yes, this should be changed.
>>
>>>
>>>> -                    #interrupt-cells = <1>;
>>>> -                    compatible = "riscv,clint0";
>>>> +                    compatible = "canaan,k210-clint", "sifive,clint0";
>>>>                      reg = <0x2000000 0xC000>;
>>>> -                    interrupts-extended =  <&cpu0_intc 3 &cpu0_intc 7
>>>> -                                            &cpu1_intc 3 &cpu1_intc 7>;
>>>> +                    interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>>>> +                                          &cpu1_intc 3 &cpu1_intc 7>;
>>>>              };
>>>>
>>>> -            plic0: interrupt-controller@c000000 {
>>>> +            plic0: interrupt-controller@C000000 {
>>>
>>> No, lowercase hex in unit-addresses was correct.
>>
>> Do you have any authoritative source for this? When I was creating this
>> tree, I didn't see anything about what case the addresses should be in
>> (and a quick grep for lower-case in Documentation/devicetree doesn't
>> have any relevant results).
> 
> It's ultimately up to the bus definition, but the DT spec may say
> something. I don't recall offhand. For most bus definitions we have a
> schema enforcement and dtc has some checks. (Though I just noticed the
> simple-bus schema allows uppercase which is an error. dtc does not.)
> This case doesn't get caught because there's not a bus definition for
> root node. IMO, it should be same as 'simple-bus', but there was some
> debate about that when I added the dtc checks.
> 
> The other cases should have been flagged by simple-pm-bus.yaml. You
> all did run 'make dtbs_check' on this, right?

Yes, I did, and rerun before sending any new version of the series. No warnings
at all show about anything.

> 
>>>> +                    otp0: nvmem@50420000 {
>>>> +                            #address-cells = <1>;
>>>> +                            #size-cells = <1>;
>>>> +                            compatible = "canaan,k210-otp";
>>>> +                            reg = <0x50420000 0x100>,
>>>> +                                  <0x88000000 0x20000>;
>>>> +                            reg-names = "reg", "mem";
>>>> +                            clocks = <&sysclk K210_CLK_ROM>;
>>>> +                            resets = <&sysrst K210_RST_ROM>;
>>>> +                            read-only;
>>>> +                            status = "disabled";
>>>
>>> Your disabled nodes seem a bit excessive. A device should really only be
>>> disabled if it's a board level decision to use or not. I'd assume the
>>> OTP is always there and usable.
>>
>> It's disabled because there is no driver for it (yet). Though see below
>> for the reasoning behind this.
> 
> No driver is not really a reason to disable. Why force a DT change
> when there is a driver?

We can omit the nodes that do not have drivers, but when the drivers are added,
there will be a need to change the DT. So isn't it the same in the end ?

> 
>>>> +
>>>> +                            /* Bootloader */
>>>> +                            firmware@00000 {
>>>
>>> Drop leading 0s.
>>>
>>> Is this memory mapped? If so, you are missing 'ranges' in the parent to
>>> make it translateable.
>>
>> Yes, there should be ranges.
>>
>> Though I am not sure that the ROM is controllable from this OTP...
>>
>> Perhaps it should be its own node.
>>
>>>
>>>> +                                    reg = <0x00000 0xC200>;
>>>> +                            };
>>>> +
>>>> +                            /*
>>>> +                             * config string as described in RISC-V
>>>> +                             * privileged spec 1.9
>>>> +                             */
>>>> +                            config-1-9@1c000 {
>>>> +                                    reg = <0x1C000 0x1000>;
>>>> +                            };
>>>> +
>>>> +                            /*
>>>> +                             * Device tree containing only registers,
>>>> +                             * interrupts, and cpus
>>>> +                             */
>>>> +                            fdt@1d000 {
>>>> +                                    reg = <0x1D000 0x2000>;
>>>> +                            };
>>>> +
>>>> +                            /* CPU/ROM credits */
>>>> +                            credits@1f000 {
>>>> +                                    reg = <0x1F000 0x1000>;
>>>> +                            };
>>>> +                    };
>>>> +
>>>> +                    dvp0: camera@50430000 {
>>>> +                            compatible = "canaan,k210-dvp";
>>>
>>> No documented. Seems to be several of them.
>>
>> Correct. At some point there may be drivers. But there is no
>> authoritative information (memory map, list of registers, list of
>> interrupts) outside of source code for this board.
> 
> Then you should just omit it.

OK. Will omit the unsupported nodes. Sending v18.

> 
> Rob
> 


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-08 20:00       ` Rob Herring
@ 2021-02-08 22:53         ` Sean Anderson
  2021-02-08 22:55           ` Damien Le Moal
  0 siblings, 1 reply; 27+ messages in thread
From: Sean Anderson @ 2021-02-08 22:53 UTC (permalink / raw)
  To: Rob Herring, Damien Le Moal
  Cc: palmer, linux-riscv, Atish Patra, Anup Patel, devicetree

On 2/8/21 3:00 PM, Rob Herring wrote:
> On Fri, Feb 5, 2021 at 6:13 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>>
>> On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
>> [...]
>>>> +                   otp0: nvmem@50420000 {
>>>> +                           #address-cells = <1>;
>>>> +                           #size-cells = <1>;
>>>> +                           compatible = "canaan,k210-otp";
>>>> +                           reg = <0x50420000 0x100>,
>>>> +                                 <0x88000000 0x20000>;
>>>> +                           reg-names = "reg", "mem";
>>>> +                           clocks = <&sysclk K210_CLK_ROM>;
>>>> +                           resets = <&sysrst K210_RST_ROM>;
>>>> +                           read-only;
>>>> +                           status = "disabled";
>>>
>>> Your disabled nodes seem a bit excessive. A device should really only be
>>> disabled if it's a board level decision to use or not. I'd assume the
>>> OTP is always there and usable.
>>
>> Please see below.
>>
>>>
>>>> +
>>>> +                           /* Bootloader */
>>>> +                           firmware@00000 {
>>>
>>> Drop leading 0s.
>>>
>>> Is this memory mapped? If so, you are missing 'ranges' in the parent to
>>> make it translateable.
>>>
>>>> +                                   reg = <0x00000 0xC200>;
>>>> +                           };
>>>> +
>>>> +                           /*
>>>> +                            * config string as described in RISC-V
>>>> +                            * privileged spec 1.9
>>>> +                            */
>>>> +                           config-1-9@1c000 {
>>>> +                                   reg = <0x1C000 0x1000>;
>>>> +                           };
>>>> +
>>>> +                           /*
>>>> +                            * Device tree containing only registers,
>>>> +                            * interrupts, and cpus
>>>> +                            */
>>>> +                           fdt@1d000 {
>>>> +                                   reg = <0x1D000 0x2000>;
>>>> +                           };
>>>> +
>>>> +                           /* CPU/ROM credits */
>>>> +                           credits@1f000 {
>>>> +                                   reg = <0x1F000 0x1000>;
>>>> +                           };
>>>> +                   };
>>>> +
>>>> +                   dvp0: camera@50430000 {
>>>> +                           compatible = "canaan,k210-dvp";
>>>
>>> No documented. Seems to be several of them.
>>
>> There are no Linux drivers for these undocumented nodes. That is why I did not
>> add any documentation.
> 
> Documentation is required when dts files OR Linux drivers use them.
> 
>> make dtbs_check does not complain about that as long as
>> the nodes are marked disabled.
> 
> 'disabled' should only turn off required properties missing checks.
> Undocumented compatible strings checks are about to get turned on now
> that I've made it work without false positives.
> 
>> I kept these nodes to have the DTS in sync with
>> U-Boot which has them.
> 
> That's a worthwhile goal. Doesn't u-boot require documenting bindings?

Generally, no. Usually if the bindings differ from the kernel they are
documented, but usually the device trees are just imported straight from
the kernel. This is a bit of an unusual case in that the device tree is
being imported from U-Boot instead of the other way around.

> 
>> Keeping them also creates documentation for the SoC
>> since this device tree is more detailed than the SoC specsheet...
> 
> It's already 'documented' in u-boot it seems...

I would like to keep Kernel and U-Boot device trees in-sync. However, if
there are significant divergences, that becomes more difficult.

--Sean

> 
> Rob
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-08 22:53         ` Sean Anderson
@ 2021-02-08 22:55           ` Damien Le Moal
  2021-02-08 23:04             ` Sean Anderson
  0 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2021-02-08 22:55 UTC (permalink / raw)
  To: Sean Anderson, Rob Herring
  Cc: palmer, linux-riscv, Atish Patra, Anup Patel, devicetree

On 2021/02/09 7:53, Sean Anderson wrote:
> On 2/8/21 3:00 PM, Rob Herring wrote:
>> On Fri, Feb 5, 2021 at 6:13 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>>>
>>> On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
>>> [...]
>>>>> +                   otp0: nvmem@50420000 {
>>>>> +                           #address-cells = <1>;
>>>>> +                           #size-cells = <1>;
>>>>> +                           compatible = "canaan,k210-otp";
>>>>> +                           reg = <0x50420000 0x100>,
>>>>> +                                 <0x88000000 0x20000>;
>>>>> +                           reg-names = "reg", "mem";
>>>>> +                           clocks = <&sysclk K210_CLK_ROM>;
>>>>> +                           resets = <&sysrst K210_RST_ROM>;
>>>>> +                           read-only;
>>>>> +                           status = "disabled";
>>>>
>>>> Your disabled nodes seem a bit excessive. A device should really only be
>>>> disabled if it's a board level decision to use or not. I'd assume the
>>>> OTP is always there and usable.
>>>
>>> Please see below.
>>>
>>>>
>>>>> +
>>>>> +                           /* Bootloader */
>>>>> +                           firmware@00000 {
>>>>
>>>> Drop leading 0s.
>>>>
>>>> Is this memory mapped? If so, you are missing 'ranges' in the parent to
>>>> make it translateable.
>>>>
>>>>> +                                   reg = <0x00000 0xC200>;
>>>>> +                           };
>>>>> +
>>>>> +                           /*
>>>>> +                            * config string as described in RISC-V
>>>>> +                            * privileged spec 1.9
>>>>> +                            */
>>>>> +                           config-1-9@1c000 {
>>>>> +                                   reg = <0x1C000 0x1000>;
>>>>> +                           };
>>>>> +
>>>>> +                           /*
>>>>> +                            * Device tree containing only registers,
>>>>> +                            * interrupts, and cpus
>>>>> +                            */
>>>>> +                           fdt@1d000 {
>>>>> +                                   reg = <0x1D000 0x2000>;
>>>>> +                           };
>>>>> +
>>>>> +                           /* CPU/ROM credits */
>>>>> +                           credits@1f000 {
>>>>> +                                   reg = <0x1F000 0x1000>;
>>>>> +                           };
>>>>> +                   };
>>>>> +
>>>>> +                   dvp0: camera@50430000 {
>>>>> +                           compatible = "canaan,k210-dvp";
>>>>
>>>> No documented. Seems to be several of them.
>>>
>>> There are no Linux drivers for these undocumented nodes. That is why I did not
>>> add any documentation.
>>
>> Documentation is required when dts files OR Linux drivers use them.
>>
>>> make dtbs_check does not complain about that as long as
>>> the nodes are marked disabled.
>>
>> 'disabled' should only turn off required properties missing checks.
>> Undocumented compatible strings checks are about to get turned on now
>> that I've made it work without false positives.
>>
>>> I kept these nodes to have the DTS in sync with
>>> U-Boot which has them.
>>
>> That's a worthwhile goal. Doesn't u-boot require documenting bindings?
> 
> Generally, no. Usually if the bindings differ from the kernel they are
> documented, but usually the device trees are just imported straight from
> the kernel. This is a bit of an unusual case in that the device tree is
> being imported from U-Boot instead of the other way around.
> 
>>
>>> Keeping them also creates documentation for the SoC
>>> since this device tree is more detailed than the SoC specsheet...
>>
>> It's already 'documented' in u-boot it seems...
> 
> I would like to keep Kernel and U-Boot device trees in-sync. However, if
> there are significant divergences, that becomes more difficult.

Sean,

Are you OK with removing the nodes without a driver ? I think they are the same
for the kernel and U-Boot but I have not checked in detail.

> 
> --Sean
> 
>>
>> Rob
>>
> 
> 


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-08 22:55           ` Damien Le Moal
@ 2021-02-08 23:04             ` Sean Anderson
  2021-02-09  0:12               ` Damien Le Moal
  0 siblings, 1 reply; 27+ messages in thread
From: Sean Anderson @ 2021-02-08 23:04 UTC (permalink / raw)
  To: Damien Le Moal, Rob Herring
  Cc: palmer, linux-riscv, Atish Patra, Anup Patel, devicetree

On 2/8/21 5:55 PM, Damien Le Moal wrote:
> On 2021/02/09 7:53, Sean Anderson wrote:
>> On 2/8/21 3:00 PM, Rob Herring wrote:
>>> On Fri, Feb 5, 2021 at 6:13 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>>>>
>>>> On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
>>>> [...]
>>>>>> +                   otp0: nvmem@50420000 {
>>>>>> +                           #address-cells = <1>;
>>>>>> +                           #size-cells = <1>;
>>>>>> +                           compatible = "canaan,k210-otp";
>>>>>> +                           reg = <0x50420000 0x100>,
>>>>>> +                                 <0x88000000 0x20000>;
>>>>>> +                           reg-names = "reg", "mem";
>>>>>> +                           clocks = <&sysclk K210_CLK_ROM>;
>>>>>> +                           resets = <&sysrst K210_RST_ROM>;
>>>>>> +                           read-only;
>>>>>> +                           status = "disabled";
>>>>>
>>>>> Your disabled nodes seem a bit excessive. A device should really only be
>>>>> disabled if it's a board level decision to use or not. I'd assume the
>>>>> OTP is always there and usable.
>>>>
>>>> Please see below.
>>>>
>>>>>
>>>>>> +
>>>>>> +                           /* Bootloader */
>>>>>> +                           firmware@00000 {
>>>>>
>>>>> Drop leading 0s.
>>>>>
>>>>> Is this memory mapped? If so, you are missing 'ranges' in the parent to
>>>>> make it translateable.
>>>>>
>>>>>> +                                   reg = <0x00000 0xC200>;
>>>>>> +                           };
>>>>>> +
>>>>>> +                           /*
>>>>>> +                            * config string as described in RISC-V
>>>>>> +                            * privileged spec 1.9
>>>>>> +                            */
>>>>>> +                           config-1-9@1c000 {
>>>>>> +                                   reg = <0x1C000 0x1000>;
>>>>>> +                           };
>>>>>> +
>>>>>> +                           /*
>>>>>> +                            * Device tree containing only registers,
>>>>>> +                            * interrupts, and cpus
>>>>>> +                            */
>>>>>> +                           fdt@1d000 {
>>>>>> +                                   reg = <0x1D000 0x2000>;
>>>>>> +                           };
>>>>>> +
>>>>>> +                           /* CPU/ROM credits */
>>>>>> +                           credits@1f000 {
>>>>>> +                                   reg = <0x1F000 0x1000>;
>>>>>> +                           };
>>>>>> +                   };
>>>>>> +
>>>>>> +                   dvp0: camera@50430000 {
>>>>>> +                           compatible = "canaan,k210-dvp";
>>>>>
>>>>> No documented. Seems to be several of them.
>>>>
>>>> There are no Linux drivers for these undocumented nodes. That is why I did not
>>>> add any documentation.
>>>
>>> Documentation is required when dts files OR Linux drivers use them.
>>>
>>>> make dtbs_check does not complain about that as long as
>>>> the nodes are marked disabled.
>>>
>>> 'disabled' should only turn off required properties missing checks.
>>> Undocumented compatible strings checks are about to get turned on now
>>> that I've made it work without false positives.
>>>
>>>> I kept these nodes to have the DTS in sync with
>>>> U-Boot which has them.
>>>
>>> That's a worthwhile goal. Doesn't u-boot require documenting bindings?
>>
>> Generally, no. Usually if the bindings differ from the kernel they are
>> documented, but usually the device trees are just imported straight from
>> the kernel. This is a bit of an unusual case in that the device tree is
>> being imported from U-Boot instead of the other way around.
>>
>>>
>>>> Keeping them also creates documentation for the SoC
>>>> since this device tree is more detailed than the SoC specsheet...
>>>
>>> It's already 'documented' in u-boot it seems...
>>
>> I would like to keep Kernel and U-Boot device trees in-sync. However, if
>> there are significant divergences, that becomes more difficult.
> 
> Sean,
> 
> Are you OK with removing the nodes without a driver ? I think they are the same
> for the kernel and U-Boot but I have not checked in detail.

I suppose. The 8285 uarts should be kept as iirc someone was using them.
Same with i2c. WDT has a U-Boot driver, and probably has a Linux one too
(I haven't checked). I believe the timers also have working drivers, but
I haven't tested on Linux.

--Sean

> 
>>
>> --Sean
>>
>>>
>>> Rob
>>>
>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree
  2021-02-08 23:04             ` Sean Anderson
@ 2021-02-09  0:12               ` Damien Le Moal
  0 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2021-02-09  0:12 UTC (permalink / raw)
  To: Sean Anderson, Rob Herring
  Cc: palmer, linux-riscv, Atish Patra, Anup Patel, devicetree

On 2021/02/09 8:05, Sean Anderson wrote:
> On 2/8/21 5:55 PM, Damien Le Moal wrote:
>> On 2021/02/09 7:53, Sean Anderson wrote:
>>> On 2/8/21 3:00 PM, Rob Herring wrote:
>>>> On Fri, Feb 5, 2021 at 6:13 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>>>>>
>>>>> On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
>>>>> [...]
>>>>>>> +                   otp0: nvmem@50420000 {
>>>>>>> +                           #address-cells = <1>;
>>>>>>> +                           #size-cells = <1>;
>>>>>>> +                           compatible = "canaan,k210-otp";
>>>>>>> +                           reg = <0x50420000 0x100>,
>>>>>>> +                                 <0x88000000 0x20000>;
>>>>>>> +                           reg-names = "reg", "mem";
>>>>>>> +                           clocks = <&sysclk K210_CLK_ROM>;
>>>>>>> +                           resets = <&sysrst K210_RST_ROM>;
>>>>>>> +                           read-only;
>>>>>>> +                           status = "disabled";
>>>>>>
>>>>>> Your disabled nodes seem a bit excessive. A device should really only be
>>>>>> disabled if it's a board level decision to use or not. I'd assume the
>>>>>> OTP is always there and usable.
>>>>>
>>>>> Please see below.
>>>>>
>>>>>>
>>>>>>> +
>>>>>>> +                           /* Bootloader */
>>>>>>> +                           firmware@00000 {
>>>>>>
>>>>>> Drop leading 0s.
>>>>>>
>>>>>> Is this memory mapped? If so, you are missing 'ranges' in the parent to
>>>>>> make it translateable.
>>>>>>
>>>>>>> +                                   reg = <0x00000 0xC200>;
>>>>>>> +                           };
>>>>>>> +
>>>>>>> +                           /*
>>>>>>> +                            * config string as described in RISC-V
>>>>>>> +                            * privileged spec 1.9
>>>>>>> +                            */
>>>>>>> +                           config-1-9@1c000 {
>>>>>>> +                                   reg = <0x1C000 0x1000>;
>>>>>>> +                           };
>>>>>>> +
>>>>>>> +                           /*
>>>>>>> +                            * Device tree containing only registers,
>>>>>>> +                            * interrupts, and cpus
>>>>>>> +                            */
>>>>>>> +                           fdt@1d000 {
>>>>>>> +                                   reg = <0x1D000 0x2000>;
>>>>>>> +                           };
>>>>>>> +
>>>>>>> +                           /* CPU/ROM credits */
>>>>>>> +                           credits@1f000 {
>>>>>>> +                                   reg = <0x1F000 0x1000>;
>>>>>>> +                           };
>>>>>>> +                   };
>>>>>>> +
>>>>>>> +                   dvp0: camera@50430000 {
>>>>>>> +                           compatible = "canaan,k210-dvp";
>>>>>>
>>>>>> No documented. Seems to be several of them.
>>>>>
>>>>> There are no Linux drivers for these undocumented nodes. That is why I did not
>>>>> add any documentation.
>>>>
>>>> Documentation is required when dts files OR Linux drivers use them.
>>>>
>>>>> make dtbs_check does not complain about that as long as
>>>>> the nodes are marked disabled.
>>>>
>>>> 'disabled' should only turn off required properties missing checks.
>>>> Undocumented compatible strings checks are about to get turned on now
>>>> that I've made it work without false positives.
>>>>
>>>>> I kept these nodes to have the DTS in sync with
>>>>> U-Boot which has them.
>>>>
>>>> That's a worthwhile goal. Doesn't u-boot require documenting bindings?
>>>
>>> Generally, no. Usually if the bindings differ from the kernel they are
>>> documented, but usually the device trees are just imported straight from
>>> the kernel. This is a bit of an unusual case in that the device tree is
>>> being imported from U-Boot instead of the other way around.
>>>
>>>>
>>>>> Keeping them also creates documentation for the SoC
>>>>> since this device tree is more detailed than the SoC specsheet...
>>>>
>>>> It's already 'documented' in u-boot it seems...
>>>
>>> I would like to keep Kernel and U-Boot device trees in-sync. However, if
>>> there are significant divergences, that becomes more difficult.
>>
>> Sean,
>>
>> Are you OK with removing the nodes without a driver ? I think they are the same
>> for the kernel and U-Boot but I have not checked in detail.
> 
> I suppose. The 8285 uarts should be kept as iirc someone was using them.
> Same with i2c. WDT has a U-Boot driver, and probably has a Linux one too
> (I haven't checked). I believe the timers also have working drivers, but
> I haven't tested on Linux.

Yep, all of these are supported and work. I used i2c a lot for my biped robot
and robot arm control. The watchdog timers also work but I did not enable the
drivers by default in the kernel config.

Preparing v18 with the updated dtsi then. I do not think we need to change the
dts for the boards. Will check.

Thanks !

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2021-02-09  0:13 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210205065827.577285-1-damien.lemoal@wdc.com>
2021-02-05  6:58 ` [PATCH v16 02/16] dt-bindings: add Canaan boards compatible strings Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 03/16] dt-bindings: update risc-v cpu properties Damien Le Moal
2021-02-05 19:47   ` Rob Herring
2021-02-05  6:58 ` [PATCH v16 04/16] dt-bindings: update sifive plic compatible string Damien Le Moal
2021-02-05 19:47   ` Rob Herring
2021-02-05  6:58 ` [PATCH v16 05/16] dt-bindings: update sifive clint " Damien Le Moal
2021-02-05 19:48   ` Rob Herring
2021-02-05  6:58 ` [PATCH v16 06/16] dt-bindings: update sifive uart " Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 07/16] dt-bindings: fix sifive gpio properties Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 08/16] dt-bindings: add resets property to dw-apb-timer Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree Damien Le Moal
2021-02-05 20:25   ` Rob Herring
2021-02-05 22:52     ` Sean Anderson
2021-02-05 23:49       ` Damien Le Moal
2021-02-08 19:49       ` Rob Herring
2021-02-08 22:41         ` Damien Le Moal
2021-02-06  0:13     ` Damien Le Moal
2021-02-08 20:00       ` Rob Herring
2021-02-08 22:53         ` Sean Anderson
2021-02-08 22:55           ` Damien Le Moal
2021-02-08 23:04             ` Sean Anderson
2021-02-09  0:12               ` Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 10/16] riscv: Add SiPeed MAIX BiT board " Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 11/16] riscv: Add SiPeed MAIX DOCK " Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 12/16] riscv: Add SiPeed MAIX GO " Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 13/16] riscv: Add SiPeed MAIXDUINO " Damien Le Moal
2021-02-05  6:58 ` [PATCH v16 14/16] riscv: Add Kendryte KD233 " Damien Le Moal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).