[PATCH 0/4] reset: introduct generic bit reset controller

[PATCH 0/4] reset: introduct generic bit reset controller

[Inochi Amaoto]

Many SoC have a simple reset controller with toggling bit to perform
assert/deassert. It is annoyed to add compatiable for each device when
coming a new SoC. It is necessary to introduct a generic device to
simplify these things.

Add a simple generic bit reset controller, and migrate the Sophgo
SG2042 reset controller as an example.

Inochi Amaoto (4):
  dt-bindings: reset: add generic bit reset controller
  reset: simple: add support generic reset-simple device
  dt-bindings: reset: simple: migrate sophgo sg2042 reset controller
  riscv: dts: sg2042: Adapt reset generator for new binding

 .../bindings/reset/reset-simple.yaml          | 46 +++++++++++++++++++
 .../bindings/reset/sophgo,sg2042-reset.yaml   | 35 --------------
 arch/riscv/boot/dts/sophgo/sg2042.dtsi        |  2 +-
 drivers/reset/reset-simple.c                  |  3 ++
 4 files changed, 50 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reset/reset-simple.yaml
 delete mode 100644 Documentation/devicetree/bindings/reset/sophgo,sg2042-reset.yaml

--
2.48.1

[PATCH 1/4] dt-bindings: reset: add generic bit reset controller

[Inochi Amaoto]

Some SoCs from Aspeed, Allwinner, Sophgo and Synopsys have
a simple reset controller by toggling bit. It is a hard time
for each device to add its own compatible to the driver.
Since this device share a common design, it is possible to
add a common device to reduce these unnecessary change.

Add common binding for these kind generic reset controller.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../bindings/reset/reset-simple.yaml          | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/reset-simple.yaml

diff --git a/Documentation/devicetree/bindings/reset/reset-simple.yaml b/Documentation/devicetree/bindings/reset/reset-simple.yaml
new file mode 100644
index 000000000000..77584e23e8e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/reset-simple.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/reset-simple.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic BIT Reset Controller
+
+maintainers:
+  - Inochi Amaoto <inochiama@gmail.com>
+
+description:
+  Some reset controller devices uses a simple method to perform
+  assert/deassert by toggling bit. Some SoCs from Aspeed, Allwinner,
+  Sophgo and Synopsys have this kind of reset controller instances.
+
+properties:
+  compatible:
+    enum:
+      - reset-simple-high
+      - reset-simple-low
+
+  reg:
+    maxItems: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    reset-controller@1000000 {
+        compatible = "reset-simple-high";
+        reg = <0x1000000 0x1000>;
+        #reset-cells = <1>;
+    };
-- 
2.48.1

Re: [PATCH 1/4] dt-bindings: reset: add generic bit reset controller

[Krzysztof Kozlowski]

On Thu, Feb 13, 2025 at 10:08:54AM +0800, Inochi Amaoto wrote:
> Some SoCs from Aspeed, Allwinner, Sophgo and Synopsys have
> a simple reset controller by toggling bit. It is a hard time
> for each device to add its own compatible to the driver.
> Since this device share a common design, it is possible to
> add a common device to reduce these unnecessary change.

SoC components are rarely that simple and even if it is just a bit,
usually it is part of one or few registers.

Anyway, there are already bindings for reset-simple and I do not
understand why this has to be duplicated.

> 
> Add common binding for these kind generic reset controller.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../bindings/reset/reset-simple.yaml          | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/reset-simple.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/reset-simple.yaml b/Documentation/devicetree/bindings/reset/reset-simple.yaml
> new file mode 100644
> index 000000000000..77584e23e8e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/reset-simple.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/reset-simple.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic BIT Reset Controller
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@gmail.com>
> +
> +description:
> +  Some reset controller devices uses a simple method to perform
> +  assert/deassert by toggling bit. Some SoCs from Aspeed, Allwinner,
> +  Sophgo and Synopsys have this kind of reset controller instances.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - reset-simple-high
> +      - reset-simple-low

It would be one compatible and set of properties describing resets.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: reset: add generic bit reset controller

[Philipp Zabel]

On Do, 2025-02-13 at 10:35 +0100, Krzysztof Kozlowski wrote:
> On Thu, Feb 13, 2025 at 10:08:54AM +0800, Inochi Amaoto wrote:
> > Some SoCs from Aspeed, Allwinner, Sophgo and Synopsys have
> > a simple reset controller by toggling bit. It is a hard time
> > for each device to add its own compatible to the driver.
> > Since this device share a common design, it is possible to
> > add a common device to reduce these unnecessary change.
> 
> SoC components are rarely that simple and even if it is just a bit,
> usually it is part of one or few registers.

