[RFC PATCH 0/3] Introduce reset driver for T-HEAD th1520 SoC

[RFC PATCH 0/3] Introduce reset driver for T-HEAD th1520 SoC

[Kwanghoon Son]

This patchset adds initial support for reset driver.
Register information is from vendor kernel [1].
I sent an e-mail to get permission the original author,
but there was no answer. So I upload patch since it has GPL license.

This patch also can be tested with watchdog simply with 
```
		watchdog0: watchdog@ffefc30000 {
			compatible = "snps,dw-wdt";
			reg = <0xff 0xefc30000 0x0 0x1000>;
			interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
			clocks = <&osc>;
			resets = <&rst TH1520_RESET_WDT0>;
			status = "disabled";
		};
```

[1] https://github.com/revyos/thead-kernel.git

Kwanghoon Son (3):
  dt-bindings: reset: Document th1520 reset control
  reset: Add th1520 reset driver support
  riscv: dts: Add th1520 reset device tree

 .../bindings/reset/thead,th1520-reset.yaml    |  39 +++++++
 .../dts/thead/th1520-lichee-module-4a.dtsi    |   4 +
 arch/riscv/boot/dts/thead/th1520.dtsi         |   8 ++
 drivers/reset/Kconfig                         |  10 ++
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-th1520.c                  | 109 ++++++++++++++++++
 include/dt-bindings/reset/th1520-reset.h      |   9 ++
 7 files changed, 180 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
 create mode 100644 drivers/reset/reset-th1520.c
 create mode 100644 include/dt-bindings/reset/th1520-reset.h

-- 
2.34.1

[RFC PATCH 1/3] dt-bindings: reset: Document th1520 reset control

[Kwanghoon Son]

Add documentation to describe th1520 reset driver

Signed-off-by: Kwanghoon Son <k.son@samsung.com>
---
 .../bindings/reset/thead,th1520-reset.yaml    | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml

diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
new file mode 100644
index 000000000000..a56d77af464a
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/thead,th1520-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: T-HEAD th1520 SoC Reset Controller
+
+maintainers:
+  - Kwanghoon Son <k.son@samsung.com>
+
+properties:
+  compatible:
+    - items:
+        - const: thead,th1520-reset
+        - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#reset-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/reset/th1520-reset.h>
+
+    reset-controller@ffef014000 {
+        compatible = "thead,th1520-reset", "syscon";
+        reg = <0xff 0xef014000 0x0 0x1000>;
+        #reset-cells = <1>;
+    };
-- 
2.34.1

Re: [RFC PATCH 1/3] dt-bindings: reset: Document th1520 reset control

[Rob Herring]

On Mon, 04 Sep 2023 04:25:57 +0000, Kwanghoon Son wrote:
> Add documentation to describe th1520 reset driver
> 
> Signed-off-by: Kwanghoon Son <k.son@samsung.com>
> ---
>  .../bindings/reset/thead,th1520-reset.yaml    | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Traceback (most recent call last):
  File "/usr/local/bin/dt-doc-validate", line 64, in <module>
    ret |= check_doc(f)
           ^^^^^^^^^^^^
  File "/usr/local/bin/dt-doc-validate", line 32, in check_doc
    for error in sorted(dtsch.iter_errors(), key=lambda e: e.linecol):
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 135, in iter_errors
    self.annotate_error(scherr, meta_schema, scherr.schema_path)
  File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 114, in annotate_error
    schema = schema[p]
             ~~~~~~^^^
KeyError: 'type'
Documentation/devicetree/bindings/reset/thead,th1520-reset.example.dts:18:18: fatal error: dt-bindings/reset/th1520-reset.h: No such file or directory
   18 |         #include <dt-bindings/reset/th1520-reset.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/reset/thead,th1520-reset.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1500: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230904042559.2322997-2-k.son@samsung.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [RFC PATCH 1/3] dt-bindings: reset: Document th1520 reset control

[Krzysztof Kozlowski]

On 04/09/2023 06:25, Kwanghoon Son wrote:
> Add documentation to describe th1520 reset driver

Bindings are for hardware, not drivers.

> 
> Signed-off-by: Kwanghoon Son <k.son@samsung.com>
> ---
>  .../bindings/reset/thead,th1520-reset.yaml    | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> new file mode 100644
> index 000000000000..a56d77af464a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/thead,th1520-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: T-HEAD th1520 SoC Reset Controller
> +
> +maintainers:
> +  - Kwanghoon Son <k.son@samsung.com>
> +
> +properties:
> +  compatible:
> +    - items:

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

Drop '-'

Best regards,
Krzysztof

[RFC PATCH 2/3] reset: Add th1520 reset driver support

[Kwanghoon Son]

This driver supports th1520 T-HEAD SoC reset platform device.

