[PATCH v2 0/4] Add Armada8K reset controller support

[PATCH v2 0/4] Add Armada8K reset controller support

[Wilson Ding]

Armada8K has one simple register for unit soft reset, which is part of
the system controller register area. The simple reset code doesn't
support register access via regmap for the syscon devices. This patch
series created a new driver based on the simple reset code, and add
Armada8K support then.

Thanks,
Wilson

---

Changes in v2:
  - Created a new driver for SYSCON device instead of extending the
    simple reset code.
  - Allow to retreive the register offset from the 'reg' property as
    an alternative to the 'offset' property.
  - Allow to retrevie the register size from the 'reg' property to
    calculate the number of reset lines.
  - Added the new dt-binding files to document the device-tree scheme
    and fix DT check issues.
  - Updated the device-tree node name to 'reset-controller' to follow
    the name conventions.

Changes in v1:
  - Init version.

Wilson Ding (4):
  dt-bindings: reset: Add Armada8K reset controller
  dt-bindings: cp110: Document the reset controller
  reset: Add support for Armada8K reset controller
  arm64: dts: marvell: cp11x: Add reset controller node

 .../arm/marvell/cp110-system-controller.txt   |  43 ++++
 .../reset/marvell,armada8k-reset.yaml         |  45 ++++
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi |   6 +
 drivers/reset/Kconfig                         |  12 ++
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-simple-syscon.c           | 195 ++++++++++++++++++
 include/linux/reset/reset-simple.h            |   3 +
 7 files changed, 305 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
 create mode 100644 drivers/reset/reset-simple-syscon.c

-- 
2.43.0

[PATCH v2 1/4] dt-bindings: reset: Add Armada8K reset controller

[Wilson Ding]

Add device-tree binding documentation for the Armada8K reset driver.

Signed-off-by: Wilson Ding <dingwei@marvell.com>
---
 .../reset/marvell,armada8k-reset.yaml         | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml

diff --git a/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
new file mode 100644
index 000000000000..b9f7f3c24d3c
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2025 Wilson Ding <dingwei@marvell.com>
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/marvell,armada8k-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Armada8K reset controller
+
+maintainers:
+  - Wilson Ding <dingwei@marvell.com>
+
+description: The reset controller node must be a sub-node of the system
+  controller node on Armada7K/8K or CN913x SoCs.
+
+properties:
+  compatible:
+    const: marvell,armada8k-reset
+
+  offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Offset in the register map for the gpio registers (in bytes)
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - offset
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    syscon0: system-controller@440000 {
+      compatible = "syscon", "simple-mfd";
+      reg = <0x440000 0x2000>;
+
+        unit_swrst: reset-controller@268 {
+            compatible = "marvell,armada8k-reset";
+            offset = <0x268>;
+            #reset-cells = <1>;
+        };
+    };
-- 
2.43.0

[PATCH v2 2/4] dt-bindings: cp110: Document the reset controller

[Wilson Ding]

Add reset-controller sub-node into the system controller node, and
document the supported reset lines.

Signed-off-by: Wilson Ding <dingwei@marvell.com>
---
 .../arm/marvell/cp110-system-controller.txt   | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
index 9d5d70c98058..5e2c2f2b147c 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt
@@ -164,6 +164,43 @@ Required properties:
 
 - offset: offset address inside the syscon block
 