Yes, in those cases (which are probably most of them), I would argue
this binding doesn't really fit.

> Anyway, there are already bindings for reset-simple and I do not
> understand why this has to be duplicated.

I think the motivation is to not have to add a new binding document and
modify reset-simple.c every time there is a new SoC. I wonder if some
of this can be mitigated by adding just the binding document similar to
trivial-devices.yaml, without the actual "reset-simple" compatible.

> 
regards
Philipp

Re: [PATCH 1/4] dt-bindings: reset: add generic bit reset controller

[Inochi Amaoto]

On Thu, Feb 13, 2025 at 11:22:07AM +0100, Philipp Zabel wrote:
> On Do, 2025-02-13 at 10:35 +0100, Krzysztof Kozlowski wrote:
> > On Thu, Feb 13, 2025 at 10:08:54AM +0800, Inochi Amaoto wrote:
> > > Some SoCs from Aspeed, Allwinner, Sophgo and Synopsys have
> > > a simple reset controller by toggling bit. It is a hard time
> > > for each device to add its own compatible to the driver.
> > > Since this device share a common design, it is possible to
> > > add a common device to reduce these unnecessary change.
> > 
> > SoC components are rarely that simple and even if it is just a bit,
> > usually it is part of one or few registers.
> 
> Yes, in those cases (which are probably most of them), I would argue
> this binding doesn't really fit.
> 

Yes, I agree. 

> > Anyway, there are already bindings for reset-simple and I do not
> > understand why this has to be duplicated.
> 
> I think the motivation is to not have to add a new binding document and
> modify reset-simple.c every time there is a new SoC. 

Yeah, this is the motivation for me to write this patch. It seems that I
describe it in a wrong way.

> I wonder if some of this can be mitigated by adding just the 
> binding document similar to trivial-devices.yaml, without the
> actual "reset-simple" compatible.
> 

It is better to keep the original compatible. Adding new base compatible
will break existing device and make the migration hard.

Regards,
Inochi

Re: [PATCH 1/4] dt-bindings: reset: add generic bit reset controller

[Philipp Zabel]

On Do, 2025-02-13 at 10:08 +0800, Inochi Amaoto wrote:
> Some SoCs from Aspeed, Allwinner, Sophgo and Synopsys have
> a simple reset controller by toggling bit. It is a hard time
> for each device to add its own compatible to the driver.
> Since this device share a common design, it is possible to
> add a common device to reduce these unnecessary change.
> 
> Add common binding for these kind generic reset controller.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../bindings/reset/reset-simple.yaml          | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/reset-simple.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/reset-simple.yaml b/Documentation/devicetree/bindings/reset/reset-simple.yaml
> new file mode 100644
> index 000000000000..77584e23e8e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/reset-simple.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/reset-simple.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic BIT Reset Controller
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@gmail.com>
> +
> +description:
> +  Some reset controller devices uses a simple method to perform
> +  assert/deassert by toggling bit. Some SoCs from Aspeed, Allwinner,
> +  Sophgo and Synopsys have this kind of reset controller instances.

I think some properties should be documented that make reset
controllers "simple" according to this binding.

For example, right now, the reset-simple driver assumes the following:

  - There is a single, contiguous range of 32-bit registers.
  - All bits in each register directly control a reset line.
     - There are no self-deasserting resets.
     - There are no timing requirements.
     - The bits are exclusively resets, nothing else.
  - All bits behave the same, so all reset bits are either
    active-high or all are active-low.
  - The bits can be read back, but the read status may
    be active-low independently from the writes.

> +properties:
> +  compatible:
> +    enum:
> +      - reset-simple-high
> +      - reset-simple-low

I wonder if it would be better to have a single

  compatible:
    const: reset-simple

and a boolean property, e.g.

  active-low:
    type: boolean

like in leds/common.yaml. Also it should be documented clearly what
this means for reads and writes.