Signed-off-by: Kwanghoon Son <k.son@samsung.com>
---
 drivers/reset/Kconfig                    |  10 +++
 drivers/reset/Makefile                   |   1 +
 drivers/reset/reset-th1520.c             | 109 +++++++++++++++++++++++
 include/dt-bindings/reset/th1520-reset.h |   9 ++
 4 files changed, 129 insertions(+)
 create mode 100644 drivers/reset/reset-th1520.c
 create mode 100644 include/dt-bindings/reset/th1520-reset.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index ccd59ddd7610..ec69e6bbba6e 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -253,6 +253,16 @@ config RESET_SUNXI
 	help
 	  This enables the reset driver for Allwinner SoCs.
 
+config RESET_TH1520
+	bool "TH1520 Reset Driver"
+	depends on (ARCH_THEAD || COMPILE_TEST) && OF
+	select MFD_SYSCON
+	default ARCH_THEAD
+	help
+	  Support for the T-HEAD 1520 RISC-V SoC reset controller.
+	  Say Y if you want to control reset signals provided by this
+	  controller.
+
 config RESET_TI_SCI
 	tristate "TI System Control Interface (TI-SCI) reset driver"
 	depends on TI_SCI_PROTOCOL || (COMPILE_TEST && TI_SCI_PROTOCOL=n)
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 8270da8a4baa..5c858e62241a 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
 obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
+obj-$(CONFIG_RESET_TH1520) += reset-th1520.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
 obj-$(CONFIG_RESET_TI_TPS380X) += reset-tps380x.o