+Reset Controller:
+-----------------
+
+For common binding part and usage, refer to
+Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml.
+
+The Device Tree node representing this System Controller 0 provides a
+number of reset lines:
+
+The following reset lines are available:
+
+-  0: Audio Software RESETn
+-  1: TDM Software RESETn
+-  2: Interrupt controller unit Software RESETn
+-  3: Packet processor Software RESETn
+-  4: SDIO Software RESETn
+-  7: XOR-1 engine Software RESETn
+-  8: XOR-0 engine Software RESETn
+- 11: PCIe-0 Gen.3 x1 Software RESETn
+- 12: PCIe-1 Gen.3 x1 Software RESETn
+- 13: PCIe Gen.3 x4 Software RESETn
+- 15: SATA port 0 and port 1 Software RESETn
+- 22: USB3 Host 0 Software RESETn
+- 23: USB3 Host 1 Software RESETn
+- 24: USB3 Device Software RESETn
+- 25: EIP150F Software RESETn
+- 26: EIP197 Software RESETn
+- 29: MSS Software RESETn
+
+Required properties:
+
+ - compatible: "marvell,armada8k-reset"
+
+ - offset: offset address inside the syscon block
+
+ - #reset-cells: must be set to 1
+
 Example:
 
 CP110_LABEL(syscon0): system-controller@440000 {
@@ -188,6 +225,12 @@ CP110_LABEL(syscon0): system-controller@440000 {
 		gpio-ranges = <&CP110_LABEL(pinctrl) 0 0 32>;
 	};
 
+	CP11X_LABEL(unit_swrst): reset-controller@268 {
+		compatible = "marvell,armada8k-reset";
+		offset = <0x268>;
+		#reset-cells = <1>;
+	};
+
 };
 
 SYSTEM CONTROLLER 1
-- 
2.43.0

[PATCH v2 3/4] reset: Add support for Armada8K reset controller

[Wilson Ding]

Armada8K has one register for unit soft-reset configuration within the
system controller register area. This patch created a new driver based
on the simple-reset driver to support the reset controller of a SYSCON
device.

Signed-off-by: Wilson Ding <dingwei@marvell.com>
---
 drivers/reset/Kconfig               |  12 ++
 drivers/reset/Makefile              |   1 +
 drivers/reset/reset-simple-syscon.c | 195 ++++++++++++++++++++++++++++
 include/linux/reset/reset-simple.h  |   3 +
 4 files changed, 211 insertions(+)
 create mode 100644 drivers/reset/reset-simple-syscon.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 5b3abb6db248..e98b22195317 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -248,6 +248,18 @@ config RESET_SIMPLE
 	   - SiFive FU740 SoCs
 	   - Sophgo SoCs
 
+config RESET_SIMPLE_SYSCON
+	bool "Simple SYSCON Reset Controller Driver" if COMPILE_TEST || EXPERT
+	depends on HAS_IOMEM
+	select MFD_SYSCON
+	help
+	  This enables a simple reset controller driver for reset lines that
+	  that can be asserted and deasserted by toggling bits in a contiguous,
+	  exclusive register space.
+
+	  Currently this driver supports:
+	   - Marvell Armada8K SoCs
+
 config RESET_SOCFPGA
 	bool "SoCFPGA Reset Driver" if COMPILE_TEST && (!ARM || !ARCH_INTEL_SOCFPGA)
 	default ARM && ARCH_INTEL_SOCFPGA
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 677c4d1e2632..c43642fe4d9b 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o
 obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o
 obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
 obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
+obj-$(CONFIG_RESET_SIMPLE_SYSCON) += reset-simple-syscon.o
 obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
