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

[PATCH v3 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>;
};

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

Changelog:
v2:
https://lore.kernel.org/linux-riscv/20230912024914.3769440-1-k.son@samsung.com/
- wrong patch version
- yaml file indent
- missing vendor prefix
- drop okay

rfc:
https://lore.kernel.org/linux-riscv/20230904042559.2322997-1-k.son@samsung.com/
- dt_binding_check
- enable reset controller default

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    |  44 +++++++
 arch/riscv/boot/dts/thead/th1520.dtsi         |   7 ++
 drivers/reset/Kconfig                         |  10 ++
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-th1520.c                  | 109 ++++++++++++++++++
 .../dt-bindings/reset/thead,th1520-reset.h    |   9 ++
 6 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/thead,th1520-reset.h

-- 
2.34.1

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

[Kwanghoon Son]

Add documentation to describe th1520 reset device

Signed-off-by: Kwanghoon Son <k.son@samsung.com>
---
 .../bindings/reset/thead,th1520-reset.yaml    | 44 +++++++++++++++++++
 .../dt-bindings/reset/thead,th1520-reset.h    |  9 ++++
 2 files changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
 create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h

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..49ea8c6a331f
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/thead,th1520-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/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/thead,th1520-reset.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        reset-controller@ffef014000 {
+            compatible = "thead,th1520-reset", "syscon";
+            reg = <0xff 0xef014000 0x0 0x1000>;
+            #reset-cells = <1>;
+        };
+    };
diff --git a/include/dt-bindings/reset/thead,th1520-reset.h b/include/dt-bindings/reset/thead,th1520-reset.h
new file mode 100644
index 000000000000..ec10751814e5
--- /dev/null
+++ b/include/dt-bindings/reset/thead,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: [PATCH v3 1/3] dt-bindings: reset: Document th1520 reset control

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2565 bytes --]

On Mon, Sep 18, 2023 at 04:51:23AM +0000, Kwanghoon Son wrote:
> Add documentation to describe th1520 reset device

I would appreciate if you could add some information here as to what is
missing from this - the reset controller isn't just for two WDTs is it?

> 
> Signed-off-by: Kwanghoon Son <k.son@samsung.com>
> ---
>  .../bindings/reset/thead,th1520-reset.yaml    | 44 +++++++++++++++++++
>  .../dt-bindings/reset/thead,th1520-reset.h    |  9 ++++
>  2 files changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
>  create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h
> 
> 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..49ea8c6a331f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/thead,th1520-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/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

iDumb question perhaps, but why is this a syscon?

Cheers,
Conor.

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#reset-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/reset/thead,th1520-reset.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        reset-controller@ffef014000 {
> +            compatible = "thead,th1520-reset", "syscon";
> +            reg = <0xff 0xef014000 0x0 0x1000>;
> +            #reset-cells = <1>;
> +        };
> +    };
> diff --git a/include/dt-bindings/reset/thead,th1520-reset.h b/include/dt-bindings/reset/thead,th1520-reset.h
> new file mode 100644
> index 000000000000..ec10751814e5
> --- /dev/null
> +++ b/include/dt-bindings/reset/thead,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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Kwanghoon Son]

> > +      - const: thead,th1520-reset
> > +      - const: syscon
> 
> iDumb question perhaps, but why is this a syscon?

This is good point.
In fact, I took it from the vendor kernel, and I tried to keep it as same as I could.
Vendor also followed drivers/reset/reset-imx7.c
As Rob said, if don't need it, I'll change it.

Regards,
Kwang.

> Cheers,
> Conor.
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#reset-cells':
> > +    const: 1

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

[Kwanghoon Son]

> > > +      - const: thead,th1520-reset
> > > +      - const: syscon
> >
> > iDumb question perhaps, but why is this a syscon?
> 
> This is good point.
> In fact, I took it from the vendor kernel, and I tried to keep it as same as I could.
> Vendor also followed drivers/reset/reset-imx7.c
> As Rob said, if don't need it, I'll change it.
> 
> Regards,
> Kwang.
> 
I have investigated vendor kernels, but it is still not sure reset information.
I thought this is about sys_reg, but without datasheet, can't tell.
Therefore, should be stopped patch for a while until knows better comes out.

Kwang.

> > Cheers,
> > Conor.
> >
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  '#reset-cells':
> > > +    const: 1

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

Guo,

On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote:
> > > > +      - const: thead,th1520-reset
> > > > +      - const: syscon
> > >
> > > iDumb question perhaps, but why is this a syscon?
> > 
> > This is good point.
> > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could.
> > Vendor also followed drivers/reset/reset-imx7.c
> > As Rob said, if don't need it, I'll change it.

> I have investigated vendor kernels, but it is still not sure reset information.
> I thought this is about sys_reg, but without datasheet, can't tell.
> Therefore, should be stopped patch for a while until knows better comes out.

