Add support for Cix Sky1 resets
Gary Yang
maintainerv3 cover Add support for Cix Sky1 resets
Patch 1: Add yaml file for Cix Sky1 resets
Patch 2: Add driver codes for Cix Sky1 resets
Patch 3: Add dts nodes for Cix Sky1 resets
v3 changes:
- Pass dts build check with below commands:
make O=$OUTKNL dt_binding_check
make O=$OUTKNL dt_binding_check DT_SCHEMA_FILES=cix,sky1-rst.yaml scripts/checkpatch.pl 000*.patch
make O=$OUTKNL CHECK_DTBS=y W=1 cix/sky1-orion-o6.dtb
- remove syscon
v2 changes:
- Pass dts build check with below commands:
make O=$OUTKNL dt_binding_check
make O=$OUTKNL dt_binding_check DT_SCHEMA_FILES=cix,sky1-rst.yaml scripts/checkpatch.pl 000*.patch
make O=$OUTKNL CHECK_DTBS=y W=1 cix/sky1-orion-o6.dtb
- fix dt-bindings style
- refine reset driver
Gary Yang (3):
dt-bindings: reset: add sky1 reset controller
reset: cix: add support for cix sky1 resets
arm64: dts: cix: add support for cix sky1 resets
.../bindings/reset/cix,sky1-rst.yaml | 50 ++ arch/arm64/boot/dts/cix/sky1.dtsi | 14 + drivers/reset/Kconfig | 7 + drivers/reset/Makefile | 1 + drivers/reset/reset-sky1.c | 426 ++++++++++++++++++ include/dt-bindings/reset/cix,sky1-rst-fch.h | 42 ++ include/dt-bindings/reset/cix,sky1-rst.h | 164 +++++++ 7 files changed, 704 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml create mode 100644 drivers/reset/reset-sky1.c create mode 100644 include/dt-bindings/reset/cix,sky1-rst-fch.h create mode 100644 include/dt-bindings/reset/cix,sky1-rst.h
Signature · 1 line
2.49.0
Gary Yang
maintainerPATCH v3 1/3 dt-bindings: reset: add sky1 reset controller
There are two reset controllers on Cix sky1 Soc. One is located in S0 domain, and the other is located in S5 domain.
--- .../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.h
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +50 -0
@@ -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 <[email protected]> + +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 + + 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>; + #reset-cells = <1>; + };
include/dt-bindings/reset/cix,sky1-rst-fch.h new file +42 -0
42 lines not on this page · raw message
include/dt-bindings/reset/cix,sky1-rst.h new file +164 -0
164 lines not on this page · raw message
Signature · 1 line
2.49.0
Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller
Quoted 35 lines · Gary Yang
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 <[email protected]>--- .../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.hDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +15
-0@@ -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 <[email protected]> + +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.
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 25
Quoted 10 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +10
-0+ + 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/
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.
Quoted 14 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +14
-0+ + reg: + minItems: 1 + maxItems: 3 + + '#reset-cells': + const: 1 + +required: + - compatible + - reg + - '#reset-cells' + +additionalProperties: false
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 48
Quoted 10 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +9
-0+ +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
Gary Yang
maintainer回复: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller
Hi Conor:
Thanks for your comments
Quoted 55 lines
-----邮件原件-----
发件人: Conor Dooley <[email protected]>
发送时间: 2025年11月25日 3:54
收件人: Gary Yang <[email protected]>
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; cix-kernel-upstream <[email protected]>
主题: Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controllerQuoted 44 lines · Gary Yang
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 <[email protected]>--- .../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.hDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +23
-0@@ -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 <[email protected]> + +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:
Quoted 9 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 25
Quoted 2 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +2
-0+ - cix,sky1-rst + - cix,sky1-rst-fchYou've not addressed my v2 commentary:
https://lore.kernel.org/all/20251114-problem-overbook-383f8e45cd0b@spud
/
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.
First I'm sorry for not responding your questions earlier. We agree the fact the register space of reset should not depends on other modules. We found that while the reset register spaces on the sky1 platform are non-contiguous, a specific register space among them is exclusively used by reset. So we can remove syscon property and split serval register spaces. All right?
Quoted 32 lines
Quoted 14 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +14
-0+ + reg: + minItems: 1 + maxItems: 3 + + '#reset-cells': + const: 1 + +required: + - compatible + - reg + - '#reset-cells' + +additionalProperties: falseDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 48
Quoted 10 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +9
-0+ +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.
Quoted 3 lines
Cheers,
Conor.
Yes, I agree your view.
This scheme is discussed in our team. It is our decision, not only mine.
There are some modules here that haven't been pushed upstream yet.
If we take them as our internal names, maybe make you confuse. For example,
The register space based 0x16000000 belongs to PMCTRL_S5. It is a system power control module, not SCP. It not only includes reset controller, but also some usb control, wakeup sources, clk gates, sleep states settings, generic registers for software, and so on. But In kernel, we mainly focus on reset controller and usb control. They are controlled by the different registers. So we decide to adopt this scheme.
If you have any questions, please let us know. If make any mistakes, please remind me kindly. Thanks
Best wishes
Gary
Quoted 1 line
pw-bot: changes-requested
Re: 回复: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller
Quoted 69 lines · Gary Yang
Hi Conor:
Thanks for your comments
Quoted 55 lines
-----邮件原件-----
发件人: Conor Dooley <[email protected]>
发送时间: 2025年11月25日 3:54
收件人: Gary Yang <[email protected]>
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; cix-kernel-upstream <[email protected]>
主题: Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controllerQuoted 44 lines · Gary Yang
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 <[email protected]>--- .../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.hDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +23
-0@@ -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 <[email protected]> + +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:Quoted 9 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 25
Quoted 2 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +2
-0+ - cix,sky1-rst + - cix,sky1-rst-fchYou've not addressed my v2 commentary:
https://lore.kernel.org/all/20251114-problem-overbook-383f8e45cd0b@spud
/
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.
Quoted 3 lines
First I'm sorry for not responding your questions earlier. We agree the fact the register space of reset should not depends on other modules. We found that while the reset register spaces on the sky1 platform are non-contiguous, a specific register space among them is exclusively used by reset. So we can remove syscon property and split serval register spaces. All right?
No, not all right, sorry.
It's perfectly okay for some region to do multiple things, most SoCs have multiple regions exactly like this.
The normal thing to do is to treat these regions as a syscon like your earlier version did. The problem with your v1 was that you called the whole thing a reset, when it isn't just that.
There's plenty of examples using mfd for how these kinds of devices are handled in the kernel. There's some using the simple-mfd compatible, which is for when there are subdevices with their own nodes and other defining mfd_cells and calling mfd_add_devices() when the subdevices do not have enough complexity for a node (like your reset controller that has one property and is unlikely to be reusable on another platform).
Quoted 31 lines
Quoted 24 lines
Quoted 13 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +13
-0+ reg: + minItems: 1 + maxItems: 3 + + '#reset-cells': + const: 1 + +required: + - compatible + - reg + - '#reset-cells' + +additionalProperties: falseDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 48
Quoted 10 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +9
-0+ +examples: + - | + #include <dt-bindings/reset/cix,sky1-rst.h> + reset-controller@16000304 { + compatible = "cix,sky1-rst"; + reg = <0x16000304 0xc>, + <0x16000400 0x10>, + <0x16000800 0x8>;Quoted 6 lines
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.
Quoted 4 lines
Yes, I agree your view.
This scheme is discussed in our team. It is our decision, not only mine. There are some modules here that haven't been pushed upstream yet. If we take them as our internal names, maybe make you confuse. For example,
If the naming is going to be confusing, then explain things in the description property.
Quoted 4 lines
The register space based
0x16000000belongs toPMCTRL_S5. It is a system power control module, not SCP. It not only includes reset controller, but also some usb control, wakeup sources, clk gates, sleep states settings, generic registers for software, and so on. But In kernel, we mainly focus on reset controller and usb control. They are controlled by the different registers. So we decide to adopt this scheme.
This is all very normal stuff that syscons are used for on other platforms. Describe the register region based on what it contains, not based on what you currently thing that linux is going to use. Maybe later you'll need the other functions either in linux, or in other projects (like u-boot) that import our devicetrees.
Quoted 1 line
If you have any questions, please let us know. If make any mistakes, please remind me kindly.
Gary Yang
maintainer回复: 回复: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller
Hi Conor:
Thanks for your comments.
Quoted 150 lines
-----邮件原件-----
发件人: Conor Dooley <[email protected]>
发送时间: 2025年11月27日 3:15
收件人: Gary Yang <[email protected]>
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; cix-kernel-upstream <[email protected]>
主题: Re: 回复: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controllerQuoted 23 lines · Gary Yang
Hi Conor:
Thanks for your comments
Quoted 19 lines
-----邮件原件-----
发件人: Conor Dooley <[email protected]>
发送时间: 2025年11月25日 3:54
收件人: Gary Yang <[email protected]>
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; cix-kernel-upstream <[email protected]>
主题: Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controllerQuoted 8 lines · Gary Yang
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 <[email protected]>--- .../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++++++++++++++++++
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
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.yamlb/Documentation/devicetree/bindings/reset/cix,sky1-rst.yamlnew file mode 100644
index 000000000000..a28f938a283dDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +5
-0@@ -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#Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
Quoted 8 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +8
-0+ +title: CIX Sky1 Reset Controller + +maintainers: + - Gary Yang <[email protected]> + +description: | + CIX Sky1 reset controller can be used to reset various set ofperipherals.
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +12
-0+ 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-rstQuoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 25
Quoted 1 line
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +1
-0+ - cix,sky1-rst-fchYou've not addressed my v2 commentary:
https://lore.kernel.org/all/20251114-problem-overbook-383f8e45cd0b@s
pud
/
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 otherfunctions.
Quoted 3 lines
Quoted 1 line
First I'm sorry for not responding your questions earlier. We agree the fact
the register space of reset should not depends on other modules.
Quoted 1 line
We found that while the reset register spaces on the sky1 platform are
non-contiguous, a specific register space among them is exclusively used by reset.
Quoted 1 line
So we can remove syscon property and split serval register spaces. All right?
No, not all right, sorry.
It's perfectly okay for some region to do multiple things, most SoCs have multiple regions exactly like this.
The normal thing to do is to treat these regions as a syscon like your earlier version did. The problem with your v1 was that you called the whole thing a reset, when it isn't just that.
There's plenty of examples using mfd for how these kinds of devices are handled in the kernel. There's some using the simple-mfd compatible, which is for when there are subdevices with their own nodes and other definingmfd_cellsand callingmfd_add_devices()when the subdevices do not have enough complexity for a node (like your reset controller that has one property and is unlikely to be reusable on another platform).Quoted 31 lines
Quoted 24 lines
Quoted 13 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +13
-0+ reg: + minItems: 1 + maxItems: 3 + + '#reset-cells': + const: 1 + +required: + - compatible + - reg + - '#reset-cells' + +additionalProperties: falseDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 48
Quoted 10 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +9
-0+ +examples: + - | + #include <dt-bindings/reset/cix,sky1-rst.h> + reset-controller@16000304 { + compatible = "cix,sky1-rst"; + reg = <0x16000304 0xc>, + <0x16000400 0x10>, + <0x16000800 0x8>;Quoted 6 lines
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.Quoted 5 lines
Yes, I agree your view.
This scheme is discussed in our team. It is our decision, not only mine. There are some modules here that haven't been pushed upstream yet. If we take them as our internal names, maybe make you confuse. For example,If the naming is going to be confusing, then explain things in the description property.
Quoted 1 line
The register space based
0x16000000belongs toPMCTRL_S5. It is a systempower control module, not SCP.
Quoted 2 lines
It not only includes reset controller, but also some usb control, wakeup sources, clk gates, sleep states settings, generic registers for software,
and so on. But In kernel, we mainly focus on reset controller and usb control.
Quoted 9 lines
Quoted 1 line
They are controlled by the different registers. So we decide to adopt this
scheme.
This is all very normal stuff that syscons are used for on other platforms. Describe the register region based on what it contains, not based on what you currently thing that linux is going to use. Maybe later you'll need the other functions either in linux, or in other projects (like u-boot) that import our devicetrees.
Yes，First we agree syscon scheme. It is very common in kernel. We want to verify the next actions with you. One, we add a node syscon@16000000 as pmctrl_s5 an example.
Two, the reset node and usb node both contain a phandle used to point to syscon node Three, In corresponding driver files, we can get the regmap pointers via syscon API.
All right? By the way, How should we describe syscon in yaml file? Are there some files used to refer? Please show us your suggestions. Thanks
Best regards
Gary
Quoted 2 lines
Quoted 1 line
If you have any questions, please let us know. If make any mistakes, please
remind me kindly.
Re: 回复: 回复: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller
Quoted 164 lines · Gary Yang
Hi Conor:
Thanks for your comments.
Quoted 150 lines
-----邮件原件-----
发件人: Conor Dooley <[email protected]>
发送时间: 2025年11月27日 3:15
收件人: Gary Yang <[email protected]>
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; cix-kernel-upstream <[email protected]>
主题: Re: 回复: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controllerQuoted 23 lines · Gary Yang
Hi Conor:
Thanks for your comments
Quoted 19 lines
-----邮件原件-----
发件人: Conor Dooley <[email protected]>
发送时间: 2025年11月25日 3:54
收件人: Gary Yang <[email protected]>
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; cix-kernel-upstream <[email protected]>
主题: Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controllerQuoted 8 lines · Gary Yang
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 <[email protected]>--- .../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++++++++++++++++++
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
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.yamlb/Documentation/devicetree/bindings/reset/cix,sky1-rst.yamlnew file mode 100644
index 000000000000..a28f938a283dDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +5
-0@@ -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#Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
Quoted 8 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +8
-0+ +title: CIX Sky1 Reset Controller + +maintainers: + - Gary Yang <[email protected]> + +description: | + CIX Sky1 reset controller can be used to reset various set ofperipherals.
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +12
-0+ 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-rstQuoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 25
Quoted 1 line
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +1
-0+ - cix,sky1-rst-fchYou've not addressed my v2 commentary:
https://lore.kernel.org/all/20251114-problem-overbook-383f8e45cd0b@s
pud
/
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 otherfunctions.
Quoted 3 lines
Quoted 1 line
First I'm sorry for not responding your questions earlier. We agree the fact
the register space of reset should not depends on other modules.
Quoted 1 line
We found that while the reset register spaces on the sky1 platform are
non-contiguous, a specific register space among them is exclusively used by reset.
Quoted 1 line
So we can remove syscon property and split serval register spaces. All right?
No, not all right, sorry.
It's perfectly okay for some region to do multiple things, most SoCs have multiple regions exactly like this.
The normal thing to do is to treat these regions as a syscon like your earlier version did. The problem with your v1 was that you called the whole thing a reset, when it isn't just that.
There's plenty of examples using mfd for how these kinds of devices are handled in the kernel. There's some using the simple-mfd compatible, which is for when there are subdevices with their own nodes and other definingmfd_cellsand callingmfd_add_devices()when the subdevices do not have enough complexity for a node (like your reset controller that has one property and is unlikely to be reusable on another platform).Quoted 31 lines
Quoted 24 lines
Quoted 13 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +13
-0+ reg: + minItems: 1 + maxItems: 3 + + '#reset-cells': + const: 1 + +required: + - compatible + - reg + - '#reset-cells' + +additionalProperties: falseDocumentation/devicetree/bindings/reset/cix,sky1-rst.yaml line 48
Quoted 10 lines
Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml new file +9
-0+ +examples: + - | + #include <dt-bindings/reset/cix,sky1-rst.h> + reset-controller@16000304 { + compatible = "cix,sky1-rst"; + reg = <0x16000304 0xc>, + <0x16000400 0x10>, + <0x16000800 0x8>;Quoted 6 lines
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.Quoted 5 lines
Yes, I agree your view.
This scheme is discussed in our team. It is our decision, not only mine. There are some modules here that haven't been pushed upstream yet. If we take them as our internal names, maybe make you confuse. For example,If the naming is going to be confusing, then explain things in the description property.
Quoted 1 line
The register space based
0x16000000belongs toPMCTRL_S5. It is a systempower control module, not SCP.
Quoted 2 lines
It not only includes reset controller, but also some usb control, wakeup sources, clk gates, sleep states settings, generic registers for software,
and so on. But In kernel, we mainly focus on reset controller and usb control.
Quoted 9 lines
Quoted 1 line
They are controlled by the different registers. So we decide to adopt this
scheme.
This is all very normal stuff that syscons are used for on other platforms. Describe the register region based on what it contains, not based on what you currently thing that linux is going to use. Maybe later you'll need the other functions either in linux, or in other projects (like u-boot) that import our devicetrees.
Quoted 3 lines
Yes，First we agree syscon scheme. It is very common in kernel. We want to verify the next actions with you. One, we add a node syscon@16000000 as
pmctrl_s5an example.
Two, the reset node and usb node both contain a phandle used to point to syscon node
Not quite. The rest node would be empty, other than the phandle, and is therefore pointless. The #reset-cells property should go in the syscon node itself. For usb, because it sounds like the syscon is only providing some control registers and is not the usb controller itself, a phandle is okay. It's a case by case thing, but if the syscon node provides an entire feature then a phandle is not suitable and instead either a child node of the syscon (which would make it a simple-mfd too) should be used or the properties should go directly into the syscon node. That depends on how complex the feature is and how likely it is to be reused between devices. reset is very unlikely to be suitable for reuse, for example.
Quoted 2 lines
Three, In corresponding driver files, we can get the regmap pointers via syscon API. All right? By the way, How should we describe syscon in yaml file? Are there some files used to refer?
microchip,mpfs-mss-top-sysreg.yaml is one I wrote recently. Going to the corresponding driver you will be able to see how the reset controller driver is probed. You should be able to find it in linux-next. sophgo,sg2044-top-syscon.yaml is another that works in the same way.
Quoted 7 lines
Please show us your suggestions. Thanks
Best regards
GaryQuoted 2 lines
Quoted 1 line
If you have any questions, please let us know. If make any mistakes, please
remind me kindly.
Gary Yang
maintainerPATCH v3 2/3 reset: cix: add support for cix sky1 resets
In reply to Gary YangThere are two reset controllers on Cix Sky1 Soc. One is located in S0 domain, and the other is located in S5 domain.
--- drivers/reset/Kconfig | 7 + drivers/reset/Makefile | 1 + drivers/reset/reset-sky1.c | 426 +++++++++++++++++++++++++++++++++++++ 3 files changed, 434 insertions(+) create mode 100644 drivers/reset/reset-sky1.c
drivers/reset/Kconfig +7 -0
@@ -278,6 +278,13 @@ config RESET_SIMPLE - SiFive FU740 SoCs - Sophgo SoCs +config RESET_SKY1 + bool "Cix Sky1 reset controller" + depends on HAS_IOMEM + depends on ARCH_CIX || COMPILE_TEST + help + This enables the reset controller for Cix Sky1. + config RESET_SOCFPGA bool "SoCFPGA Reset Driver" if COMPILE_TEST && (!ARM || !ARCH_INTEL_SOCFPGA) default ARM && ARCH_INTEL_SOCFPGA
drivers/reset/Makefile +1 -0
@@ -36,6 +36,7 @@ obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o obj-$(CONFIG_RESET_RZV2H_USB2PHY) += reset-rzv2h-usb2phy.o obj-$(CONFIG_RESET_SCMI) += reset-scmi.o obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o +obj-$(CONFIG_RESET_SKY1) += reset-sky1.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_RESET_SPACEMIT) += reset-spacemit.o obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
drivers/reset/reset-sky1.c new file +426 -0
426 lines not on this page · raw message
Signature · 1 line
2.49.0
Gary Yang
maintainerPATCH v3 3/3 arm64: dts: cix: add support for cix sky1 resets
In reply to Gary YangThere are two reset conctrollers on Cix Sky1 Soc. One is located in S0 domain, and the other is located in S5 domain.
--- arch/arm64/boot/dts/cix/sky1.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
arch/arm64/boot/dts/cix/sky1.dtsi +14 -0
@@ -348,6 +348,12 @@ i3c1: i3c@4100000 { status = "disabled"; }; + src_fch: reset-controller@4160008 { + compatible = "cix,sky1-rst-fch"; + reg = <0x0 0x04160008 0x0 0xc>; + #reset-cells = <1>; + }; + iomuxc: pinctrl@4170000 { compatible = "cix,sky1-iomuxc"; reg = <0x0 0x04170000 0x0 0x1000>;
@@ -568,6 +574,14 @@ ppi_partition1: interrupt-partition-1 { }; }; + src: reset-controller@16000304 { + compatible = "cix,sky1-rst"; + reg = <0x0 0x16000304 0x0 0xc>, + <0x0 0x16000400 0x0 0x10>, + <0x0 0x16000800 0x0 0x8>; + #reset-cells = <1>; + }; + iomuxc_s5: pinctrl@16007000 { compatible = "cix,sky1-iomuxc-s5"; reg = <0x0 0x16007000 0x0 0x1000>;
Signature · 1 line
2.49.0