diff --git a/drivers/reset/reset-simple-syscon.c b/drivers/reset/reset-simple-syscon.c
new file mode 100644
index 000000000000..aeeb9e9550f5
--- /dev/null
+++ b/drivers/reset/reset-simple-syscon.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Simple SYSCON Reset Controller Driver
+ *
+ * Copyright (C) 2025 Marvell, Wilson Ding <dingwei@marvell.com>
+ *
+ * Based on Simple Reset Controller Driver
+ *
+ * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
+ * Copyright 2013 Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/reset/reset-simple.h>
+#include <linux/spinlock.h>
+
+static inline struct reset_simple_data *
+to_reset_simple_data(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct reset_simple_data, rcdev);
+}
+
+static int reset_simple_syscon_update(struct reset_controller_dev *rcdev,
+				      unsigned long id, bool assert)
+{
+	struct reset_simple_data *data = to_reset_simple_data(rcdev);
+	int reg_width = sizeof(u32);
+	int bank = id / (reg_width * BITS_PER_BYTE);
+	int offset = id % (reg_width * BITS_PER_BYTE);
+	u32 mask, val;
+
+	mask = BIT(offset);
+
+	if (assert ^ data->active_low)
+		val = mask;
+	else
+		val = 0;
+
+	return regmap_write_bits(data->regmap,
+				 data->reg_offset + (bank * reg_width),
+				 mask, val);
+}
+
+static int reset_simple_syscon_assert(struct reset_controller_dev *rcdev,
+				      unsigned long id)
+{
+	return reset_simple_syscon_update(rcdev, id, true);
+}
+
+static int reset_simple_syscon_deassert(struct reset_controller_dev *rcdev,
+					unsigned long id)
+{
+	return reset_simple_syscon_update(rcdev, id, false);
+}
+
+static int reset_simple_syscon_reset(struct reset_controller_dev *rcdev,
+				     unsigned long id)
+{
+	struct reset_simple_data *data = to_reset_simple_data(rcdev);
+	int ret;
+
+	if (!data->reset_us)
+		return -EINVAL;
+
+	ret = reset_simple_syscon_assert(rcdev, id);
+	if (ret)
+		return ret;
+
+	usleep_range(data->reset_us, data->reset_us * 2);
+
+	return reset_simple_syscon_deassert(rcdev, id);
+}
+
+static int reset_simple_syscon_status(struct reset_controller_dev *rcdev,
+				      unsigned long id)
+{
+	struct reset_simple_data *data = to_reset_simple_data(rcdev);
+	int reg_width = sizeof(u32);
+	int bank = id / (reg_width * BITS_PER_BYTE);
+	int offset = id % (reg_width * BITS_PER_BYTE);
+	u32 reg;
+	int ret;
+
+	ret = regmap_read(data->regmap, data->reg_offset + (bank * reg_width),
+			  &reg);
+	if (ret)
+		return ret;
+
+	return !(reg & BIT(offset)) ^ !data->status_active_low;
+}
+
+const struct reset_control_ops reset_simple_syscon_ops = {
+	.assert		= reset_simple_syscon_assert,
+	.deassert	= reset_simple_syscon_deassert,
+	.reset		= reset_simple_syscon_reset,
+	.status		= reset_simple_syscon_status,
+};
+EXPORT_SYMBOL_GPL(reset_simple_syscon_ops);
+
+/**
+ * struct reset_simple_syscon_devdata - simple reset controller properties
+ * @reg_offset: offset between base address and first reset register.
+ * @nr_resets: number of resets. If not set, default to resource size in bits.
+ * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
+ *              are set to assert the reset.
+ * @status_active_low: if true, bits read back as cleared while the reset is
+ *                     asserted. Otherwise, bits read back as set while the
+ *                     reset is asserted.
+ */
+struct reset_simple_syscon_devdata {
+	u32 reg_offset;
+	u32 nr_resets;
+	bool active_low;
+	bool status_active_low;
+};
+
+static const struct reset_simple_syscon_devdata reset_simple_syscon_armada8k = {
+	.nr_resets = 32,
+	.active_low = true,
+	.status_active_low = true,
+};
+
+static const struct of_device_id reset_simple_syscon_dt_ids[] = {
+	{ .compatible = "marvell,armada8k-reset",
+		.data = &reset_simple_syscon_armada8k },
+	{ /* sentinel */ },
+};
+
+static int reset_simple_syscon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct reset_simple_syscon_devdata *devdata;
+	struct reset_simple_data *data;
+	u32 reg[2];
+
+	devdata = of_device_get_match_data(dev);
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->regmap = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	/*
+	 * If the "reg" property is available, retrieve the reg_offset and
+	 * calculate the nr_resets based on the register size. If "reg" is not
+	 * available, attempt to read the "offset" property for reg_offset and
+	 * use nr_resets from devdata. If "offset" is also not available,
+	 * default to using reg_offset from devdata.
+	 */
+	if (device_property_read_u32_array(&pdev->dev, "reg", reg, 2)) {
+		if (device_property_read_u32(&pdev->dev, "offset",
+					     &data->reg_offset))
+			data->reg_offset = devdata->reg_offset;
+		data->rcdev.nr_resets = devdata->nr_resets;
+	} else {
+		data->reg_offset = reg[0];
+		data->rcdev.nr_resets = reg[1] * BITS_PER_BYTE;
+	}
+
+	data->active_low = devdata->active_low;
+	data->status_active_low = devdata->status_active_low;
+
+	if (data->rcdev.nr_resets == 0) {
+		dev_err(dev, "no reset lines\n");
+		return -EINVAL;
+	}
+
+	spin_lock_init(&data->lock);
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.ops = &reset_simple_syscon_ops;
+	data->rcdev.of_node = dev->of_node;
+
+	return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static struct platform_driver reset_simple_syscon_driver = {
+	.probe	= reset_simple_syscon_probe,
+	.driver = {
+		.name		= "simple-reset-syscon",
+		.of_match_table	= reset_simple_syscon_dt_ids,
+	},
+};
+builtin_platform_driver(reset_simple_syscon_driver);
diff --git a/include/linux/reset/reset-simple.h b/include/linux/reset/reset-simple.h
index c3e44f45b0f7..9a8eebd5892f 100644
--- a/include/linux/reset/reset-simple.h
+++ b/include/linux/reset/reset-simple.h
@@ -13,6 +13,7 @@
 #define __RESET_SIMPLE_H__
 
 #include <linux/io.h>