diff --git a/drivers/reset/reset-th1520.c b/drivers/reset/reset-th1520.c
new file mode 100644
index 000000000000..4c781377ac23
--- /dev/null
+++ b/drivers/reset/reset-th1520.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/regmap.h>
+#include <dt-bindings/reset/th1520-reset.h>
+
+struct th1520_rst_signal {
+	unsigned int offset, bit;
+};
+
+struct th1520_rst {
+	struct reset_controller_dev rcdev;
+	struct regmap *regmap;
+	const struct th1520_rst_signal *signals;
+};
+
+enum th1520_rst_registers {
+	RST_WDT0 = 0x0034,
+	RST_WDT1 = 0x0038,
+};
+
+static int th1520_reset_update(struct th1520_rst *rst, unsigned long id,
+			       unsigned int value)
+{
+	const struct th1520_rst_signal *signal = &rst->signals[id];
+
+	return regmap_update_bits(rst->regmap, signal->offset, signal->bit,
+				  value);
+}
+
+static const struct th1520_rst_signal th1520_rst_signals[] = {
+	[TH1520_RESET_WDT0] = { RST_WDT0, BIT(0) },
+	[TH1520_RESET_WDT1] = { RST_WDT1, BIT(0) },
+};
+
+static struct th1520_rst *to_th1520_rst(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct th1520_rst, rcdev);
+}
+
+static int th1520_reset_set(struct reset_controller_dev *rcdev,
+			    unsigned long id, bool assert)
+{
+	struct th1520_rst *rst = to_th1520_rst(rcdev);
+	const unsigned int bit = rst->signals[id].bit;
+	unsigned int value = assert ? bit : 0;
+
+	return th1520_reset_update(rst, id, value);
+}
+
+static int th1520_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	return th1520_reset_set(rcdev, id, false);
+}
+
+static int th1520_reset_deassert(struct reset_controller_dev *rcdev,
+				 unsigned long id)
+{
+	return th1520_reset_set(rcdev, id, true);
+}
+
+static const struct reset_control_ops th1520_rst_ops = {
+	.assert = th1520_reset_assert,
+	.deassert = th1520_reset_deassert,
+};
+
+static int th1520_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct th1520_rst *rst;
+	struct regmap_config config = { .name = "rst" };
+
+	rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
+	if (!rst)
+		return -ENOMEM;
+
+	rst->signals = th1520_rst_signals;
+	rst->regmap = syscon_node_to_regmap(dev->of_node);
+	if (IS_ERR(rst->regmap))
+		return PTR_ERR(rst->regmap);
+
+	regmap_attach_dev(dev, rst->regmap, &config);
+
+	rst->rcdev.owner = THIS_MODULE;
+	rst->rcdev.dev = dev;
+	rst->rcdev.of_node = dev->of_node;
+	rst->rcdev.ops = &th1520_rst_ops;
+	rst->rcdev.nr_resets = ARRAY_SIZE(th1520_rst_signals);
+
+	return devm_reset_controller_register(dev, &rst->rcdev);
+}
+
+static const struct of_device_id th1520_reset_dt_ids[] = {
+	{ .compatible = "thead,th1520-reset" },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver th1520_reset_driver = {
+	.probe	= th1520_reset_probe,
+	.driver = {
+		.name		= "th1520-reset",
+		.of_match_table	= th1520_reset_dt_ids,
+	},
+};
+builtin_platform_driver(th1520_reset_driver);
diff --git a/include/dt-bindings/reset/th1520-reset.h b/include/dt-bindings/reset/th1520-reset.h
new file mode 100644
index 000000000000..ec10751814e5
--- /dev/null
+++ b/include/dt-bindings/reset/th1520-reset.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef DT_BINDING_RESET_TH1520_H
+#define DT_BINDING_RESET_TH1520_H
+
+#define TH1520_RESET_WDT0 0
+#define TH1520_RESET_WDT1 1
+
+#endif
-- 
2.34.1

Re: [RFC PATCH 2/3] reset: Add th1520 reset driver support

[Krzysztof Kozlowski]

On 04/09/2023 06:25, Kwanghoon Son wrote:
> This driver supports th1520 T-HEAD SoC reset platform device.
> 
> Signed-off-by: Kwanghoon Son <k.son@samsung.com>
> ---
>  drivers/reset/Kconfig                    |  10 +++
>  drivers/reset/Makefile                   |   1 +
>  drivers/reset/reset-th1520.c             | 109 +++++++++++++++++++++++
>  include/dt-bindings/reset/th1520-reset.h |   9 ++

Bindings are always separate patches. In this case, all bindings should
be together. Also you clearly forgot one step:

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.

Best regards,
Krzysztof

RE: [RFC PATCH 2/3] reset: Add th1520 reset driver support

[Kwanghoon Son]

> >  drivers/reset/Kconfig                    |  10 +++
> >  drivers/reset/Makefile                   |   1 +
> >  drivers/reset/reset-th1520.c             | 109 +++++++++++++++++++++++
> >  include/dt-bindings/reset/th1520-reset.h |   9 ++
> 
> Bindings are always separate patches. In this case, all bindings should
> be together. Also you clearly forgot one step:

Thanks for the feedback. Does it mean 2 files
`include/dt-bindings/reset/th1520-reset.h`, `Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml`
Should be one patch first together?

Kwnag.

[RFC PATCH 3/3] riscv: dts: Add th1520 reset device tree

[Kwanghoon Son]

Add reset device tree for th1520 SoC

Signed-off-by: Kwanghoon Son <k.son@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi | 4 ++++
 arch/riscv/boot/dts/thead/th1520.dtsi                  | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
index a802ab110429..b689cc049ad8 100644
--- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
@@ -36,3 +36,7 @@ &uart_sclk {
 &dmac0 {
 	status = "okay";
 };
+
+&rst {
+	status = "okay";
+};
diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index ce708183b6f6..1cacfa386d19 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -5,6 +5,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/reset/th1520-reset.h>
 
 / {
 	compatible = "thead,th1520";
@@ -418,5 +419,12 @@ portf: gpio-controller@0 {
 				interrupts = <55 IRQ_TYPE_LEVEL_HIGH>;
 			};
 		};
+
+		rst: reset-controller@ffef014000 {
+			compatible = "thead,th1520-reset", "syscon";
+			reg = <0xff 0xef014000 0x0 0x1000>;
+			#reset-cells = <1>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.34.1

Re: [RFC PATCH 3/3] riscv: dts: Add th1520 reset device tree

[Krzysztof Kozlowski]

On 04/09/2023 06:25, Kwanghoon Son wrote:
> Add reset device tree for th1520 SoC
> 
> Signed-off-by: Kwanghoon Son <k.son@samsung.com>
> ---

...

>  / {
>  	compatible = "thead,th1520";
> @@ -418,5 +419,12 @@ portf: gpio-controller@0 {
>  				interrupts = <55 IRQ_TYPE_LEVEL_HIGH>;
>  			};
>  		};
> +
> +		rst: reset-controller@ffef014000 {
> +			compatible = "thead,th1520-reset", "syscon";
> +			reg = <0xff 0xef014000 0x0 0x1000>;
> +			#reset-cells = <1>;
> +			status = "disabled";

Why do you disable it? There are no missing resources.

Best regards,
Krzysztof

Re: [RFC PATCH 0/3] Introduce reset driver for T-HEAD th1520 SoC

[Philipp Zabel]

Hi,

On Mo, 2023-09-04 at 04:25 +0000, Kwanghoon Son wrote:
> This patchset adds initial support for reset driver.

what is meant by "initial support"? Are there just more resets than the
two watchdog resets, or is there other functionality that might require
describing in the device tree? Is the dt-binding document complete?
Is this the only reset controller on TH1520? In the thead-kernel.git it
looks like there are at least two different ones.

regards
Philipp

RE: [RFC PATCH 0/3] Introduce reset driver for T-HEAD th1520 SoC

[Kwanghoon Son]

> what is meant by "initial support"? Are there just more resets than the
> two watchdog resets, or is there other functionality that might require
> describing in the device tree? Is the dt-binding document complete?
> Is this the only reset controller on TH1520? In the thead-kernel.git it
> looks like there are at least two different ones.

True, in vendor kernel has more registers npu, vpsys reset.
But since I can't test other reset so I only added few register (CPU reset)
Also other reset can be added with same driver.

Best regards,
Kwang.

> 
> regards
> Philipp