[PATCH v3 0/3] Add Armada8K reset controller support

[PATCH v3 0/3] 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 v3:
  - Abandoned the use of the 'offset' property to specify the register
    offset.
  - Dropped the changes to 'cp110-system-controller.txt'.
  - Created a new dt-binding head file to define the reset lines.
  - Fixed the errors and warnings reported by dt_binding_check and
    dtbs_check.

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 (3):
  dt-bindings: reset: Add Armada8K reset controller
  reset: Add support for Armada8K reset controller
  arm64: dts: marvell: cp11x: Add reset controller node

 .../reset/marvell,armada8k-reset.yaml         |  48 +++++
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi |   8 +
 drivers/reset/Kconfig                         |  12 ++
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-simple-syscon.c           | 188 ++++++++++++++++++
 .../reset/marvell,armada8k-reset.h            |  27 +++
 include/linux/reset/reset-simple.h            |   3 +
 7 files changed, 287 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
 create mode 100644 drivers/reset/reset-simple-syscon.c
 create mode 100644 include/dt-bindings/reset/marvell,armada8k-reset.h

-- 
2.43.0

[PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Wilson Ding]

Add device-tree binding documentation for the Armada8K reset driver and
create the new head file for the reset line index definitions.

Signed-off-by: Wilson Ding <dingwei@marvell.com>
---
 .../reset/marvell,armada8k-reset.yaml         | 48 +++++++++++++++++++
 .../reset/marvell,armada8k-reset.h            | 27 +++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
 create mode 100644 include/dt-bindings/reset/marvell,armada8k-reset.h

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..9af352f528de
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
@@ -0,0 +1,48 @@
+# 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
+
+  reg:
+    description:
+      The register offset (to syscon register address) and size
+    maxItems: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    syscon0: system-controller@440000 {
+        compatible = "syscon", "simple-mfd";
+        reg = <0x440000 0x2000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        swrst: reset-controller@268 {
+            compatible = "marvell,armada8k-reset";
+            reg = <0x268 0x4>;
+            #reset-cells = <1>;
+        };
+    };
diff --git a/include/dt-bindings/reset/marvell,armada8k-reset.h b/include/dt-bindings/reset/marvell,armada8k-reset.h
new file mode 100644
index 000000000000..18c6f4f761e2
--- /dev/null
+++ b/include/dt-bindings/reset/marvell,armada8k-reset.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2025, Marvell. All Rights Reserved.
+ */
+
+#ifndef _DT_BINDINGS_MARVELL_ARMADA8K_RESET_H_
+#define _DT_BINDINGS_MARVELL_ARMADA8K_RESET_H_
+
+#define CP110_RESET_AUDIO		0
+#define CP110_RESET_TDM			1
+#define CP110_RESET_ICU			2
+#define CP110_RESET_PP2			3
+#define CP110_RESET_SDIO		4
+#define CP110_RESET_XOR1		7
+#define CP110_RESET_XOR0		8
+#define CP110_RESET_PCIE0_X1		11
+#define CP110_RESET_PCIE1_X1		12
+#define CP110_RESET_PCIE_X4		13
+#define CP110_RESET_SATA		15
+#define CP110_RESET_USB3_HOST0		22
+#define CP110_RESET_USB3_HOST1		23
+#define CP110_RESET_USB3_DEV		24
+#define CP110_RESET_EIP150F		25
+#define CP110_RESET_EIP197		26
+#define CP110_RESET_MSS			29
+
+#endif /* _DT_BINDINGS_MARVELL_ARMADA8K_RESET_H_ */
-- 
2.43.0

[PATCH v3 2/3] 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 | 188 ++++++++++++++++++++++++++++
 include/linux/reset/reset-simple.h  |   3 +
 4 files changed, 204 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..798467f29911
