Re: [PATCH v6 04/12] dt-bindings: reset: nuvoton: add binding for ma35d1 IP reset control
From: Jacky Huang
Date: Wed Mar 29 2023 - 05:13:03 EST
Dear Krzysztof,
On 2023/3/29 下午 04:17, Krzysztof Kozlowski wrote:
On 28/03/2023 04:19, Jacky Huang wrote:
From: Jacky Huang <ychuang3@xxxxxxxxxxx>
Subject: drop second/last, redundant "bindings". The "dt-bindings"
prefix is already stating that these are bindings.
This is a friendly reminder during the review process.
It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.
Thank you.
Add the dt-bindings header for Nuvoton ma35d1, that gets shared
between the reset controller and reset references in the dts.
Add documentation to describe nuvoton ma35d1 reset driver.
Signed-off-by: Jacky Huang <ychuang3@xxxxxxxxxxx>
---
.../bindings/reset/nuvoton,ma35d1-reset.yaml | 44 +++++++
.../dt-bindings/reset/nuvoton,ma35d1-reset.h | 108 ++++++++++++++++++
2 files changed, 152 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
create mode 100644 include/dt-bindings/reset/nuvoton,ma35d1-reset.h
diff --git a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
new file mode 100644
index 000000000000..342717257e5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/nuvoton,ma35d1-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 Reset Controller
+
+maintainers:
+ - Chi-Fang Li <cfli0@xxxxxxxxxxx>
+ - Jacky Huang <ychuang3@xxxxxxxxxxx>
+
+description:
+ The system reset controller can be used to reset various peripheral
+ controllers in MA35D1 SoC.
+
+properties:
+ compatible:
+ const: nuvoton,ma35d1-reset
+
+ '#reset-cells':
+ const: 1
+
+required:
+ - compatible
+ - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+ # system reset controller node:
+ - |
+
+ system-management@40460000 {
+ compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";
+ reg = <0x40460000 0x200>;
+
+ reset-controller {
+ compatible = "nuvoton,ma35d1-reset";
+ #reset-cells = <1>;
You do not take any resources here, thus this should be rather part of
the parent node.
I will modify it as:
sys: system-management@40460000 {
compatible = "nuvoton,ma35d1-sys", "syscon",;
reg = <0x0 0x40460000 0x0 0x200>;
#reset-cells = <1>;
};
};
+ };
+ };
+...
+
diff --git a/include/dt-bindings/reset/nuvoton,ma35d1-reset.h b/include/dt-bindings/reset/nuvoton,ma35d1-reset.h
new file mode 100644
index 000000000000..f3088bc0afec
--- /dev/null
+++ b/include/dt-bindings/reset/nuvoton,ma35d1-reset.h
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause) */
Weird license, why 2.0+?
You already got here comment about license in previous version of this
patch.
In fact, I always be confused with the license.
It should be right if I just copy one from the same directory
/include/dt-bindings/reset/.
But, when I find there are several versions there, I was confused and
just select one of them.
/* SPDX-License-Identifier: GPL-2.0-only */
/* SPDX-License-Identifier: GPL-2.0+ */
/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)*/
/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
So, should I fix the license as
/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
just remove the '+' ?
+/*
+ * Copyright (C) 2023 Nuvoton Technologies.
+ * Author: Chi-Fen Li <cfli0@xxxxxxxxxxx>
+ *
+ * Device Tree binding constants for MA35D1 reset controller.
+ */
+
+#ifndef __DT_BINDINGS_RESET_MA35D1_H
+#define __DT_BINDINGS_RESET_MA35D1_H
+
+#define MA35D1_RESET_CHIP 0
+#define MA35D1_RESET_CA35CR0 1
+#define MA35D1_RESET_CA35CR1 2
+#define MA35D1_RESET_CM4 3
+#define MA35D1_RESET_PDMA0 4
+#define MA35D1_RESET_PDMA1 5
+#define MA35D1_RESET_PDMA2 6
+#define MA35D1_RESET_PDMA3 7
+#define MA35D1_RESET_DISP 9
+#define MA35D1_RESET_VCAP0 10
+#define MA35D1_RESET_VCAP1 11
+#define MA35D1_RESET_GFX 12
+#define MA35D1_RESET_VDEC 13
+#define MA35D1_RESET_WHC0 14
+#define MA35D1_RESET_WHC1 15
+#define MA35D1_RESET_GMAC0 16
+#define MA35D1_RESET_GMAC1 17
+#define MA35D1_RESET_HWSEM 18
+#define MA35D1_RESET_EBI 19
+#define MA35D1_RESET_HSUSBH0 20
+#define MA35D1_RESET_HSUSBH1 21
+#define MA35D1_RESET_HSUSBD 22
+#define MA35D1_RESET_USBHL 23
+#define MA35D1_RESET_SDH0 24
+#define MA35D1_RESET_SDH1 25
+#define MA35D1_RESET_NAND 26
+#define MA35D1_RESET_GPIO 27
+#define MA35D1_RESET_MCTLP 28
+#define MA35D1_RESET_MCTLC 29
+#define MA35D1_RESET_DDRPUB 30
+#define MA35D1_RESET_TMR0 34
+#define MA35D1_RESET_TMR1 35
+#define MA35D1_RESET_TMR2 36
+#define MA35D1_RESET_TMR3 37
+#define MA35D1_RESET_I2C0 40
+#define MA35D1_RESET_I2C1 41
+#define MA35D1_RESET_I2C2 42
+#define MA35D1_RESET_I2C3 43
+#define MA35D1_RESET_QSPI0 44
+#define MA35D1_RESET_SPI0 45
+#define MA35D1_RESET_SPI1 46
+#define MA35D1_RESET_SPI2 47
+#define MA35D1_RESET_UART0 48
+#define MA35D1_RESET_UART1 49
+#define MA35D1_RESET_UART2 50
+#define MA35D1_RESET_UAER3 51
+#define MA35D1_RESET_UART4 52
+#define MA35D1_RESET_UART5 53
+#define MA35D1_RESET_UART6 54
+#define MA35D1_RESET_UART7 55
+#define MA35D1_RESET_CANFD0 56
+#define MA35D1_RESET_CANFD1 57
+#define MA35D1_RESET_EADC0 60
Why do you have gaps here? These should be IDs, not register offsets.
Best regards,
Krzysztof
The reset controller registers are composed of four 32-bits register.
And the ID will be translated into the corresponding bit of these register.
Each ID is mapped to a unique bit position.
Best regards,
Jacky Huang