Is there documentation in English you can provide to these people that
are trying to add support for your product?

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Guo Ren]

On Tue, Sep 19, 2023 at 4:47 PM Conor Dooley <conor@kernel.org> wrote:
>
> Guo,
>
> On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote:
> > > > > +      - const: thead,th1520-reset
> > > > > +      - const: syscon
> > > >
> > > > iDumb question perhaps, but why is this a syscon?
> > >
> > > This is good point.
> > > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could.
> > > Vendor also followed drivers/reset/reset-imx7.c
> > > As Rob said, if don't need it, I'll change it.
>
> > I have investigated vendor kernels, but it is still not sure reset information.
> > I thought this is about sys_reg, but without datasheet, can't tell.
> > Therefore, should be stopped patch for a while until knows better comes out.
>
> Is there documentation in English you can provide to these people that
> are trying to add support for your product?
Yes, Fuwei would help with this, and thx for all of your work.

>
> Thanks,
> Conor.



-- 
Best Regards
 Guo Ren

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

[Xi Ruoyao]

On Tue, 2023-09-19 at 09:47 +0100, Conor Dooley wrote:
> Guo,
> 
> On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote:
> > > > > +      - const: thead,th1520-reset
> > > > > +      - const: syscon
> > > > 
> > > > iDumb question perhaps, but why is this a syscon?
> > > 
> > > This is good point.
> > > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could.
> > > Vendor also followed drivers/reset/reset-imx7.c
> > > As Rob said, if don't need it, I'll change it.
> 
> > I have investigated vendor kernels, but it is still not sure reset information.
> > I thought this is about sys_reg, but without datasheet, can't tell.
> > Therefore, should be stopped patch for a while until knows better comes out.
> 
> Is there documentation in English you can provide to these people that
> are trying to add support for your product?

It can be downloaded from
https://dl.sipeed.com/shareURL/LICHEE/licheepi4a/09_Doc now.  Click on
"TH1520 User Manual.zip" and enter a captcha.  The doc about resetting
is in chapter 5 of "TH1520 System User Manual.pdf".


-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]

On Thu, Nov 30, 2023 at 12:42:09AM +0800, Xi Ruoyao wrote:
> On Tue, 2023-09-19 at 09:47 +0100, Conor Dooley wrote:
> > Guo,
> > 
> > On Tue, Sep 19, 2023 at 02:08:32PM +0900, Kwanghoon Son wrote:
> > > > > > +      - const: thead,th1520-reset
> > > > > > +      - const: syscon
> > > > > 
> > > > > iDumb question perhaps, but why is this a syscon?
> > > > 
> > > > This is good point.
> > > > In fact, I took it from the vendor kernel, and I tried to keep it as same as I could.
> > > > Vendor also followed drivers/reset/reset-imx7.c
> > > > As Rob said, if don't need it, I'll change it.
> > 
> > > I have investigated vendor kernels, but it is still not sure reset information.
> > > I thought this is about sys_reg, but without datasheet, can't tell.
> > > Therefore, should be stopped patch for a while until knows better comes out.
> > 
> > Is there documentation in English you can provide to these people that
> > are trying to add support for your product?
> 
> It can be downloaded from
> https://dl.sipeed.com/shareURL/LICHEE/licheepi4a/09_Doc now.  Click on
> "TH1520 User Manual.zip" and enter a captcha.  The doc about resetting
> is in chapter 5 of "TH1520 System User Manual.pdf".

Thanks Xi! Pity there was no response from the vendor to this for
months :(

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Rob Herring]

On Mon, Sep 18, 2023 at 04:51:23AM +0000, Kwanghoon Son wrote:
> Add documentation to describe th1520 reset device
> 
> Signed-off-by: Kwanghoon Son <k.son@samsung.com>
> ---
>  .../bindings/reset/thead,th1520-reset.yaml    | 44 +++++++++++++++++++
>  .../dt-bindings/reset/thead,th1520-reset.h    |  9 ++++
>  2 files changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
>  create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h
> 
> 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..49ea8c6a331f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/thead,th1520-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/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

Why do you need "syscon"? Are there other functions? If so, please 
describe them in top-level "description". If not, drop "syscon" and use 
device_node_to_regmap() instead of syscon_node_to_regmap().

Rob

[PATCH v3 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 +++++++++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+)
 create mode 100644 drivers/reset/reset-th1520.c

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..5a89d201fc0c
--- /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/thead,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);
-- 
2.34.1

[PATCH v3 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.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index ce708183b6f6..210f234839c3 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/thead,th1520-reset.h>
 
 / {
 	compatible = "thead,th1520";
@@ -418,5 +419,11 @@ 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>;
+		};
 	};
 };
-- 
2.34.1