--- /dev/null
+++ b/drivers/reset/reset-simple-syscon.c
@@ -0,0 +1,188 @@
+// 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_active_low = {
+	.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_active_low },
+	{ /* 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 (!device_property_read_u32_array(&pdev->dev, "reg", reg, 2)) {
+		data->reg_offset = reg[0];
+		data->rcdev.nr_resets = reg[1] * BITS_PER_BYTE;
+	}
+
+	spin_lock_init(&data->lock);
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.ops = &reset_simple_syscon_ops;
+	data->rcdev.of_node = dev->of_node;
+
+	if (devdata) {
+		if (devdata->reg_offset)
+			data->reg_offset = devdata->reg_offset;
+		if (devdata->nr_resets)
+			data->rcdev.nr_resets = devdata->nr_resets;
+		data->active_low = devdata->active_low;
+		data->status_active_low = devdata->status_active_low;
+	}
+
+	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 v3 3/3] 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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 161beec0b6b0..c27058d1534e 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
 		CP11X_LABEL(syscon0): system-controller@440000 {
 			compatible = "syscon", "simple-mfd";
 			reg = <0x440000 0x2000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
 
 			CP11X_LABEL(clk): clock {
 				compatible = "marvell,cp110-clock";
@@ -273,6 +275,12 @@ CP11X_LABEL(gpio2): gpio@140 {
 					 <&CP11X_LABEL(clk) 1 17>;
 				status = "disabled";
 			};
+
+			CP11X_LABEL(swrst): reset-controller@268 {
+				compatible = "marvell,armada8k-reset";
+				reg = <0x268 0x4>;
+				#reset-cells = <1>;
+			};
 		};
 
 		CP11X_LABEL(syscon1): system-controller@400000 {
-- 
2.43.0

Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Rob Herring (Arm)]

On Thu, 27 Feb 2025 11:25:34 -0800, Wilson Ding wrote:
> Add device-tree binding documentation for the Armada8K reset driver and
> create the new head file for the reset line index definitions.
> 
> Signed-off-by: Wilson Ding <dingwei@marvell.com>
> ---
>  .../reset/marvell,armada8k-reset.yaml         | 48 +++++++++++++++++++
>  .../reset/marvell,armada8k-reset.h            | 27 +++++++++++
>  2 files changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
>  create mode 100644 include/dt-bindings/reset/marvell,armada8k-reset.h
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/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/20250227192536.2426490-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: [EXTERNAL] Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Wilson Ding]

> -----Original Message-----
> From: Rob Herring (Arm) <robh@kernel.org>
> Sent: Thursday, February 27, 2025 12:24 PM
> To: Wilson Ding <dingwei@marvell.com>
> Cc: linux-arm-kernel@lists.infradead.org; Sanghoon Lee <salee@marvell.com>;
> conor+dt@kernel.org; linux-kernel@vger.kernel.org;
> gregory.clement@bootlin.com; p.zabel@pengutronix.de; Geethasowjanya
> Akula <gakula@marvell.com>; devicetree@vger.kernel.org; andrew@lunn.ch;
> krzk+dt@kernel.org; sebastian.hesselbarth@gmail.com
> Subject: [EXTERNAL] Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K
> reset controller
> 
> On Thu, 27 Feb 2025 11: 25: 34 -0800, Wilson Ding wrote: > Add device-tree
> binding documentation for the Armada8K reset driver and > create the new
> head file for the reset line index definitions. > > Signed-off-by: Wilson Ding
> <dingwei@ marvell. com>
> 
> 
> On Thu, 27 Feb 2025 11:25:34 -0800, Wilson Ding wrote:
> > Add device-tree binding documentation for the Armada8K reset driver and
> > create the new head file for the reset line index definitions.
> >
> > Signed-off-by: Wilson Ding <dingwei@marvell.com>
> > ---
> >  .../reset/marvell,armada8k-reset.yaml         | 48 +++++++++++++++++++
> >  .../reset/marvell,armada8k-reset.h            | 27 +++++++++++
> >  2 files changed, 75 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/reset/marvell,armada8k-reset.yaml
> >  create mode 100644 include/dt-bindings/reset/marvell,armada8k-reset.h
> >
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /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: https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__devicetree.org_schemas_mfd_syscon-2Dcommon.yaml-
> 23&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4GyqNVDlFUXakS
> GJl0Dh81ZIPlU26YS4KHGIA&m=xw0M--
> 2th9TCt05M0q_c8C0jvg1t4qbuXx9_d3WgCc0HOBpg5_f5E6TjXP_xdcrU&s=3
> Gdm4ABV10PnYEpAvJXrV9x-TsfBAHIp5KCn60ohngM&e=
> 
> doc reference errors (make refcheckdocs):
> 
> See https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patchwork.ozlabs.org_project_devicetree-
> 2Dbindings_patch_20250227192536.2426490-2D2-2Ddingwei-
> 40marvell.com&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4GyqN
> VDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=xw0M--
> 2th9TCt05M0q_c8C0jvg1t4qbuXx9_d3WgCc0HOBpg5_f5E6TjXP_xdcrU&s=
> WJznALBWhejS9hq88jCvZqevaNdN_5-meKfowRfl-bA&e=
> 
> 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.

I cannot reproduce the above error with the latest 'yamllint' and 'dt-schema'.

$ pip3 install dtschema --upgrade
Requirement already satisfied: dtschema in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (2025.2)
Requirement already satisfied: ruamel.yaml>0.15.69 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (0.18.10)
Requirement already satisfied: jsonschema<4.18,>=4.1.2 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (4.17.3)
Requirement already satisfied: rfc3987 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (1.3.8)
Requirement already satisfied: pylibfdt in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (1.7.2)
Requirement already satisfied: attrs>=17.4.0 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (25.1.0)
Requirement already satisfied: pyrsistent!=0.17.0,!=0.17.1,!=0.17.2,>=0.14.0 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (0.20.0)
Requirement already satisfied: ruamel.yaml.clib>=0.2.7 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from ruamel.yaml>0.15.69->dtschema) (0.2.12)
$ pip3 install yamllint
Requirement already satisfied: yamllint in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (1.35.1)
Requirement already satisfied: pathspec>=0.5.3 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from yamllint) (0.12.1)
Requirement already satisfied: pyyaml in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from yamllint) (6.0.2)
$ make dt_binding_check
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  CHKDT   ./Documentation/devicetree/bindings
  LINT    ./Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example.dts
  DTC [C] Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example.dtb

- Wilson

Re: [EXTERNAL] Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Krzysztof Kozlowski]

On 27/02/2025 22:58, Wilson Ding wrote:
>>
>> dtschema/dtc warnings/errors:
>> /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: https://urldefense.proofpoint.com/v2/url?u=http-
>> 3A__devicetree.org_schemas_mfd_syscon-2Dcommon.yaml-
>> 23&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4GyqNVDlFUXakS
>> GJl0Dh81ZIPlU26YS4KHGIA&m=xw0M--
>> 2th9TCt05M0q_c8C0jvg1t4qbuXx9_d3WgCc0HOBpg5_f5E6TjXP_xdcrU&s=3
>> Gdm4ABV10PnYEpAvJXrV9x-TsfBAHIp5KCn60ohngM&e=
>>
>> doc reference errors (make refcheckdocs):
>>
>> See https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__patchwork.ozlabs.org_project_devicetree-
>> 2Dbindings_patch_20250227192536.2426490-2D2-2Ddingwei-
>> 40marvell.com&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4GyqN
>> VDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=xw0M--
>> 2th9TCt05M0q_c8C0jvg1t4qbuXx9_d3WgCc0HOBpg5_f5E6TjXP_xdcrU&s=
>> WJznALBWhejS9hq88jCvZqevaNdN_5-meKfowRfl-bA&e=
>>
>> 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.
> 
> I cannot reproduce the above error with the latest 'yamllint' and 'dt-schema'.
> 
> $ pip3 install dtschema --upgrade
> Requirement already satisfied: dtschema in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (2025.2)
> Requirement already satisfied: ruamel.yaml>0.15.69 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (0.18.10)
> Requirement already satisfied: jsonschema<4.18,>=4.1.2 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (4.17.3)
> Requirement already satisfied: rfc3987 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (1.3.8)
> Requirement already satisfied: pylibfdt in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from dtschema) (1.7.2)
> Requirement already satisfied: attrs>=17.4.0 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (25.1.0)
> Requirement already satisfied: pyrsistent!=0.17.0,!=0.17.1,!=0.17.2,>=0.14.0 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from jsonschema<4.18,>=4.1.2->dtschema) (0.20.0)
> Requirement already satisfied: ruamel.yaml.clib>=0.2.7 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from ruamel.yaml>0.15.69->dtschema) (0.2.12)
> $ pip3 install yamllint
> Requirement already satisfied: yamllint in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (1.35.1)
> Requirement already satisfied: pathspec>=0.5.3 in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from yamllint) (0.12.1)
> Requirement already satisfied: pyyaml in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-packages (from yamllint) (6.0.2)
> $ make dt_binding_check
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>   CHKDT   ./Documentation/devicetree/bindings
>   LINT    ./Documentation/devicetree/bindings
>   DTEX    Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example.dts
>   DTC [C] Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example.dtb
> 

That's a bit odd but anyway warning is correct: you cannot have such
compatibles alone.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Krzysztof Kozlowski]

On 27/02/2025 20:25, Wilson Ding wrote:

> +    maxItems: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    syscon0: system-controller@440000 {
> +        compatible = "syscon", "simple-mfd";

Drop entire node. Neither correct, nor needed.

> +        reg = <0x440000 0x2000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        swrst: reset-controller@268 {
> +            compatible = "marvell,armada8k-reset";
> +            reg = <0x268 0x4>;
> +            #reset-cells = <1>;
> +        };



Best regards,
Krzysztof

Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Krzysztof Kozlowski]

On 27/02/2025 20:25, 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 | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> index 161beec0b6b0..c27058d1534e 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> @@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
>  		CP11X_LABEL(syscon0): system-controller@440000 {
>  			compatible = "syscon", "simple-mfd";
>  			reg = <0x440000 0x2000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
>  
>  			CP11X_LABEL(clk): clock {

Wait, no unit address here.

>  				compatible = "marvell,cp110-clock";
> @@ -273,6 +275,12 @@ CP11X_LABEL(gpio2): gpio@140 {
>  					 <&CP11X_LABEL(clk) 1 17>;
>  				status = "disabled";
>  			};
> +
> +			CP11X_LABEL(swrst): reset-controller@268 {


So why here it appeared? This is wrong and not even necessary. Entire
child should be folded into parent, so finally you will fix the
incomplete parent compatible.



Best regards,
Krzysztof

RE: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, February 27, 2025 10:57 PM
> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x: Add reset
> controller node
> 
> On 27/02/2025 20: 25, 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
> 
> On 27/02/2025 20:25, 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 | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> > index 161beec0b6b0..c27058d1534e 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> > +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> > @@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
> >  		CP11X_LABEL(syscon0): system-controller@440000 {
> >  			compatible = "syscon", "simple-mfd";
> >  			reg = <0x440000 0x2000>;
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> >
> >  			CP11X_LABEL(clk): clock {
> 
> Wait, no unit address here.

This subnode came from the existing code. I didn't touch this subnode
in my patch. As you can see, the system-controller has a wide address
range, which includes clock, GPIO registers as well as the unit-softreset
register.

> 
> >  				compatible = "marvell,cp110-clock";
> > @@ -273,6 +275,12 @@ CP11X_LABEL(gpio2): gpio@140 {
> >  					 <&CP11X_LABEL(clk) 1 17>;
> >  				status = "disabled";
> >  			};
> > +
> > +			CP11X_LABEL(swrst): reset-controller@268 {
> 
> 
> So why here it appeared? This is wrong and not even necessary. Entire
> child should be folded into parent, so finally you will fix the
> incomplete parent compatible.

We do need the reset-controller as a subnode under system-controller node
for the following reasons:

- We need to have 'reg' property in this subnode so that we can get the offset
  to system-controller register base defined in parent node. This is suggested
  by Rob in V2 comments. 
  And we need to know the register size to calculate the number of reset lines.
  This is suggested by Philipp in V1 comments.

- We also need to define the 'reset-cells' in this subnode. And the consumer of
  the reset controller uses the label of this subnode for the phandle and reset
  specifier pair. 

As I mentioned in my reply to the first comment, the reset-controller is not the
only device within the system-controller register spaces. Do you still think I
should fold it into the parent node. And what I proposed is exactly same as
that the armada_thermal driver did (See below). I wonder why what was accepted
in the past become not accepted now. 

CP11X_LABEL(syscon1): system-controller@400000 {
	compatible = "syscon", "simple-mfd";
	reg = <0x400000 0x1000>;
	#address-cells = <1>;
	#size-cells = <1>;

	CP11X_LABEL(thermal): thermal-sensor@70 {
		compatible = "marvell,armada-cp110-thermal";
		reg = <0x70 0x10>;
		interrupts-extended =
			<&CP11X_LABEL(icu_sei) 116 IRQ_TYPE_LEVEL_HIGH>;
		#thermal-sensor-cells = <1>;
	};
};

> 
> 
> 
> Best regards,
> Krzysztof

Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Krzysztof Kozlowski]

On 28/02/2025 21:18, Wilson Ding wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Thursday, February 27, 2025 10:57 PM
>> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x: Add reset
>> controller node
>>
>> On 27/02/2025 20: 25, 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
>>
>> On 27/02/2025 20:25, 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 | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>> b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>> index 161beec0b6b0..c27058d1534e 100644
>>> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>> @@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
>>>  		CP11X_LABEL(syscon0): system-controller@440000 {
>>>  			compatible = "syscon", "simple-mfd";
>>>  			reg = <0x440000 0x2000>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>>
>>>  			CP11X_LABEL(clk): clock {
>>
>> Wait, no unit address here.
> 
> This subnode came from the existing code. I didn't touch this subnode
> in my patch. As you can see, the system-controller has a wide address
> range, which includes clock, GPIO registers as well as the unit-softreset
> register.
> 
>>
>>>  				compatible = "marvell,cp110-clock";
>>> @@ -273,6 +275,12 @@ CP11X_LABEL(gpio2): gpio@140 {
>>>  					 <&CP11X_LABEL(clk) 1 17>;
>>>  				status = "disabled";
>>>  			};
>>> +
>>> +			CP11X_LABEL(swrst): reset-controller@268 {
>>
>>
>> So why here it appeared? This is wrong and not even necessary. Entire
>> child should be folded into parent, so finally you will fix the
>> incomplete parent compatible.
> 
> We do need the reset-controller as a subnode under system-controller node
> for the following reasons:
> 
> - We need to have 'reg' property in this subnode so that we can get the offset
>   to system-controller register base defined in parent node. This is suggested
>   by Rob in V2 comments. 
>   And we need to know the register size to calculate the number of reset lines.
>   This is suggested by Philipp in V1 comments.

You do not need and you received that comment as well. It is implied by
compatible.

> 
> - We also need to define the 'reset-cells' in this subnode. And the consumer of
>   the reset controller uses the label of this subnode for the phandle and reset
>   specifier pair. 

reset-cells will be in the parent once you fold it.

> 
> As I mentioned in my reply to the first comment, the reset-controller is not the
> only device within the system-controller register spaces. Do you still think I

You provided very little hardware description of the device. So based on
hardware description you provided: yes.

> should fold it into the parent node. And what I proposed is exactly same as
> that the armada_thermal driver did (See below). I wonder why what was accepted
> in the past become not accepted now. 

We did not discuss here drivers, but if you insist talking about
"marvell,armada-cp110-thermal" then point me to review or ack from DT
people. You claim it was accepted so how did we accept it?

It was 2013 so that's another answer: many things done 12 years ago were
done not according to best practices. Also best practices evolved.

Best regards,
Krzysztof

Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Krzysztof Kozlowski]

On 01/03/2025 14:46, Krzysztof Kozlowski wrote:
>>>> Signed-off-by: Wilson Ding <dingwei@marvell.com>
>>>> ---
>>>>  arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>> b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>>> index 161beec0b6b0..c27058d1534e 100644
>>>> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>>> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>>> @@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
>>>>  		CP11X_LABEL(syscon0): system-controller@440000 {
>>>>  			compatible = "syscon", "simple-mfd";
>>>>  			reg = <0x440000 0x2000>;
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <1>;
>>>>
>>>>  			CP11X_LABEL(clk): clock {
>>>
>>> Wait, no unit address here.
>>
>> This subnode came from the existing code. I didn't touch this subnode
>> in my patch. As you can see, the system-controller has a wide address
>> range, which includes clock, GPIO registers as well as the unit-softreset
>> register.
>>
>>>
>>>>  				compatible = "marvell,cp110-clock";
>>>> @@ -273,6 +275,12 @@ CP11X_LABEL(gpio2): gpio@140 {
>>>>  					 <&CP11X_LABEL(clk) 1 17>;
>>>>  				status = "disabled";
>>>>  			};
>>>> +
>>>> +			CP11X_LABEL(swrst): reset-controller@268 {
>>>
>>>
>>> So why here it appeared? This is wrong and not even necessary. Entire
>>> child should be folded into parent, so finally you will fix the
>>> incomplete parent compatible.
>>
>> We do need the reset-controller as a subnode under system-controller node
>> for the following reasons:
>>
>> - We need to have 'reg' property in this subnode so that we can get the offset
>>   to system-controller register base defined in parent node. This is suggested
>>   by Rob in V2 comments. 
>>   And we need to know the register size to calculate the number of reset lines.
>>   This is suggested by Philipp in V1 comments.
> 
> You do not need and you received that comment as well. It is implied by
> compatible.
> 
>>
>> - We also need to define the 'reset-cells' in this subnode. And the consumer of
>>   the reset controller uses the label of this subnode for the phandle and reset
>>   specifier pair. 
> 
> reset-cells will be in the parent once you fold it.
> 
>>
>> As I mentioned in my reply to the first comment, the reset-controller is not the
>> only device within the system-controller register spaces. Do you still think I
> 
> You provided very little hardware description of the device. So based on
> hardware description you provided: yes.

and to clarify - by device I mean the parent node, the system controller.

Your commit even mentions driver, not the hardware:
"Add device-tree binding documentation for the Armada8K reset driver..."
not hardware, so you will get the review as good as you describe things.

Bindings are about hardware so if you disagree with the review here,
please provide arguments in terms of hardware.

> 
>> should fold it into the parent node. And what I proposed is exactly same as
>> that the armada_thermal driver did (See below). I wonder why what was accepted
>> in the past become not accepted now. 
> 
> We did not discuss here drivers, but if you insist talking about
> "marvell,armada-cp110-thermal" then point me to review or ack from DT
> people. You claim it was accepted so how did we accept it?
> 
> It was 2013 so that's another answer: many things done 12 years ago were
> done not according to best practices. Also best practices evolved.



Best regards,
Krzysztof

RE: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Saturday, March 1, 2025 5:46 AM
> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: 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: Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add
> reset controller node
> 
> On 28/02/2025 21:18, Wilson Ding wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Thursday, February 27, 2025 10:57 PM
> >> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> >> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x: Add
> reset
> >> controller node
> >>
> >> On 27/02/2025 20: 25, 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
> >>
> >> On 27/02/2025 20:25, 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 | 8 ++++++++
> >>>  1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >> b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >>> index 161beec0b6b0..c27058d1534e 100644
> >>> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >>> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >>> @@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
> >>>  		CP11X_LABEL(syscon0): system-controller@440000 {
> >>>  			compatible = "syscon", "simple-mfd";
> >>>  			reg = <0x440000 0x2000>;
> >>> +			#address-cells = <1>;
> >>> +			#size-cells = <1>;
> >>>
> >>>  			CP11X_LABEL(clk): clock {
> >>
> >> Wait, no unit address here.
> >
> > This subnode came from the existing code. I didn't touch this subnode
> > in my patch. As you can see, the system-controller has a wide address
> > range, which includes clock, GPIO registers as well as the unit-softreset
> > register.
> >
> >>
> >>>  				compatible = "marvell,cp110-clock";
> >>> @@ -273,6 +275,12 @@ CP11X_LABEL(gpio2): gpio@140 {
> >>>  					 <&CP11X_LABEL(clk) 1 17>;
> >>>  				status = "disabled";
> >>>  			};
> >>> +
> >>> +			CP11X_LABEL(swrst): reset-controller@268 {
> >>
> >>
> >> So why here it appeared? This is wrong and not even necessary. Entire
> >> child should be folded into parent, so finally you will fix the
> >> incomplete parent compatible.
> >
> > We do need the reset-controller as a subnode under system-controller node
> > for the following reasons:
> >
> > - We need to have 'reg' property in this subnode so that we can get the
> offset
> >   to system-controller register base defined in parent node. This is suggested
> >   by Rob in V2 comments.
> >   And we need to know the register size to calculate the number of reset
> lines.
> >   This is suggested by Philipp in V1 comments.
> 
> You do not need and you received that comment as well. It is implied by
> compatible.
> 
> >
> > - We also need to define the 'reset-cells' in this subnode. And the consumer
> of
> >   the reset controller uses the label of this subnode for the phandle and reset
> >   specifier pair.
> 
> reset-cells will be in the parent once you fold it.
> 
> >
> > As I mentioned in my reply to the first comment, the reset-controller is not
> the
> > only device within the system-controller register spaces. Do you still think I
> 
> You provided very little hardware description of the device. So based on
> hardware description you provided: yes.
> 
> > should fold it into the parent node. And what I proposed is exactly same as
> > that the armada_thermal driver did (See below). I wonder why what was
> accepted
> > in the past become not accepted now.
> 
> We did not discuss here drivers, but if you insist talking about
> "marvell,armada-cp110-thermal" then point me to review or ack from DT
> people. You claim it was accepted so how did we accept it?
> 

I didn't intend to extend discussion to the driver in this thread. The following
Is the review thread of the dt-binding for the thermal device (in 2018).
Indeed, there is no comments challenging why not fold the thermal sub-node
Into the parent 'syscon' node.

https://lore.kernel.org/linux-arm-kernel/20180703211335.GA8858@rob-hp-laptop/

Digging further, I found some interesting history about the parent 'syscon' node
of the reset-controller. I'd appreciate if you can take a look into the following
patches/thread -

The syscon0 node was initially added along with Armada clock driver support.
It was the very beginning of the upstream for Armada SoCs support (2016).
And the clock driver is one of the earliest drivers to be mainlined. At that time,
the clock controller is the only supported device within sycon register range.
As you can see, the clock dt-binding was exactly aligned with what your suggested
(no sub-node, compatible and clock-cells just in syscon). 

https://lore.kernel.org/all/1460648013-31320-5-git-send-email-thomas.petazzoni@free-electrons.com/

Besides the clock controller, the system controller also includes the GPIO controller,
pinctl controller, reset controller and other miscellaneous configurations. Before
adding the pinctl dt-binding, it's decided to use the sub-nodes to present the multiple
function blocks of various devices.

https://lore.kernel.org/all/b27495e10fb4f4d8a7fd1a760d49402bbae83b58.1496328934.git-series.gregory.clement@free-electrons.com/

In the following patch, it was clearly addressed why sub-nodes was chosen
over one flat node.

https://lore.kernel.org/all/bb21ee9acc55efac884450ff710049b99b27f8bf.1496328934.git-series.gregory.clement@free-electrons.com/

"The initial intent when the binding of the cp110 system controller was to
have one flat node. The idea being that what is currently a clock-only
driver in drivers would become a MFD driver, exposing the clock, GPIO and
pinctrl functionality. However, after taking a step back, this would lead
to a messy binding. Indeed, a single node would be a GPIO controller,
clock controller, pinmux controller, and more.

This patch adopts a more classical solution of a top-level syscon node
with sub-nodes for the individual devices. The main benefit will be to
have each functional block associated to its own sub-node where we can
put its own properties."

Since then, the dt-binding of Armada's system controller became an
exception. But I think it's sensible. If we do put all these controllers into
one node, you can image the properties of different devices will be
messed up, e.g., not just #reset-cells, #clock-cells and #gpio-cells will
be gathered. There will be a long compatible list of all devices.

Going back to my current patch - if we fold the reset controller into the
parent node, the syscon node will become a hybrid, which GPIO and 
clock controller are still sub-nodes while reset controller is folded into
the syscon node. Isn't it very confusing?

> It was 2013 so that's another answer: many things done 12 years ago were
> done not according to best practices. Also best practices evolved.
> 
> Best regards,
> Krzysztof

Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Krzysztof Kozlowski]

On 04/03/2025 03:17, Wilson Ding wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Saturday, March 1, 2025 5:46 AM
>> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>> Cc: 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: Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add
>> reset controller node
>>
>> On 28/02/2025 21:18, Wilson Ding wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <krzk@kernel.org>
>>>> Sent: Thursday, February 27, 2025 10:57 PM
>>>> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
>>>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>>>> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x: Add
>> reset
>>>> controller node
>>>>
>>>> On 27/02/2025 20: 25, 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
>>>>
>>>> On 27/02/2025 20:25, 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 | 8 ++++++++
>>>>>  1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>>> b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>>>> index 161beec0b6b0..c27058d1534e 100644
>>>>> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>>>> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
>>>>> @@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
>>>>>  		CP11X_LABEL(syscon0): system-controller@440000 {
>>>>>  			compatible = "syscon", "simple-mfd";
>>>>>  			reg = <0x440000 0x2000>;
>>>>> +			#address-cells = <1>;
>>>>> +			#size-cells = <1>;
>>>>>
>>>>>  			CP11X_LABEL(clk): clock {
>>>>
>>>> Wait, no unit address here.
>>>
>>> This subnode came from the existing code. I didn't touch this subnode
>>> in my patch. As you can see, the system-controller has a wide address
>>> range, which includes clock, GPIO registers as well as the unit-softreset
>>> register.
>>>
>>>>
>>>>>  				compatible = "marvell,cp110-clock";
>>>>> @@ -273,6 +275,12 @@ CP11X_LABEL(gpio2): gpio@140 {
>>>>>  					 <&CP11X_LABEL(clk) 1 17>;
>>>>>  				status = "disabled";
>>>>>  			};
>>>>> +
>>>>> +			CP11X_LABEL(swrst): reset-controller@268 {
>>>>
>>>>
>>>> So why here it appeared? This is wrong and not even necessary. Entire
>>>> child should be folded into parent, so finally you will fix the
>>>> incomplete parent compatible.
>>>
>>> We do need the reset-controller as a subnode under system-controller node
>>> for the following reasons:
>>>
>>> - We need to have 'reg' property in this subnode so that we can get the
>> offset
>>>   to system-controller register base defined in parent node. This is suggested
>>>   by Rob in V2 comments.
>>>   And we need to know the register size to calculate the number of reset
>> lines.
>>>   This is suggested by Philipp in V1 comments.
>>
>> You do not need and you received that comment as well. It is implied by
>> compatible.
>>
>>>
>>> - We also need to define the 'reset-cells' in this subnode. And the consumer
>> of
>>>   the reset controller uses the label of this subnode for the phandle and reset
>>>   specifier pair.
>>
>> reset-cells will be in the parent once you fold it.
>>
>>>
>>> As I mentioned in my reply to the first comment, the reset-controller is not
>> the
>>> only device within the system-controller register spaces. Do you still think I
>>
>> You provided very little hardware description of the device. So based on
>> hardware description you provided: yes.
>>
>>> should fold it into the parent node. And what I proposed is exactly same as
>>> that the armada_thermal driver did (See below). I wonder why what was
>> accepted
>>> in the past become not accepted now.
>>
>> We did not discuss here drivers, but if you insist talking about
>> "marvell,armada-cp110-thermal" then point me to review or ack from DT
>> people. You claim it was accepted so how did we accept it?
>>
> 
> I didn't intend to extend discussion to the driver in this thread. The following
> Is the review thread of the dt-binding for the thermal device (in 2018).
> Indeed, there is no comments challenging why not fold the thermal sub-node
> Into the parent 'syscon' node.
> 
> https://lore.kernel.org/linux-arm-kernel/20180703211335.GA8858@rob-hp-laptop/

Indeed, this one got review. I was checking armada-thermal and it never
got any, when it was merged back in the 2013.

> 
> Digging further, I found some interesting history about the parent 'syscon' node
> of the reset-controller. I'd appreciate if you can take a look into the following
> patches/thread -
> 
> The syscon0 node was initially added along with Armada clock driver support.
> It was the very beginning of the upstream for Armada SoCs support (2016).
> And the clock driver is one of the earliest drivers to be mainlined. At that time,
> the clock controller is the only supported device within sycon register range.
> As you can see, the clock dt-binding was exactly aligned with what your suggested
> (no sub-node, compatible and clock-cells just in syscon). 
> 
> https://lore.kernel.org/all/1460648013-31320-5-git-send-email-thomas.petazzoni@free-electrons.com/
> 
> Besides the clock controller, the system controller also includes the GPIO controller,
> pinctl controller, reset controller and other miscellaneous configurations. Before
> adding the pinctl dt-binding, it's decided to use the sub-nodes to present the multiple
> function blocks of various devices.
> 
> https://lore.kernel.org/all/b27495e10fb4f4d8a7fd1a760d49402bbae83b58.1496328934.git-series.gregory.clement@free-electrons.com/

So this is the source here. I see.

Commit comes with a rationale that it will grow significantly.

> 
> In the following patch, it was clearly addressed why sub-nodes was chosen
> over one flat node.
> 
> https://lore.kernel.org/all/bb21ee9acc55efac884450ff710049b99b27f8bf.1496328934.git-series.gregory.clement@free-electrons.com/
> 
> "The initial intent when the binding of the cp110 system controller was to
> have one flat node. The idea being that what is currently a clock-only
> driver in drivers would become a MFD driver, exposing the clock, GPIO and
> pinctrl functionality. However, after taking a step back, this would lead
> to a messy binding. Indeed, a single node would be a GPIO controller,
> clock controller, pinmux controller, and more.
> 
> This patch adopts a more classical solution of a top-level syscon node
> with sub-nodes for the individual devices. The main benefit will be to
> have each functional block associated to its own sub-node where we can
> put its own properties."
> 
> Since then, the dt-binding of Armada's system controller became an
> exception. But I think it's sensible. If we do put all these controllers into
> one node, you can image the properties of different devices will be
> messed up, e.g., not just #reset-cells, #clock-cells and #gpio-cells will
> be gathered. There will be a long compatible list of all devices.
> 
> Going back to my current patch - if we fold the reset controller into the
> parent node, the syscon node will become a hybrid, which GPIO and 
> clock controller are still sub-nodes while reset controller is folded into
> the syscon node. Isn't it very confusing?

Yes, it will be. But more confusing is existing pattern of mixing MMIO
nodes with non-MMIO which you grow. So okay, keep them as separate
child, but drop offset in your patch or unify everything into 'reg'.


Best regards,
Krzysztof

Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, March 3, 2025 11:14 PM
> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> robh@kernel.org
> Cc: 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: Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add
> reset controller node
> 
> On 04/03/2025 03:17, Wilson Ding wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Saturday, March 1, 2025 5:46 AM
> >> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> >> Cc: 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: Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell:
> >> cp11x: Add reset controller node
> >>
> >> On 28/02/2025 21:18, Wilson Ding wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Krzysztof Kozlowski <krzk@kernel.org>
> >>>> Sent: Thursday, February 27, 2025 10:57 PM
> >>>> To: Wilson Ding <dingwei@marvell.com>;
> >>>> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> >>>> linux-arm-kernel@lists.infradead.org
> >>>> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x:
> >>>> Add
> >> reset
> >>>> controller node
> >>>>
> >>>> On 27/02/2025 20: 25, 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
> >>>>
> >>>> On 27/02/2025 20:25, 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 | 8 ++++++++
> >>>>>  1 file changed, 8 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >>>> b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >>>>> index 161beec0b6b0..c27058d1534e 100644
> >>>>> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> >>>>> @@ -226,6 +226,8 @@ CP11X_LABEL(rtc): rtc@284000 {
> >>>>>  		CP11X_LABEL(syscon0): system-controller@440000 {
> >>>>>  			compatible = "syscon", "simple-mfd";
> >>>>>  			reg = <0x440000 0x2000>;
> >>>>> +			#address-cells = <1>;
> >>>>> +			#size-cells = <1>;
> >>>>>
> >>>>>  			CP11X_LABEL(clk): clock {
> >>>>
> >>>> Wait, no unit address here.
> >>>
> >>> This subnode came from the existing code. I didn't touch this
> >>> subnode in my patch. As you can see, the system-controller has a
> >>> wide address range, which includes clock, GPIO registers as well as
> >>> the unit-softreset register.
> >>>
> >>>>
> >>>>>  				compatible = "marvell,cp110-clock"; @@ -
> 273,6 +275,12 @@
> >>>>> CP11X_LABEL(gpio2): gpio@140 {
> >>>>>  					 <&CP11X_LABEL(clk) 1 17>;
> >>>>>  				status = "disabled";
> >>>>>  			};
> >>>>> +
> >>>>> +			CP11X_LABEL(swrst): reset-controller@268 {
> >>>>
> >>>>
> >>>> So why here it appeared? This is wrong and not even necessary.
> >>>> Entire child should be folded into parent, so finally you will fix
> >>>> the incomplete parent compatible.
> >>>
> >>> We do need the reset-controller as a subnode under system-controller
> >>> node for the following reasons:
> >>>
> >>> - We need to have 'reg' property in this subnode so that we can get
> >>> the
> >> offset
> >>>   to system-controller register base defined in parent node. This is
> suggested
> >>>   by Rob in V2 comments.
> >>>   And we need to know the register size to calculate the number of
> >>> reset
> >> lines.
> >>>   This is suggested by Philipp in V1 comments.
> >>
> >> You do not need and you received that comment as well. It is implied
> >> by compatible.
> >>
> >>>
> >>> - We also need to define the 'reset-cells' in this subnode. And the
> >>> consumer
> >> of
> >>>   the reset controller uses the label of this subnode for the phandle and
> reset
> >>>   specifier pair.
> >>
> >> reset-cells will be in the parent once you fold it.
> >>
> >>>
> >>> As I mentioned in my reply to the first comment, the
> >>> reset-controller is not
> >> the
> >>> only device within the system-controller register spaces. Do you
> >>> still think I
> >>
> >> You provided very little hardware description of the device. So based
> >> on hardware description you provided: yes.
> >>
> >>> should fold it into the parent node. And what I proposed is exactly
> >>> same as that the armada_thermal driver did (See below). I wonder why
> >>> what was
> >> accepted
> >>> in the past become not accepted now.
> >>
> >> We did not discuss here drivers, but if you insist talking about
> >> "marvell,armada-cp110-thermal" then point me to review or ack from DT
> >> people. You claim it was accepted so how did we accept it?
> >>
> >
> > I didn't intend to extend discussion to the driver in this thread. The
> > following Is the review thread of the dt-binding for the thermal device (in
> 2018).
> > Indeed, there is no comments challenging why not fold the thermal
> > sub-node Into the parent 'syscon' node.
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_l
> > inux-2Darm-2Dkernel_20180703211335.GA8858-40rob-2Dhp-
> 2Dlaptop_&d=DwIDa
> >
> Q&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4GyqNVDlFUXakSGJl0Dh81ZIPl
> U26YS4KHG
> >
> IA&m=38vmzSKDh1f33oyqXyKxLMelJiyurSZl38hR43cMNb1JmGrdorIuj2bCEn
> HRs37W&
> > s=MsDwvAhgZcRztTZgawJ_imU4Ld5aZebUxhtCaXUgY5A&e=
> 
> Indeed, this one got review. I was checking armada-thermal and it never got
> any, when it was merged back in the 2013.
> 
> >
> > Digging further, I found some interesting history about the parent
> > 'syscon' node of the reset-controller. I'd appreciate if you can take
> > a look into the following patches/thread -
> >
> > The syscon0 node was initially added along with Armada clock driver
> support.
> > It was the very beginning of the upstream for Armada SoCs support (2016).
> > And the clock driver is one of the earliest drivers to be mainlined.
> > At that time, the clock controller is the only supported device within sycon
> register range.
> > As you can see, the clock dt-binding was exactly aligned with what
> > your suggested (no sub-node, compatible and clock-cells just in syscon).
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_a
> > ll_1460648013-2D31320-2D5-2Dgit-2Dsend-2Demail-
> 2Dthomas.petazzoni-40fr
> > ee-
> 2Delectrons.com_&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4Gy
> qNVDl
> >
> FUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=38vmzSKDh1f33oyqXyKxLMelJiyurSZl
> 38hR43
> > cMNb1JmGrdorIuj2bCEnHRs37W&s=pvsI6D-
> tAm32RQzrFFUl7pAclLGStG4bBgQ_iHs2R
> > _Q&e=
> >
> > Besides the clock controller, the system controller also includes the
> > GPIO controller, pinctl controller, reset controller and other
> > miscellaneous configurations. Before adding the pinctl dt-binding,
> > it's decided to use the sub-nodes to present the multiple function blocks of
> various devices.
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_a
> > ll_b27495e10fb4f4d8a7fd1a760d49402bbae83b58.1496328934.git-
> 2Dseries.gr
> > egory.clement-40free-
> 2Delectrons.com_&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtf
> >
> Q&r=sXDQZu4GyqNVDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=38vmzSKDh1
> f33oyqXyKx
> >
> LMelJiyurSZl38hR43cMNb1JmGrdorIuj2bCEnHRs37W&s=R58U_4PGdQrcLB
> Wh1gfNnZe
> > voNQfbABbW5QlvgOrAGY&e=
> 
> So this is the source here. I see.
> 
> Commit comes with a rationale that it will grow significantly.
> 
> >
> > In the following patch, it was clearly addressed why sub-nodes was
> > chosen over one flat node.
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_a
> > ll_bb21ee9acc55efac884450ff710049b99b27f8bf.1496328934.git-
> 2Dseries.gr
> > egory.clement-40free-
> 2Delectrons.com_&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtf
> >
> Q&r=sXDQZu4GyqNVDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=38vmzSKDh1
> f33oyqXyKx
> >
> LMelJiyurSZl38hR43cMNb1JmGrdorIuj2bCEnHRs37W&s=8U7S14wRqQiL6eO
> C6G2GPpY
> > tlvgFIbmNmEjljAKDdCA&e=
> >
> > "The initial intent when the binding of the cp110 system controller
> > was to have one flat node. The idea being that what is currently a
> > clock-only driver in drivers would become a MFD driver, exposing the
> > clock, GPIO and pinctrl functionality. However, after taking a step
> > back, this would lead to a messy binding. Indeed, a single node would
> > be a GPIO controller, clock controller, pinmux controller, and more.
> >
> > This patch adopts a more classical solution of a top-level syscon node
> > with sub-nodes for the individual devices. The main benefit will be to
> > have each functional block associated to its own sub-node where we can
> > put its own properties."
> >
> > Since then, the dt-binding of Armada's system controller became an
> > exception. But I think it's sensible. If we do put all these
> > controllers into one node, you can image the properties of different
> > devices will be messed up, e.g., not just #reset-cells, #clock-cells
> > and #gpio-cells will be gathered. There will be a long compatible list of all
> devices.
> >
> > Going back to my current patch - if we fold the reset controller into
> > the parent node, the syscon node will become a hybrid, which GPIO and
> > clock controller are still sub-nodes while reset controller is folded
> > into the syscon node. Isn't it very confusing?
> 
> Yes, it will be. But more confusing is existing pattern of mixing MMIO nodes
> with non-MMIO which you grow.So okay, keep them as separate child, but

I did consider shrinking the syscon's register address range to
make the reset-controller node to be independent from the
syscon node. However, I found the syscon node is also referred
by some devices for miscellaneous configurations . The reset
configuration register happens to be located in between these
registers and clock/GPIO registers.

> drop offset in your patch or unify everything into 'reg'.
> 

This is exactly what I proposed in v3 patch. Do I misunderstand
you?

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

> 
> Best regards,
> Krzysztof

Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Krzysztof Kozlowski]

On 04/03/2025 20:08, Wilson Ding wrote:
> 
> I did consider shrinking the syscon's register address range to
> make the reset-controller node to be independent from the
> syscon node. However, I found the syscon node is also referred
> by some devices for miscellaneous configurations . The reset
> configuration register happens to be located in between these
> registers and clock/GPIO registers.
> 
>> drop offset in your patch or unify everything into 'reg'.
>>
> 
> This is exactly what I proposed in v3 patch. Do I misunderstand
> you?
> 
> CP11X_LABEL(swrst): reset-controller@268 {
> 	compatible = "marvell,armada8k-reset";
> 	reg = <0x268 0x4>;
> 	#reset-cells = <1>;
> };

I don't see the other device being fixed here. How did you unify them?

Best regards,
Krzysztof

RE: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, March 5, 2025 11:29 PM
> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> robh@kernel.org
> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x: Add reset
> controller node
> 
> On 04/03/2025 20:08, Wilson Ding wrote:
> >
> > I did consider shrinking the syscon's register address range to make
> > the reset-controller node to be independent from the syscon node.
> > However, I found the syscon node is also referred by some devices for
> > miscellaneous configurations . The reset configuration register
> > happens to be located in between these registers and clock/GPIO
> > registers.
> >
> >> drop offset in your patch or unify everything into 'reg'.
> >>
> >
> > This is exactly what I proposed in v3 patch. Do I misunderstand you?
> >
> > CP11X_LABEL(swrst): reset-controller@268 {
> > 	compatible = "marvell,armada8k-reset";
> > 	reg = <0x268 0x4>;
> > 	#reset-cells = <1>;
> > };
> 
> I don't see the other device being fixed here. How did you unify them?

This patch series is about the proposal of Armada8K's reset controller
dt-binding. The dt-bindings issues of clock/GPIO controllers have been
there for years. Having to say, it is not just a simple patch to fix it. It
will require to convert the dt-binding document into json schemas as
well as adapt these dt changes in the drivers. So I would suggest to
fixing it in later patches. How do you think about it?

> 
> Best regards,
> Krzysztof

Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, February 27, 2025 10:53 PM
> To: Wilson Ding <dingwei@marvell.com>; Rob Herring (Arm)
> <robh@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org; Sanghoon Lee <salee@marvell.com>;
> conor+dt@kernel.org; linux-kernel@vger.kernel.org;
> gregory.clement@bootlin.com; p.zabel@pengutronix.de; Geethasowjanya
> Akula <gakula@marvell.com>; devicetree@vger.kernel.org; andrew@lunn.ch;
> krzk+dt@kernel.org; sebastian.hesselbarth@gmail.com
> Subject: Re: [EXTERNAL] Re: [PATCH v3 1/3] dt-bindings: reset: Add
> Armada8K reset controller
> 
> On 27/02/2025 22:58, Wilson Ding wrote:
> >>
> >> dtschema/dtc warnings/errors:
> >> /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: https://urldefense.proofpoint.com/v2/url?u=http-
> >> <https://urldefense.proofpoint.com/v2/url?u=http-  >>>
> >> 3A__devicetree.org_schemas_mfd_syscon-2Dcommon.yaml-
> >>
> 23&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4GyqNVDlFUXakS
> >> GJl0Dh81ZIPlU26YS4KHGIA&m=xw0M--
> >>
> 2th9TCt05M0q_c8C0jvg1t4qbuXx9_d3WgCc0HOBpg5_f5E6TjXP_xdcrU&s=3
> >> Gdm4ABV10PnYEpAvJXrV9x-TsfBAHIp5KCn60ohngM&e=
> >>
> >> doc reference errors (make refcheckdocs):
> >>
> >> See https://urldefense.proofpoint.com/v2/url?u=https-
> >> <https://urldefense.proofpoint.com/v2/url?u=https-  >>>
> >> 3A__patchwork.ozlabs.org_project_devicetree-
> >> 2Dbindings_patch_20250227192536.2426490-2D2-2Ddingwei-
> >>
> 40marvell.com&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4GyqN
> >> VDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=xw0M--
> >>
> 2th9TCt05M0q_c8C0jvg1t4qbuXx9_d3WgCc0HOBpg5_f5E6TjXP_xdcrU&s=
> >> WJznALBWhejS9hq88jCvZqevaNdN_5-meKfowRfl-bA&e=
> >>
> >> 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.
> >
> > I cannot reproduce the above error with the latest 'yamllint' and 'dt-schema'.
> >
> > $ pip3 install dtschema --upgrade
> > Requirement already satisfied: dtschema in
> > /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> package
> > s (2025.2) Requirement already satisfied: ruamel.yaml>0.15.69 in
> > /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> package
> > s (from dtschema) (0.18.10) Requirement already satisfied:
> > jsonschema<4.18,>=4.1.2 in
> > /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> package
> > s (from dtschema) (4.17.3) Requirement already satisfied: rfc3987 in
> > /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> package
> > s (from dtschema) (1.3.8) Requirement already satisfied: pylibfdt in
> > /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> package
> > s (from dtschema) (1.7.2) Requirement already satisfied: attrs>=17.4.0
> > in /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> packages (from jsonschema<4.18,>=4.1.2->dtschema) (25.1.0) Requirement
> already satisfied: pyrsistent!=0.17.0,!=0.17.1,!=0.17.2,>=0.14.0 in
> /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> packages (from jsonschema<4.18,>=4.1.2->dtschema) (0.20.0) Requirement
> already satisfied: ruamel.yaml.clib>=0.2.7 in
> /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> packages (from ruamel.yaml>0.15.69->dtschema) (0.2.12) $ pip3 install
> yamllint Requirement already satisfied: yamllint in
> /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> packages (1.35.1) Requirement already satisfied: pathspec>=0.5.3 in
> /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> packages (from yamllint) (0.12.1) Requirement already satisfied: pyyaml in
> /home/wilson/workspace/repos/external/venv/lib/python3.12/site-
> packages (from yamllint) (6.0.2) $ make dt_binding_check
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> >   CHKDT   ./Documentation/devicetree/bindings
> >   LINT    ./Documentation/devicetree/bindings
> >   DTEX    Documentation/devicetree/bindings/reset/marvell,armada8k-
> reset.example.dts
> >   DTC [C]
> > Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example
> > .dtb
> >
> 
> That's a bit odd but anyway warning is correct: you cannot have such
> compatibles alone.

I understand I need to add one compatible here to resolve the
warning. However, as we agreed, we keep the sud-nodes while
there will be no new compatibles in parent node. So how shall
I avoid this warning?

If we do want to add a compatible in parent node, what should
be used for the compatible name? I think the most suitable name
would be something like "marvell,cp110-system-controller0".
However, it was already taken for in Armada clock controller to
be compatible for legacy dt.

https://lore.kernel.org/all/bb21ee9acc55efac884450ff710049b99b27f8bf.1496328934.git-series.gregory.clement@free-electrons.com/

This is something about 8 years ago. I wonder if we do need to
have the backward compatibility for the field devices at that
time. Can we drop it now?

    syscon0: system-controller@440000 {
        compatible = "marvell,cp110-system-controller0",
                                "syscon", "simple-mfd";
        reg = <0x440000 0x2000>;
        #address-cells = <1>;
        #size-cells = <1>;

        swrst: reset-controller@268 {
            compatible = "marvell,armada8k-reset";
            reg = <0x268 0x4>;
            #reset-cells = <1>;
        };
    };

> 
> Best regards,
> Krzysztof

Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Krzysztof Kozlowski]

On 07/03/2025 01:03, Wilson Ding wrote:
>> reset.example.dts
>>>   DTC [C]
>>> Documentation/devicetree/bindings/reset/marvell,armada8k-reset.example
>>> .dtb
>>>
>>
>> That's a bit odd but anyway warning is correct: you cannot have such
>> compatibles alone.
> 
> I understand I need to add one compatible here to resolve the
> warning. However, as we agreed, we keep the sud-nodes while
> there will be no new compatibles in parent node. So how shall
> I avoid this warning?
> 
> If we do want to add a compatible in parent node, what should
> be used for the compatible name? I think the most suitable name
> would be something like "marvell,cp110-system-controller0".

I don't know. I don't work in Marvell, I know nothing about Marvell
Armada and I was not involved in any Armada SoCs.

Read your datasheet and come up with some reasonable name based on
datasheet. Why do you ask people who do not have datasheet?

> However, it was already taken for in Armada clock controller to
> be compatible for legacy dt.
> 
> https://lore.kernel.org/all/bb21ee9acc55efac884450ff710049b99b27f8bf.1496328934.git-series.gregory.clement@free-electrons.com/

I don't understand what it means.

> 
> This is something about 8 years ago. I wonder if we do need to
> have the backward compatibility for the field devices at that
> time. Can we drop it now?

Drop what? You need to keep ABI.

> 
>     syscon0: system-controller@440000 {
>         compatible = "marvell,cp110-system-controller0",
>                                 "syscon", "simple-mfd";
>         reg = <0x440000 0x2000>;
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         swrst: reset-controller@268 {
>             compatible = "marvell,armada8k-reset";
>             reg = <0x268 0x4>;
>             #reset-cells = <1>;
>         };
>     };
> 
Best regards,
Krzysztof

Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Krzysztof Kozlowski]

On 06/03/2025 18:42, Wilson Ding wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Wednesday, March 5, 2025 11:29 PM
>> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> robh@kernel.org
>> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x: Add reset
>> controller node
>>
>> On 04/03/2025 20:08, Wilson Ding wrote:
>>>
>>> I did consider shrinking the syscon's register address range to make
>>> the reset-controller node to be independent from the syscon node.
>>> However, I found the syscon node is also referred by some devices for
>>> miscellaneous configurations . The reset configuration register
>>> happens to be located in between these registers and clock/GPIO
>>> registers.
>>>
>>>> drop offset in your patch or unify everything into 'reg'.
>>>>
>>>
>>> This is exactly what I proposed in v3 patch. Do I misunderstand you?
>>>
>>> CP11X_LABEL(swrst): reset-controller@268 {
>>> 	compatible = "marvell,armada8k-reset";
>>> 	reg = <0x268 0x4>;
>>> 	#reset-cells = <1>;
>>> };
>>
>> I don't see the other device being fixed here. How did you unify them?
> 
> This patch series is about the proposal of Armada8K's reset controller
> dt-binding. The dt-bindings issues of clock/GPIO controllers have been
> there for years. Having to say, it is not just a simple patch to fix it. It


I understand, you just want to throw your patch here over the wall. It's
reasonable, I feel it. Just like previous cases for this binding -
everyone wanted one subnode at a time, ignoring bigger picture, each
time making it franken-node or franken-binding.

Please listen to Greg's talk from years ago about upstreaming. "I don't
want your code":
https://www.youtube.com/watch?v=fMeH7wqOwXA


Best regards,
Krzysztof

Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add reset controller node

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Friday, March 7, 2025 1:00 AM
> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> robh@kernel.org
> Cc: 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: Re: [EXTERNAL] Re: [PATCH v3 3/3] arm64: dts: marvell: cp11x: Add
> reset controller node
> 
> On 06/03/2025 18:42, Wilson Ding wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Wednesday, March 5, 2025 11:29 PM
> >> To: Wilson Ding <dingwei@marvell.com>; linux-kernel@vger.kernel.org;
> >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> robh@kernel.org
> >> Cc: 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 v3 3/3] arm64: dts: marvell: cp11x:
> >> Add reset controller node
> >>
> >> On 04/03/2025 20:08, Wilson Ding wrote:
> >>>
> >>> I did consider shrinking the syscon's register address range to make
> >>> the reset-controller node to be independent from the syscon node.
> >>> However, I found the syscon node is also referred by some devices
> >>> for miscellaneous configurations . The reset configuration register
> >>> happens to be located in between these registers and clock/GPIO
> >>> registers.
> >>>
> >>>> drop offset in your patch or unify everything into 'reg'.
> >>>>
> >>>
> >>> This is exactly what I proposed in v3 patch. Do I misunderstand you?
> >>>
> >>> CP11X_LABEL(swrst): reset-controller@268 {
> >>> 	compatible = "marvell,armada8k-reset";
> >>> 	reg = <0x268 0x4>;
> >>> 	#reset-cells = <1>;
> >>> };
> >>
> >> I don't see the other device being fixed here. How did you unify them?
> >
> > This patch series is about the proposal of Armada8K's reset controller
> > dt-binding. The dt-bindings issues of clock/GPIO controllers have been
> > there for years. Having to say, it is not just a simple patch to fix
> > it. It
> 
> 
> I understand, you just want to throw your patch here over the wall. It's
> reasonable, I feel it. Just like previous cases for this binding - everyone wanted
> one subnode at a time, ignoring bigger picture, each time making it franken-
> node or franken-binding.
> 
> Please listen to Greg's talk from years ago about upstreaming. "I don't want
> your code":
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__www.youtube.com_watch-3Fv-
> 3DfMeH7wqOwXA&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=sXDQZu4G
> yqNVDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=JlqowP3KS_dAuPARdfHNUE
> mdfiBr1kVd2ia1atnDNxBMMB5ccqNUuU1qtNhJMhzE&s=WpwrGxdqS7L0Bz-
> fKr_AdTuLBD2v-0OEKSRBOG05wKs&e=
> 

Sorry, I will fix them in next version.

> 
> Best regards,
> Krzysztof

Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K reset controller

[Wilson Ding]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Friday, March 7, 2025 12:57 AM
> To: Wilson Ding <dingwei@marvell.com>; Rob Herring (Arm)
> <robh@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org; Sanghoon Lee <salee@marvell.com>;
> conor+dt@kernel.org; linux-kernel@vger.kernel.org;
> gregory.clement@bootlin.com; p.zabel@pengutronix.de; Geethasowjanya
> Akula <gakula@marvell.com>; devicetree@vger.kernel.org; andrew@lunn.ch;
> krzk+dt@kernel.org; sebastian.hesselbarth@gmail.com
> Subject: [EXTERNAL] Re: [PATCH v3 1/3] dt-bindings: reset: Add Armada8K
> reset controller
> 
> On 07/03/2025 01:03, Wilson Ding wrote:
> >> reset.example.dts
> >>>   DTC [C]
> >>> Documentation/devicetree/bindings/reset/marvell,armada8k-
> reset.examp
> >>> le
> >>> .dtb
> >>>
> >>
> >> That's a bit odd but anyway warning is correct: you cannot have such
> >> compatibles alone.
> >
> > I understand I need to add one compatible here to resolve the warning.
> > However, as we agreed, we keep the sud-nodes while there will be no
> > new compatibles in parent node. So how shall I avoid this warning?
> >
> > If we do want to add a compatible in parent node, what should be used
> > for the compatible name? I think the most suitable name would be
> > something like "marvell,cp110-system-controller0".
> 
> I don't know. I don't work in Marvell, I know nothing about Marvell Armada
> and I was not involved in any Armada SoCs.
> 
> Read your datasheet and come up with some reasonable name based on
> datasheet. Why do you ask people who do not have datasheet?
> 
> > However, it was already taken for in Armada clock controller to be
> > compatible for legacy dt.
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_a
> > ll_bb21ee9acc55efac884450ff710049b99b27f8bf.1496328934.git-
> 2Dseries.gr
> > egory.clement-40free-
> 2Delectrons.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtf
> >
> Q&r=sXDQZu4GyqNVDlFUXakSGJl0Dh81ZIPlU26YS4KHGIA&m=nfrB1z6tgLEY
> 3BeUK8cH
> >
> 5CBEnUJhq1yHXS9xCDegRMGapzD2rOL7z092sJd1dzIo&s=LqUI_K0rnOXPmy
> dkE9n-hdK
> > KO2dmGhyN5H-raW6-6Dc&e=
> 
> I don't understand what it means.
> 
> >
> > This is something about 8 years ago. I wonder if we do need to have
> > the backward compatibility for the field devices at that time. Can we
> > drop it now?
> 
> Drop what? You need to keep ABI.

Do you mean once a compatible name was present in dt-binding, it
Should never be deleted in future (even these is no dts file using it)?

> 
> >
> >     syscon0: system-controller@440000 {
> >         compatible = "marvell,cp110-system-controller0",
> >                                 "syscon", "simple-mfd";
> >         reg = <0x440000 0x2000>;
> >         #address-cells = <1>;
> >         #size-cells = <1>;
> >
> >         swrst: reset-controller@268 {
> >             compatible = "marvell,armada8k-reset";
> >             reg = <0x268 0x4>;
> >             #reset-cells = <1>;
> >         };
> >     };
> >
> Best regards,
> Krzysztof