> +  reg:
> +    maxItems: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    reset-controller@1000000 {
> +        compatible = "reset-simple-high";

The example should probably include a SoC specific compatible?

regards
Philipp

Re: [PATCH 1/4] dt-bindings: reset: add generic bit reset controller

[Inochi Amaoto]

On Thu, Feb 13, 2025 at 11:08:59AM +0100, Philipp Zabel wrote:
> On Do, 2025-02-13 at 10:08 +0800, Inochi Amaoto wrote:
> > Some SoCs from Aspeed, Allwinner, Sophgo and Synopsys have
> > a simple reset controller by toggling bit. It is a hard time
> > for each device to add its own compatible to the driver.
> > Since this device share a common design, it is possible to
> > add a common device to reduce these unnecessary change.
> > 
> > Add common binding for these kind generic reset controller.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  .../bindings/reset/reset-simple.yaml          | 42 +++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/reset/reset-simple.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/reset/reset-simple.yaml b/Documentation/devicetree/bindings/reset/reset-simple.yaml
> > new file mode 100644
> > index 000000000000..77584e23e8e8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/reset-simple.yaml
> > @@ -0,0 +1,42 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reset/reset-simple.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic BIT Reset Controller
> > +
> > +maintainers:
> > +  - Inochi Amaoto <inochiama@gmail.com>
> > +
> > +description:
> > +  Some reset controller devices uses a simple method to perform
> > +  assert/deassert by toggling bit. Some SoCs from Aspeed, Allwinner,
> > +  Sophgo and Synopsys have this kind of reset controller instances.
> 
> I think some properties should be documented that make reset
> controllers "simple" according to this binding.
> 
> For example, right now, the reset-simple driver assumes the following:
> 
>   - There is a single, contiguous range of 32-bit registers.
>   - All bits in each register directly control a reset line.
>      - There are no self-deasserting resets.
>      - There are no timing requirements.
>      - The bits are exclusively resets, nothing else.
>   - All bits behave the same, so all reset bits are either
>     active-high or all are active-low.
>   - The bits can be read back, but the read status may
>     be active-low independently from the writes.
> 

Thanks, I will add these assumes in to the binding.

> > +properties:
> > +  compatible:
> > +    enum:
> > +      - reset-simple-high
> > +      - reset-simple-low
> 
> I wonder if it would be better to have a single
> 
>   compatible:
>     const: reset-simple
> 
> and a boolean property, e.g.
> 
>   active-low:
>     type: boolean
> 
> like in leds/common.yaml. Also it should be documented clearly what
> this means for reads and writes.
> 

Yeah, it is better to have a property instead of defining a base
compatible. With this property, there are two ways to process this
property with existing device:

1. If device has defined the data, the active-low is ignored
2. If device has defined the data, the active-low will overwrite
   the device data.

I wonder which one is better?

> > +  reg:
> > +    maxItems: 1
> > +
> > +  "#reset-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#reset-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    reset-controller@1000000 {
> > +        compatible = "reset-simple-high";
> 
> The example should probably include a SoC specific compatible?
> 
> regards
> Philipp

I think it is OK. But I think it should be added when a
specific compatible is coming.

Regards,
Inochi

[PATCH 2/4] reset: simple: add support generic reset-simple device

[Inochi Amaoto]

Add support for generic bit reset device.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/reset/reset-simple.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 276067839830..a24f8fcc6a06 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -148,6 +148,9 @@ static const struct of_device_id reset_simple_dt_ids[] = {
 		.data = &reset_simple_active_low },
 	{ .compatible = "brcm,bcm4908-misc-pcie-reset",
 		.data = &reset_simple_active_low },
+	{ .compatible = "reset-simple-high" },
+	{ .compatible = "reset-simple-low",
+		.data = &reset_simple_active_low },
 	{ .compatible = "snps,dw-high-reset" },
 	{ .compatible = "snps,dw-low-reset",
 		.data = &reset_simple_active_low },
-- 
2.48.1

Re: [PATCH 2/4] reset: simple: add support generic reset-simple device

[Krzysztof Kozlowski]

On Thu, Feb 13, 2025 at 10:08:55AM +0800, Inochi Amaoto wrote:
> Add support for generic bit reset device.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  drivers/reset/reset-simple.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 276067839830..a24f8fcc6a06 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -148,6 +148,9 @@ static const struct of_device_id reset_simple_dt_ids[] = {
>  		.data = &reset_simple_active_low },
>  	{ .compatible = "brcm,bcm4908-misc-pcie-reset",
>  		.data = &reset_simple_active_low },
> +	{ .compatible = "reset-simple-high" },
> +	{ .compatible = "reset-simple-low",
> +		.data = &reset_simple_active_low },

So what is the point of the binding?

Best regards,
Krzysztof

Re: [PATCH 2/4] reset: simple: add support generic reset-simple device

[Inochi Amaoto]

On Thu, Feb 13, 2025 at 10:36:14AM +0100, Krzysztof Kozlowski wrote:
> On Thu, Feb 13, 2025 at 10:08:55AM +0800, Inochi Amaoto wrote:
> > Add support for generic bit reset device.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  drivers/reset/reset-simple.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> > index 276067839830..a24f8fcc6a06 100644
> > --- a/drivers/reset/reset-simple.c
> > +++ b/drivers/reset/reset-simple.c
> > @@ -148,6 +148,9 @@ static const struct of_device_id reset_simple_dt_ids[] = {
> >  		.data = &reset_simple_active_low },
> >  	{ .compatible = "brcm,bcm4908-misc-pcie-reset",
> >  		.data = &reset_simple_active_low },
> > +	{ .compatible = "reset-simple-high" },
> > +	{ .compatible = "reset-simple-low",
> > +		.data = &reset_simple_active_low },
> 
> So what is the point of the binding?
> 

I wanted to add these comptaibles as a base to avoid adding
too many compatible in driver.

Regards,
Inochi

[PATCH 3/4] dt-bindings: reset: simple: migrate sophgo sg2042 reset controller

[Inochi Amaoto]

Migrate SG2042 reset controller to reuse the simple device binding.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../bindings/reset/reset-simple.yaml          | 10 ++++--
 .../bindings/reset/sophgo,sg2042-reset.yaml   | 35 -------------------
 2 files changed, 7 insertions(+), 38 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/reset/sophgo,sg2042-reset.yaml

diff --git a/Documentation/devicetree/bindings/reset/reset-simple.yaml b/Documentation/devicetree/bindings/reset/reset-simple.yaml
index 77584e23e8e8..1c86b67ae190 100644
--- a/Documentation/devicetree/bindings/reset/reset-simple.yaml
+++ b/Documentation/devicetree/bindings/reset/reset-simple.yaml
@@ -16,9 +16,13 @@ description:
 
 properties:
   compatible:
-    enum:
-      - reset-simple-high
-      - reset-simple-low
+    oneOf:
+      - items:
+          - enum:
+              - sophgo,sg2042-reset
+          - const: reset-simple-low
+      - const: reset-simple-high
+      - const: reset-simple-low
 
   reg:
     maxItems: 1
diff --git a/Documentation/devicetree/bindings/reset/sophgo,sg2042-reset.yaml b/Documentation/devicetree/bindings/reset/sophgo,sg2042-reset.yaml
deleted file mode 100644
index 76e1931f0908..000000000000
--- a/Documentation/devicetree/bindings/reset/sophgo,sg2042-reset.yaml
+++ /dev/null
@@ -1,35 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/reset/sophgo,sg2042-reset.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Sophgo SG2042 SoC Reset Controller
-
-maintainers:
-  - Chen Wang <unicorn_wang@outlook.com>
-
-properties:
-  compatible:
-    const: sophgo,sg2042-reset
-
-  reg:
-    maxItems: 1
-
-  "#reset-cells":
-    const: 1
-
-required:
-  - compatible
-  - reg
-  - "#reset-cells"
-
-additionalProperties: false
-
-examples:
-  - |
-    rstgen: reset-controller@c00 {
-        compatible = "sophgo,sg2042-reset";
-        reg = <0xc00 0xc>;
-        #reset-cells = <1>;
-    };
-- 
2.48.1

[PATCH 4/4] riscv: dts: sg2042: Adapt reset generator for new binding

[Inochi Amaoto]

As the SG2042 reset controller reuse new binding, change the device
compatible with new string.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 arch/riscv/boot/dts/sophgo/sg2042.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
index e62ac51ac55a..1e29f5975af9 100644
--- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
+++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
@@ -499,7 +499,7 @@ intc: interrupt-controller@7090000000 {
 		};
 
 		rstgen: reset-controller@7030013000 {
-			compatible = "sophgo,sg2042-reset";
+			compatible = "sophgo,sg2042-reset", "reset-simple-low";
 			reg = <0x00000070 0x30013000 0x00000000 0x0000000c>;
 			#reset-cells = <1>;
 		};
-- 
2.48.1

Re: [PATCH 0/4] reset: introduct generic bit reset controller

[Philipp Zabel]

On Do, 2025-02-13 at 10:08 +0800, Inochi Amaoto wrote:
> Many SoC have a simple reset controller with toggling bit to perform
> assert/deassert. It is annoyed to add compatiable for each device when
> coming a new SoC. It is necessary to introduct a generic device to
> simplify these things.

You'll still have to add a compatible to reset-simple.yaml for each new
SoC, see patch 3.

regards
Philipp

Re: [PATCH 0/4] reset: introduct generic bit reset controller

[Inochi Amaoto]

On Thu, Feb 13, 2025 at 11:08:03AM +0100, Philipp Zabel wrote:
> On Do, 2025-02-13 at 10:08 +0800, Inochi Amaoto wrote:
> > Many SoC have a simple reset controller with toggling bit to perform
> > assert/deassert. It is annoyed to add compatiable for each device when
> > coming a new SoC. It is necessary to introduct a generic device to
> > simplify these things.
> 
> You'll still have to add a compatible to reset-simple.yaml for each new
> SoC, see patch 3.
> 
> regards
> Philipp

Yeah, the binding is can not avoided, but it can avoided adding new
compatible in the driver. I will update the description.

Regards,
Inochi