Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller
From: Conor Dooley <conor@kernel.org>
Date: 2025-11-24 19:54:11
Also in:
linux-devicetree, lkml
On Mon, Nov 24, 2025 at 02:32:33PM +0800, Gary Yang wrote:
quoted hunk ↗ jump to hunk
There are two reset controllers on Cix sky1 Soc. One is located in S0 domain, and the other is located in S5 domain. Signed-off-by: Gary Yang <gary.yang@cixtech.com> --- .../bindings/reset/cix,sky1-rst.yaml | 50 ++++++ include/dt-bindings/reset/cix,sky1-rst-fch.h | 42 +++++ include/dt-bindings/reset/cix,sky1-rst.h | 164 ++++++++++++++++++ 3 files changed, 256 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml create mode 100644 include/dt-bindings/reset/cix,sky1-rst-fch.h create mode 100644 include/dt-bindings/reset/cix,sky1-rst.hdiff --git a/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml b/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file mode 100644 index 000000000000..a28f938a283d --- /dev/null +++ b/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/reset/cix,sky1-rst.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: CIX Sky1 Reset Controller + +maintainers: + - Gary Yang <gary.yang@cixtech.com> + +description: | + CIX Sky1 reset controller can be used to reset various set of peripherals. + There are two reset controllers, one is located in S0 domain, the other + is located in S5 domain. + + See also: + - include/dt-bindings/reset/cix,sky1-rst.h + +properties: + compatible: + items: + - enum: + - cix,sky1-rst + - cix,sky1-rst-fch
You've not addressed my v2 commentary: https://lore.kernel.org/all/20251114-problem-overbook-383f8e45cd0b@spud/ (local) I asked what else the device does, but you didn't answer me. Dropping the syscon doesn't make sense if the device genuinely has other functions.
+
+ reg:
+ minItems: 1
+ maxItems: 3
+
+ '#reset-cells':
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/reset/cix,sky1-rst.h>
+ reset-controller@16000304 {
+ compatible = "cix,sky1-rst";+ reg = <0x16000304 0xc>, + <0x16000400 0x10>, + <0x16000800 0x8>;
This is also highly suspect, and I believe what you had before was probably much more realistic. Do things properly and fully *now*, rather than pay the price of unravelling it all later. I just did this for one of my own platforms, and putting in the effort to completely describe stuff up front is actually worth it rather than having to refactor years down the line. Cheers, Conor. pw-bot: changes-requested
Attachments
- signature.asc [application/pgp-signature] 228 bytes