+#include <linux/regmap.h>
 #include <linux/reset-controller.h>
 #include <linux/spinlock.h>
 
@@ -37,6 +38,8 @@
 struct reset_simple_data {
 	spinlock_t			lock;
 	void __iomem			*membase;
+	struct regmap			*regmap;
+	u32				reg_offset;
 	struct reset_controller_dev	rcdev;
 	bool				active_low;
 	bool				status_active_low;
-- 
2.43.0

[PATCH v2 4/4] arm64: dts: marvell: cp11x: Add reset controller node

[Wilson Ding]

Add the reset controller node as a sub-node to the system controller
node.

Signed-off-by: Wilson Ding <dingwei@marvell.com>
---
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 161beec0b6b0..4cd900c02b3b 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -273,6 +273,12 @@ CP11X_LABEL(gpio2): gpio@140 {
 					 <&CP11X_LABEL(clk) 1 17>;
 				status = "disabled";
 			};
+
+			CP11X_LABEL(unit_swrst): reset-controller@268 {
+				compatible = "marvell,armada8k-reset";
+				offset = <0x268>;
+				#reset-cells = <1>;
+			};
 		};
 
 		CP11X_LABEL(syscon1): system-controller@400000 {
-- 
2.43.0

Re: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K reset controller

[Rob Herring (Arm)]

On Thu, 20 Feb 2025 15:25:24 -0800, Wilson Ding wrote:
> Add device-tree binding documentation for the Armada8K reset driver.
> 
> Signed-off-by: Wilson Ding <dingwei@marvell.com>
> ---
>  .../reset/marvell,armada8k-reset.yaml         | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example.dts:22.46-26.15: Warning (unit_address_vs_reg): /example-0/system-controller@440000/reset-controller@268: node has a unit name, but no reg or ranges property
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example.dtb: system-controller@440000: compatible: ['syscon', 'simple-mfd'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250220232527.882888-2-dingwei@marvell.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: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K reset controller

[Krzysztof Kozlowski]

On Thu, Feb 20, 2025 at 03:25:24PM -0800, Wilson Ding wrote:
> Add device-tree binding documentation for the Armada8K reset driver.
> 
> Signed-off-by: Wilson Ding <dingwei@marvell.com>
> ---
>  .../reset/marvell,armada8k-reset.yaml         | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> new file mode 100644
> index 000000000000..b9f7f3c24d3c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2025 Wilson Ding <dingwei@marvell.com>
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/marvell,armada8k-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Armada8K reset controller
> +
> +maintainers:
> +  - Wilson Ding <dingwei@marvell.com>
> +
> +description: The reset controller node must be a sub-node of the system
> +  controller node on Armada7K/8K or CN913x SoCs.
> +
> +properties:
> +  compatible:
> +    const: marvell,armada8k-reset
> +
> +  offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Offset in the register map for the gpio registers (in bytes)

That's neither correct nor needed. Your device knows ofsset based on the
compatible.

Best regards,
Krzysztof

Re: [PATCH v2 2/4] dt-bindings: cp110: Document the reset controller

[Krzysztof Kozlowski]

On Thu, Feb 20, 2025 at 03:25:25PM -0800, Wilson Ding wrote:
> Add reset-controller sub-node into the system controller node, and
> document the supported reset lines.
> 
> Signed-off-by: Wilson Ding <dingwei@marvell.com>
> ---
>  .../arm/marvell/cp110-system-controller.txt   | 43 +++++++++++++++++++

NAK.

You already got feedback that TXT bindings are not accepted.

Best regards,
Krzysztof

Re: [PATCH v2 4/4] arm64: dts: marvell: cp11x: Add reset controller node

[Krzysztof Kozlowski]

On Thu, Feb 20, 2025 at 03:25:27PM -0800, Wilson Ding wrote:
> Add the reset controller node as a sub-node to the system controller
> node.
> 
> Signed-off-by: Wilson Ding <dingwei@marvell.com>
> ---
>  arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> index 161beec0b6b0..4cd900c02b3b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> @@ -273,6 +273,12 @@ CP11X_LABEL(gpio2): gpio@140 {
>  					 <&CP11X_LABEL(clk) 1 17>;
>  				status = "disabled";
>  			};
> +
> +			CP11X_LABEL(unit_swrst): reset-controller@268 {

Test your patches before posting. You have here warnings - see W=1.

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K reset controller

[Rob Herring]

On Fri, Feb 21, 2025 at 09:46:01AM +0100, Krzysztof Kozlowski wrote:
> On Thu, Feb 20, 2025 at 03:25:24PM -0800, Wilson Ding wrote:
> > Add device-tree binding documentation for the Armada8K reset driver.
> > 
> > Signed-off-by: Wilson Ding <dingwei@marvell.com>
> > ---
> >  .../reset/marvell,armada8k-reset.yaml         | 45 +++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> > new file mode 100644
> > index 000000000000..b9f7f3c24d3c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> > @@ -0,0 +1,45 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2025 Wilson Ding <dingwei@marvell.com>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reset/marvell,armada8k-reset.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell Armada8K reset controller
> > +
> > +maintainers:
> > +  - Wilson Ding <dingwei@marvell.com>
> > +
> > +description: The reset controller node must be a sub-node of the system
> > +  controller node on Armada7K/8K or CN913x SoCs.
> > +
> > +properties:
> > +  compatible:
> > +    const: marvell,armada8k-reset
> > +
> > +  offset:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Offset in the register map for the gpio registers (in bytes)
> 
> That's neither correct nor needed. Your device knows ofsset based on the
> compatible.

Or use 'reg'.

But really, just add #reset-cells to the parent node. There's no need 
for a child node here. The parent needs a specific compatible though.

Rob

Re: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K reset controller

[Wilson Ding]

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Friday, February 21, 2025 3:41 PM
> To: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> andrew@lunn.ch; gregory.clement@bootlin.com;
> sebastian.hesselbarth@gmail.com; krzk+dt@kernel.org; conor+dt@kernel.org;
> p.zabel@pengutronix.de; Sanghoon Lee <salee@marvell.com>;
> Geethasowjanya Akula <gakula@marvell.com>
> Subject: [EXTERNAL] Re: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K
> reset controller
> 
> On Fri, Feb 21, 2025 at 09: 46: 01AM +0100, Krzysztof Kozlowski wrote: > On
> Thu, Feb 20, 2025 at 03: 25: 24PM -0800, Wilson Ding wrote: > > Add device-
> tree binding documentation for the Armada8K reset driver. > > > > Signed-off-
> by: 
> On Fri, Feb 21, 2025 at 09:46:01AM +0100, Krzysztof Kozlowski wrote:
> > On Thu, Feb 20, 2025 at 03:25:24PM -0800, Wilson Ding wrote:
> > > Add device-tree binding documentation for the Armada8K reset driver.
> > >
> > > Signed-off-by: Wilson Ding <dingwei@marvell.com>
> > > ---
> > >  .../reset/marvell,armada8k-reset.yaml         | 45 +++++++++++++++++++
> > >  1 file changed, 45 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yam
> > > l
> > > b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yam
> > > l
> > > new file mode 100644
> > > index 000000000000..b9f7f3c24d3c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset
> > > +++ .yaml
> > > @@ -0,0 +1,45 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) # Copyright
> > > +2025 Wilson Ding <dingwei@marvell.com> %YAML 1.2
> > > +---
> > > +$id:
> > > +https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_
> > > +schemas_reset_marvell-2Carmada8k-2Dreset.yaml-
> 23&d=DwIBAg&c=nKjWec2
> > >
> +b6R0mOyPaz7xtfQ&r=sXDQZu4GyqNVDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA
> &m=Q8Y
> > >
> +WjAVZaKSaPD0B9Hb3Il6KsiVTSpeB9Br9zI5bqQ9MyW0LQnEdiE2kzdfMRQfa
> &s=VRF
> > > +OPAwNVByPfjGNG1IWJt_mAet2LVsQmFVALenV7Ck&e=
> > > +$schema:
> > > +https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_
> > > +meta-2Dschemas_core.yaml-
> 23&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXD
> > >
> +QZu4GyqNVDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=Q8YWjAVZaKSaPD0B
> 9Hb3Il6K
> > >
> +siVTSpeB9Br9zI5bqQ9MyW0LQnEdiE2kzdfMRQfa&s=3F_XMbPCmCHx0pRO
> vv0KP1cZ
> > > +tvjQBFPSBpdty-yVZjY&e=
> > > +
> > > +title: Marvell Armada8K reset controller
> > > +
> > > +maintainers:
> > > +  - Wilson Ding <dingwei@marvell.com>
> > > +
> > > +description: The reset controller node must be a sub-node of the
> > > +system
> > > +  controller node on Armada7K/8K or CN913x SoCs.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: marvell,armada8k-reset
> > > +
> > > +  offset:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Offset in the register map for the gpio registers
> > > + (in bytes)
> >
> > That's neither correct nor needed. Your device knows ofsset based on
> > the compatible.
> 
> Or use 'reg'.
> 
> But really, just add #reset-cells to the parent node. There's no need for a child
> node here. The parent needs a specific compatible though.
> 

I am not inventing the 'offset' property. I just tried to follow the other existing
sub-nodes under the same parent node (system-controller). The mvebu-gpio
driver also uses 'offset' instead of 'reg' for the syscon device (see below). But it
seems also not correct from your point of view. Now, I am a bit confused what
should be the right scheme for the Armada's system-controller, including GPIO
and Reset controller. And dt_binding_check complains "system-controller@
440000:  compatible: ['syscon', 'simple-mfd'] is too short". Can you point me
any  reference for me to fix these issues.

CP110_LABEL(syscon0): system-controller@440000 {
	compatible = "syscon", "simple-mfd";
	reg = <0x440000 0x1000>;

	CP110_LABEL(clk): clock {
		compatible = "marvell,cp110-clock";
		#clock-cells = <2>;
	};

	CP110_LABEL(pinctrl): pinctrl {
		compatible = "marvell,armada-8k-cpm-pinctrl";
	};

	CP110_LABEL(gpio1): gpio@100 {
		compatible = "marvell,armada-8k-gpio";
		offset = <0x100>;
		#gpio-cells = <2>;
		...
	};

	CP11X_LABEL(unit_swrst): reset-controller@268 {
		compatible = "marvell,armada8k-reset";
		offset = <0x268>;
		#reset-cells = <1>;
	};

};

> Rob

RE: [EXTERNAL] Re: [PATCH v2 2/4] dt-bindings: cp110: Document the reset controller

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Friday, February 21, 2025 12:47 AM
> To: Wilson Ding <dingwei@marvell.com>
> Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; andrew@lunn.ch; gregory.clement@bootlin.com;
> sebastian.hesselbarth@gmail.com; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; p.zabel@pengutronix.de; Sanghoon Lee
> <salee@marvell.com>; Geethasowjanya Akula <gakula@marvell.com>
> Subject: [EXTERNAL] Re: [PATCH v2 2/4] dt-bindings: cp110: Document the
> reset controller
> 
> On Thu, Feb 20, 2025 at 03: 25: 25PM -0800, Wilson Ding wrote: > Add reset-
> controller sub-node into the system controller node, and > document the
> supported reset lines. > > Signed-off-by: Wilson Ding
> <dingwei@ marvell. com> > 
> On Thu, Feb 20, 2025 at 03:25:25PM -0800, Wilson Ding wrote:
> > Add reset-controller sub-node into the system controller node, and
> > document the supported reset lines.
> >
> > Signed-off-by: Wilson Ding <dingwei@marvell.com>
> > ---
> >  .../arm/marvell/cp110-system-controller.txt   | 43 +++++++++++++++++++
> 
> NAK.
> 
> You already got feedback that TXT bindings are not accepted.
> 

I am not trying to use this TXT for dt-binding. But as you can see this file
describes all the sub-nodes details, such as mpps, clocks information. So
that I add the reset line info here. If this is not the right place, where shall
I put this information. I wonder if you are asking to convert this txt into
yaml.

> Best regards,
> Krzysztof

Re: [EXTERNAL] Re: [PATCH v2 2/4] dt-bindings: cp110: Document the reset controller

[Krzysztof Kozlowski]

On 22/02/2025 22:02, Wilson Ding wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Friday, February 21, 2025 12:47 AM
>> To: Wilson Ding <dingwei@marvell.com>
>> Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; andrew@lunn.ch; gregory.clement@bootlin.com;
>> sebastian.hesselbarth@gmail.com; robh@kernel.org; krzk+dt@kernel.org;
>> conor+dt@kernel.org; p.zabel@pengutronix.de; Sanghoon Lee
>> <salee@marvell.com>; Geethasowjanya Akula <gakula@marvell.com>
>> Subject: [EXTERNAL] Re: [PATCH v2 2/4] dt-bindings: cp110: Document the
>> reset controller
>>
>> On Thu, Feb 20, 2025 at 03: 25: 25PM -0800, Wilson Ding wrote: > Add reset-
>> controller sub-node into the system controller node, and > document the
>> supported reset lines. > > Signed-off-by: Wilson Ding
>> <dingwei@ marvell. com> > 
>> On Thu, Feb 20, 2025 at 03:25:25PM -0800, Wilson Ding wrote:
>>> Add reset-controller sub-node into the system controller node, and
>>> document the supported reset lines.
>>>
>>> Signed-off-by: Wilson Ding <dingwei@marvell.com>
>>> ---
>>>  .../arm/marvell/cp110-system-controller.txt   | 43 +++++++++++++++++++
>>
>> NAK.
>>
>> You already got feedback that TXT bindings are not accepted.
>>
> 
> I am not trying to use this TXT for dt-binding. But as you can see this file
> describes all the sub-nodes details, such as mpps, clocks information. So
> that I add the reset line info here. If this is not the right place, where shall
> I put this information. I wonder if you are asking to convert this txt into
> yaml.
You just duplicated the YAML binding in TXT. If you wanted to reference
the child, you would say something that there is a reset device in
binding foo.yaml.

But yeah, ideally this entire binding should be also converted to DT
Schema / YAML.

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K reset controller

[Krzysztof Kozlowski]

On 22/02/2025 21:57, Wilson Ding wrote:

>>>> +  offset:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description: Offset in the register map for the gpio registers
>>>> + (in bytes)
>>>
>>> That's neither correct nor needed. Your device knows ofsset based on
>>> the compatible.
>>
>> Or use 'reg'.
>>
>> But really, just add #reset-cells to the parent node. There's no need for a child
>> node here. The parent needs a specific compatible though.
>>
> 
> I am not inventing the 'offset' property. I just tried to follow the other existing
> sub-nodes under the same parent node (system-controller). The mvebu-gpio
> driver also uses 'offset' instead of 'reg' for the syscon device (see below). But it


You never answered why do you need offset and why it cannot work without.

> seems also not correct from your point of view. Now, I am a bit confused what
> should be the right scheme for the Armada's system-controller, including GPIO
> and Reset controller. And dt_binding_check complains "system-controller@
> 440000:  compatible: ['syscon', 'simple-mfd'] is too short". Can you point me
> any  reference for me to fix these issues.

See all other syscon devices. 'git grep simple-mfd' or for syscon


> 
> CP110_LABEL(syscon0): system-controller@440000 {
> 	compatible = "syscon", "simple-mfd";
> 	reg = <0x440000 0x1000>;


Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K reset controller

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Sunday, February 23, 2025 12:49 AM
> To: Wilson Ding <dingwei@marvell.com>; Rob Herring <robh@kernel.org>
> Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; andrew@lunn.ch; gregory.clement@bootlin.com;
> sebastian.hesselbarth@gmail.com; krzk+dt@kernel.org; conor+dt@kernel.org;
> p.zabel@pengutronix.de; Sanghoon Lee <salee@marvell.com>;
> Geethasowjanya Akula <gakula@marvell.com>
> Subject: [EXTERNAL] Re: [PATCH v2 1/4] dt-bindings: reset: Add Armada8K
> reset controller
> 
> On 22/02/2025 21: 57, Wilson Ding wrote: >>>> + offset: >>>> + $ref:
> /schemas/types. yaml#/definitions/uint32 >>>> + description: Offset in the
> register map for the gpio registers >>>> + (in bytes)
> 
> On 22/02/2025 21:57, Wilson Ding wrote:
> 
> >>>> +  offset:
> >>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>> +    description: Offset in the register map for the gpio registers
> >>>> + (in bytes)
> >>>
> >>> That's neither correct nor needed. Your device knows ofsset based on
> >>> the compatible.
> >>
> >> Or use 'reg'.
> >>
> >> But really, just add #reset-cells to the parent node. There's no need
> >> for a child node here. The parent needs a specific compatible though.
> >>
> >
> > I am not inventing the 'offset' property. I just tried to follow the
> > other existing sub-nodes under the same parent node
> > (system-controller). The mvebu-gpio driver also uses 'offset' instead
> > of 'reg' for the syscon device (see below). But it
> 
> 
> You never answered why do you need offset and why it cannot work without.
> 

I see. You're right. I don't have to use 'offset'. The reason I choose 'offset' over
'reg' is that the Armada's GPIO driver use it. And I thought what was accepted
would be easier to be accepted again. I will change it in next version.

> > seems also not correct from your point of view. Now, I am a bit
> > confused what should be the right scheme for the Armada's
> > system-controller, including GPIO and Reset controller. And
> > dt_binding_check complains "system-controller@
> > 440000:  compatible: ['syscon', 'simple-mfd'] is too short". Can you
> > point me any  reference for me to fix these issues.
> 
> See all other syscon devices. 'git grep simple-mfd' or for syscon
> 
> 
> >
> > CP110_LABEL(syscon0): system-controller@440000 {
> > 	compatible = "syscon", "simple-mfd";
> > 	reg = <0x440000 0x1000>;
> 
> 
> Best regards,
> Krzysztof