[PATCH v1 0/2] reset: mediatek: add syscon-based reset controller

[PATCH v1 0/2] reset: mediatek: add syscon-based reset controller

[peter.wang]

From: Peter Wang <peter.wang@mediatek.com>

This series adds support for a MediaTek SYSCON-based reset controller
that manages reset lines through memory-mapped registers within a syscon
MFD block.

Peter Wang (2):
  Documentation: dt: reset: add mediatek,syscon-reset binding
  reset: mediatek: add syscon-based reset controller driver

 .../bindings/reset/mediatek,syscon-reset.yaml |  93 +++++++
 drivers/reset/Kconfig                         |  10 +
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-mediatek-syscon.c         | 230 ++++++++++++++++++
 4 files changed, 334 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
 create mode 100644 drivers/reset/reset-mediatek-syscon.c

-- 
2.45.2

[PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[peter.wang]

From: Peter Wang <peter.wang@mediatek.com>

Add Device Tree binding documentation for the MediaTek SYSCON reset
controller (mediatek,syscon-reset). This binding describes a reset
controller that is part of a MediaTek SYSCON MFD block and manages
multiple reset lines within that block.

The reset lines are defined via the 'mediatek,reset-bits' property,
where each entry is a five-cell tuple specifying the register offsets
and bit positions for asserting and deasserting a reset line:

  <set_offset set_bit clear_offset clear_bit delay_us>

Consumers reference individual reset lines by index into this list
using the standard '#reset-cells = <1>' scheme.

An example is provided showing a UFS subsystem use case, where the
reset controller is instantiated as a child node of the UFS syscon
MFD node.

Signed-off-by: Peter Wang <peter.wang@mediatek.com>
---
 .../bindings/reset/mediatek,syscon-reset.yaml | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml

diff --git a/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml b/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
new file mode 100644
index 000000000000..45520ae6f090
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek SYSCON Reset Controller
+
+maintainers:
+  - Peter Wang <peter.wang@mediatek.com>
+
+description: |
+  This describes a reset controller which is part of a MediaTek SYSCON block
+  and is designed to manage multiple reset lines within that block.
+
+  The node should be a child of a syscon MFD node; it uses the parent's
+  regmap and therefore does not require its own 'reg' property.
+
+  The reset specifier for consumers is an index into the 'mediatek,reset-bits'
+  list. For example, to reference the second reset line:
+
+    resets = <&ufs0cfgao_rst 1>;
+    reset-names = "hci_rst";
+
+properties:
+  compatible:
+    const: mediatek,syscon-reset
+
+  '#reset-cells':
+    const: 1
+    description:
+      The cell should contain the index into the 'mediatek,reset-bits'
+      property to select the specific reset line.
+
+  mediatek,reset-bits:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    minItems: 1
+    maxItems: 64
+    description: |
+      A list of reset bit definitions. Each reset line is defined by a
+      five-cell entry: <set_offset set_bit clear_offset clear_bit delay_us>.
+      The reset specifier in the consumer driver will be an index into
+      this list. Up to 64 reset lines are supported per controller instance.
+    items:
+      items:
+        - description: >
+            Register offset from the parent syscon base for asserting
+            the reset. Must be 4-byte aligned.
+          minimum: 0
+          multipleOf: 4
+        - description: >
+            Bit index (0-based) within the register for asserting reset.
+          minimum: 0
+          maximum: 31
+        - description: >
+            Register offset from the parent syscon base for deasserting
+            the reset. Must be 4-byte aligned.
+          minimum: 0
+          multipleOf: 4
+        - description: >
+            Bit index (0-based) within the register for deasserting reset.
+          minimum: 0
+          maximum: 31
+        - description: >
+            Minimum delay in microseconds between assertion and deassertion
+            during a full reset cycle. Use 0 if no delay is required.
+          minimum: 0
+          maximum: 1000000
+
+required:
+  - compatible
+  - '#reset-cells'
+  - mediatek,reset-bits
+
+additionalProperties: false
+
+examples:
+  - |
+    ufs0cfg_ao: syscon@16840000 {
+        compatible = "mediatek,mt8183-ufs0cfg_ao", "syscon", "simple-mfd";
+        reg = <0x16840000 0x1000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        ufs0cfgao_rst: reset-controller {
+            compatible = "mediatek,syscon-reset";
+            #reset-cells = <1>;
+            mediatek,reset-bits =
+                <0x48  3  0x4c  3  100>,
+                <0x148 0  0x14c 0  100>,
+                <0x148 1  0x14c 1  100>,
+                <0x148 2  0x14c 2  0>;
+        };
+    };
-- 
2.45.2

[PATCH v1 2/2] reset: mediatek: add syscon-based reset controller driver

[peter.wang]

From: Peter Wang <peter.wang@mediatek.com>

Add a new reset controller driver for MediaTek SoCs that expose reset
lines through memory-mapped registers managed by a syscon node.

The driver reads reset line configurations from the device tree property
"mediatek,reset-bits", which encodes each reset as a 5-tuple of:

  <assert_offset, assert_bit, deassert_offset, deassert_bit, delay_us>

This allows support for hardware designs that use separate set/clear
registers for assertion and deassertion. The regmap handle is obtained
from the parent syscon node, and the driver registers itself with the
kernel reset controller framework, providing assert, deassert, and reset
operations.

The per-line pulse width between assert and deassert is configured
via the 'delay_us' field in the 'mediatek,reset-bits' device tree
property. If set to zero, no delay is inserted.

Signed-off-by: Peter Wang <peter.wang@mediatek.com>
---
 drivers/reset/Kconfig                 |  10 ++
 drivers/reset/Makefile                |   1 +
 drivers/reset/reset-mediatek-syscon.c | 230 ++++++++++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/reset/reset-mediatek-syscon.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index d009eb0849a3..64586fa8bcd9 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -187,6 +187,16 @@ config RESET_MCHP_SPARX5
 	help
 	  This driver supports switch core reset for the Microchip Sparx5 SoC.
 
+config RESET_MEDIATEK_SYSCON
+	tristate "MediaTek Syscon Reset Driver"
+	depends on (ARCH_MEDIATEK || COMPILE_TEST) && HAS_IOMEM && OF
+	select MFD_SYSCON
+	help
+	  This enables reset driver support for MediaTek devices
+	  that use memory-mapped reset registers as part of a syscon
+	  device node. Say Y if you want to control reset signals
+	  provided by this controller. Otherwise, say N.
+
 config RESET_NPCM
 	bool "NPCM BMC Reset Driver" if COMPILE_TEST
 	default ARCH_NPCM
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 3e52569bd276..c0a7fdbe9768 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_RESET_K230) += reset-k230.o
 obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
 obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_RESET_MCHP_SPARX5) += reset-microchip-sparx5.o
+obj-$(CONFIG_RESET_MEDIATEK_SYSCON) += reset-mediatek-syscon.o
 obj-$(CONFIG_RESET_NPCM) += reset-npcm.o
 obj-$(CONFIG_RESET_NUVOTON_MA35D1) += reset-ma35d1.o
 obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
diff --git a/drivers/reset/reset-mediatek-syscon.c b/drivers/reset/reset-mediatek-syscon.c
new file mode 100644
index 000000000000..da8aa646cee7
--- /dev/null
+++ b/drivers/reset/reset-mediatek-syscon.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2026 MediaTek Inc.
+ * Authors:
+ *	Peter Wang <peter.wang@mediatek.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+/* Must match the maximum value in mediatek,syscon-reset.yaml */
+#define MEDIATEK_SYSCON_RESET_MAX_DELAY_US	1000000 /* 1 second */
+
+/**
+ * struct mediatek_syscon_reset_control - The control for a single reset line.
+ *
+ * @assert_offset: Register offset from syscon base to assert the reset.
+ * @assert_bit: Bit index within the assert register to set for assertion.
+ * @deassert_offset: Register offset from syscon base to deassert the reset.
+ * @deassert_bit: Bit index within the deassert register to set for deassertion.
+ * @delay_us: Delay in microseconds between assertion and deassertion.
+ *            Zero means no delay is inserted.
+ */
+struct mediatek_syscon_reset_control {
+	unsigned int assert_offset;
+	unsigned int assert_bit;
+	unsigned int deassert_offset;
+	unsigned int deassert_bit;
+	unsigned int delay_us;
+};
+
+/**
+ * struct mediatek_syscon_reset_data - Private data for a MediaTek syscon
+ * reset controller.
+ *
+ * @rcdev: The embedded reset controller device, registered with the core.
+ * @regmap: The regmap handle for accessing hardware reset registers.
+ * @mrstc: Pointer to an array of reset line configurations
+ *         (struct mediatek_syscon_reset_control).
+ */
+struct mediatek_syscon_reset_data {
+	struct reset_controller_dev rcdev;
+	struct regmap *regmap;
+	struct mediatek_syscon_reset_control *mrstc;
+};
+
+/**
+ * mediatek_syscon_reset_assert() - Assert a specific reset line.
+ * @rcdev: The reset controller device.
+ * @id: The index of the reset line to assert.
+ *
+ * This function implements the .assert operation for the reset controller. It
+ * sets the specific assertion bit for the given reset ID by writing to the
+ * hardware register defined in the driver's configuration data.
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+static int mediatek_syscon_reset_assert(struct reset_controller_dev *rcdev,
+					unsigned long id)
+{
+	struct mediatek_syscon_reset_data *data =
+		container_of(rcdev, struct mediatek_syscon_reset_data, rcdev);
+	struct mediatek_syscon_reset_control *rstc = &data->mrstc[id];
+	unsigned int val = BIT(rstc->assert_bit);
+
+	return regmap_write_bits(data->regmap, rstc->assert_offset, val, val);
+}
+
+/**
+ * mediatek_syscon_reset_deassert() - Deassert a specific reset line.
+ * @rcdev: The reset controller device.
+ * @id: The index of the reset line to deassert.
+ *
+ * Writes a '1' to the bit in the dedicated hardware deassert (clear) register
+ * for the given reset line. This assumes the hardware uses separate
+ * set-to-assert and set-to-deassert registers; @deassert_offset must point
+ * to the clear register, not the same register as @assert_offset.
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+static int mediatek_syscon_reset_deassert(struct reset_controller_dev *rcdev,
+					  unsigned long id)
+{
+	struct mediatek_syscon_reset_data *data =
+		container_of(rcdev, struct mediatek_syscon_reset_data, rcdev);
+	struct mediatek_syscon_reset_control *rstc = &data->mrstc[id];
+	unsigned int val = BIT(rstc->deassert_bit);
+
+	return regmap_write_bits(data->regmap, rstc->deassert_offset, val, val);
+}
+
+/**
+ * mediatek_syscon_reset_reset() - Assert then deassert a reset line.
+ * @rcdev: The reset controller device.
+ * @id: The index of the reset line to reset.
+ *
+ * Performs a full reset cycle by asserting the reset line, waiting for the
+ * per-line hardware-required pulse width defined in the 'mediatek,reset-bits'
+ * device tree property, then deasserting it. If the pulse width is zero,
+ * no delay is inserted.
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+static int mediatek_syscon_reset_reset(struct reset_controller_dev *rcdev,
+				       unsigned long id)
+{
+	struct mediatek_syscon_reset_data *data =
+		container_of(rcdev, struct mediatek_syscon_reset_data, rcdev);
+	struct mediatek_syscon_reset_control *rstc = &data->mrstc[id];
+	int ret;
+
+	ret = mediatek_syscon_reset_assert(rcdev, id);
+	if (ret)
+		return ret;
+
+	if (rstc->delay_us)
+		fsleep(rstc->delay_us);
+
+	return mediatek_syscon_reset_deassert(rcdev, id);
+}
+
+static const struct reset_control_ops mediatek_syscon_reset_ops = {
+	.assert = mediatek_syscon_reset_assert,
+	.deassert = mediatek_syscon_reset_deassert,
+	.reset = mediatek_syscon_reset_reset,
+};
+
+static int mediatek_syscon_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mediatek_syscon_reset_control *mrstc;
+	struct mediatek_syscon_reset_data *data;
+	struct device_node *np = dev->of_node;
+	int nr_resets, i, count, idx, ret;
+	struct regmap *regmap;
+	u32 *arr;
+
+	if (!dev->parent) {
+		dev_err(dev, "missing parent device node\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	regmap = syscon_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	count = of_property_count_u32_elems(np, "mediatek,reset-bits");
+	if (count <= 0) {
+		dev_err(dev, "failed to read mediatek,reset-bits property: %d\n", count);
+		return -EINVAL;
+	}
+	if (count % 5 != 0) {
+		dev_err(dev, "mediatek,reset-bits count (%d) must be a multiple of 5\n", count);
+		return -EINVAL;
+	}
+
+	nr_resets = count / 5;
+
+	arr = devm_kcalloc(dev, count, sizeof(u32), GFP_KERNEL);
+	if (!arr)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(np, "mediatek,reset-bits", arr, count);
+	if (ret) {
+		dev_err(dev, "failed to read mediatek,reset-bits: %d\n", ret);
+		return ret;
+	}
+
+	mrstc = devm_kcalloc(dev, nr_resets, sizeof(*mrstc), GFP_KERNEL);
+	if (!mrstc)
+		return -ENOMEM;
+
+	for (i = 0; i < nr_resets; i++) {
+		idx = i * 5;
+		mrstc[i].assert_offset = arr[idx];
+		mrstc[i].assert_bit = arr[idx + 1];
+		mrstc[i].deassert_offset = arr[idx + 2];
+		mrstc[i].deassert_bit = arr[idx + 3];
+		mrstc[i].delay_us = arr[idx + 4];
+
+		if (mrstc[i].assert_bit >= 32 || mrstc[i].deassert_bit >= 32) {
+			dev_err(dev, "reset[%d]: bit index out of range\n", i);
+			return -EINVAL;
+		}
+
+		if (mrstc[i].delay_us > MEDIATEK_SYSCON_RESET_MAX_DELAY_US) {
+			dev_err(dev, "reset[%d]: delay %u us exceeds maximum %u us\n",
+				i, mrstc[i].delay_us,
+				MEDIATEK_SYSCON_RESET_MAX_DELAY_US);
+			return -EINVAL;
+		}
+	}
+
+	data->regmap = regmap;
+	data->mrstc = mrstc;
+	data->rcdev.ops = &mediatek_syscon_reset_ops;
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.of_node = np;
+	data->rcdev.nr_resets = nr_resets;
+
+	return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static const struct of_device_id mediatek_syscon_reset_of_match[] = {
+	{ .compatible = "mediatek,syscon-reset" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mediatek_syscon_reset_of_match);
+
+static struct platform_driver mediatek_syscon_reset_driver = {
+	.probe = mediatek_syscon_reset_probe,
+	.driver = {
+		.name = "mediatek-syscon-reset",
+		.of_match_table = mediatek_syscon_reset_of_match,
+	},
+};
+module_platform_driver(mediatek_syscon_reset_driver);
+
+MODULE_AUTHOR("Peter Wang <peter.wang@mediatek.com>");
+MODULE_DESCRIPTION("MediaTek SYSCON Regmap Reset Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.45.2

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Philipp Zabel]

On Fr, 2026-06-26 at 15:46 +0800, peter.wang@mediatek.com wrote:
> From: Peter Wang <peter.wang@mediatek.com>
> 
> Add Device Tree binding documentation for the MediaTek SYSCON reset
> controller (mediatek,syscon-reset). This binding describes a reset
> controller that is part of a MediaTek SYSCON MFD block and manages
> multiple reset lines within that block.
> 
> The reset lines are defined via the 'mediatek,reset-bits' property,
> where each entry is a five-cell tuple specifying the register offsets
> and bit positions for asserting and deasserting a reset line:
> 
>   <set_offset set_bit clear_offset clear_bit delay_us>
> 
> Consumers reference individual reset lines by index into this list
> using the standard '#reset-cells = <1>' scheme.
> 
> An example is provided showing a UFS subsystem use case, where the
> reset controller is instantiated as a child node of the UFS syscon
> MFD node.
> 
> Signed-off-by: Peter Wang <peter.wang@mediatek.com>
> ---
>  .../bindings/reset/mediatek,syscon-reset.yaml | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml b/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
> new file mode 100644
> index 000000000000..45520ae6f090
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/mediatek,syscon-reset.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek SYSCON Reset Controller
> +
> +maintainers:
> +  - Peter Wang <peter.wang@mediatek.com>
> +
> +description: |
> +  This describes a reset controller which is part of a MediaTek SYSCON block
> +  and is designed to manage multiple reset lines within that block.
> +
> +  The node should be a child of a syscon MFD node; it uses the parent's
> +  regmap and therefore does not require its own 'reg' property.
> +
> +  The reset specifier for consumers is an index into the 'mediatek,reset-bits'
> +  list. For example, to reference the second reset line:
> +
> +    resets = <&ufs0cfgao_rst 1>;
> +    reset-names = "hci_rst";
> +
> +properties:
> +  compatible:
> +    const: mediatek,syscon-reset
> +
> +  '#reset-cells':
> +    const: 1
> +    description:
> +      The cell should contain the index into the 'mediatek,reset-bits'
> +      property to select the specific reset line.
> +
> +  mediatek,reset-bits:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    minItems: 1
> +    maxItems: 64
> +    description: |
> +      A list of reset bit definitions. Each reset line is defined by a
> +      five-cell entry: <set_offset set_bit clear_offset clear_bit delay_us>.
> +      The reset specifier in the consumer driver will be an index into
> +      this list. Up to 64 reset lines are supported per controller instance.
> +    items:
> +      items:
> +        - description: >
> +            Register offset from the parent syscon base for asserting
> +            the reset. Must be 4-byte aligned.
> +          minimum: 0
> +          multipleOf: 4
> +        - description: >
> +            Bit index (0-based) within the register for asserting reset.
> +          minimum: 0
> +          maximum: 31
> +        - description: >
> +            Register offset from the parent syscon base for deasserting
> +            the reset. Must be 4-byte aligned.
> +          minimum: 0
> +          multipleOf: 4
> +        - description: >
> +            Bit index (0-based) within the register for deasserting reset.
> +          minimum: 0
> +          maximum: 31
> +        - description: >
> +            Minimum delay in microseconds between assertion and deassertion
> +            during a full reset cycle. Use 0 if no delay is required.
> +          minimum: 0
> +          maximum: 1000000
> +
> +required:
> +  - compatible
> +  - '#reset-cells'
> +  - mediatek,reset-bits
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ufs0cfg_ao: syscon@16840000 {
> +        compatible = "mediatek,mt8183-ufs0cfg_ao", "syscon", "simple-mfd";

Where is the binding doc for mediatek,mt8183-ufs0cfg_ao? Is this
simple-mfd just to load the reset driver?

> +        reg = <0x16840000 0x1000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        ufs0cfgao_rst: reset-controller {
> +            compatible = "mediatek,syscon-reset";

It looks to me like this is just two registers inside ufs0cfg_ao, not a
separate device. Why don't you just add #reset-cells to the parent
node?

> +            #reset-cells = <1>;
> +            mediatek,reset-bits =
> +                <0x48  3  0x4c  3  100>,
> +                <0x148 0  0x14c 0  100>,
> +                <0x148 1  0x14c 1  100>,
> +                <0x148 2  0x14c 2  0>;
> +        };

Why is this in DT? This should be a table in the reset driver.

regards
Philipp

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Conor Dooley]

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

On Fri, Jun 26, 2026 at 10:33:20AM +0200, Philipp Zabel wrote:

> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    ufs0cfg_ao: syscon@16840000 {
> > +        compatible = "mediatek,mt8183-ufs0cfg_ao", "syscon", "simple-mfd";
> 
> Where is the binding doc for mediatek,mt8183-ufs0cfg_ao? Is this
> simple-mfd just to load the reset driver?

It isn't going to exist, since we will chuck a fit at the underscore!

> 
> > +        reg = <0x16840000 0x1000>;
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        ufs0cfgao_rst: reset-controller {
> > +            compatible = "mediatek,syscon-reset";
> 
> It looks to me like this is just two registers inside ufs0cfg_ao, not a
> separate device. Why don't you just add #reset-cells to the parent
> node?

I concur, seems like it should be in the parent.
pw-bot: changes-requested

> 
> > +            #reset-cells = <1>;
> > +            mediatek,reset-bits =
> > +                <0x48  3  0x4c  3  100>,
> > +                <0x148 0  0x14c 0  100>,
> > +                <0x148 1  0x14c 1  100>,
> > +                <0x148 2  0x14c 2  0>;
> > +        };
> 
> Why is this in DT? This should be a table in the reset driver.
> 
> regards
> Philipp

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

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Peter Wang (王信友)]

On Fri, 2026-06-26 at 10:33 +0200, Philipp Zabel wrote
> 
> Where is the binding doc for mediatek,mt8183-ufs0cfg_ao? Is this
> simple-mfd just to load the reset driver?
> 

Hi Philipp,

Thanks for the review, and sorry for the late reply.
Yes, "mediatek,mt8183-ufs0cfg_ao" should be removed.
I will remove it in the next version.

> > +        reg = <0x16840000 0x1000>;
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        ufs0cfgao_rst: reset-controller {
> > +            compatible = "mediatek,syscon-reset";
> 
> It looks to me like this is just two registers inside ufs0cfg_ao, not
> a
> separate device. Why don't you just add #reset-cells to the parent
> node?
> 
> > +            #reset-cells = <1>;
> > +            mediatek,reset-bits =
> > +                <0x48  3  0x4c  3  100>,
> > +                <0x148 0  0x14c 0  100>,
> > +                <0x148 1  0x14c 1  100>,
> > +                <0x148 2  0x14c 2  0>;
> > +        };
> 
> Why is this in DT? This should be a table in the reset driver.
> 
> regards
> Philipp

Regarding the child node and reset-bits in DT,
We chose the child node approach with 'mediatek,reset-bits' defined 
in DT to keep the reset line descriptions self-contained and reusable 
across different SoC variants. MediaTek has many SoCs (mt8183, mt6985, 
mt6989, ...) where the same UFS subsystem may have different register 
offsets for reset lines. By describing them in DT, we can support new 
SoC variants by updating the DT alone, without requiring a new driver 
patch for every new SoC.

This approach is also consistent with the existing 'ti,syscon-reset' 
binding, which uses a similar per-entry table property 'ti,reset-bits' 
to describe reset lines within a syscon block.

Thanks
Peter

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Peter Wang (王信友)]

On Fri, 2026-06-26 at 16:53 +0100, Conor Dooley wrote:
> 
> It isn't going to exist, since we will chuck a fit at the underscore!
> 

Hi Conor,

Thanks for the review.
I apologize for the mistake in using this bad examples.
I will remove it in the next version.

> > 
> > > +        reg = <0x16840000 0x1000>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <1>;
> > > +
> > > +        ufs0cfgao_rst: reset-controller {
> > > +            compatible = "mediatek,syscon-reset";
> > 
> > It looks to me like this is just two registers inside ufs0cfg_ao,
> > not a
> > separate device. Why don't you just add #reset-cells to the parent
> > node?
> 
> I concur, seems like it should be in the parent.
> pw-bot: changes-requested
> 
> 

This is for flexibility in MediaTek SoCs, as described in the
previous email.

Thanks
Peter

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Conor Dooley]

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

On Wed, Jul 01, 2026 at 06:35:20AM +0000, Peter Wang (王信友) wrote:
> On Fri, 2026-06-26 at 10:33 +0200, Philipp Zabel wrote
> > 
> > Where is the binding doc for mediatek,mt8183-ufs0cfg_ao? Is this
> > simple-mfd just to load the reset driver?
> > 
> 
> Hi Philipp,
> 
> Thanks for the review, and sorry for the late reply.
> Yes, "mediatek,mt8183-ufs0cfg_ao" should be removed.
> I will remove it in the next version.
> 
> > > +        reg = <0x16840000 0x1000>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <1>;
> > > +
> > > +        ufs0cfgao_rst: reset-controller {
> > > +            compatible = "mediatek,syscon-reset";
> > 
> > It looks to me like this is just two registers inside ufs0cfg_ao, not
> > a
> > separate device. Why don't you just add #reset-cells to the parent
> > node?
> > 
> > > +            #reset-cells = <1>;
> > > +            mediatek,reset-bits =
> > > +                <0x48  3  0x4c  3  100>,
> > > +                <0x148 0  0x14c 0  100>,
> > > +                <0x148 1  0x14c 1  100>,
> > > +                <0x148 2  0x14c 2  0>;
> > > +        };
> > 
> > Why is this in DT? This should be a table in the reset driver.
> > 
> > regards
> > Philipp
> 
> Regarding the child node and reset-bits in DT,
> We chose the child node approach with 'mediatek,reset-bits' defined 
> in DT to keep the reset line descriptions self-contained and reusable 
> across different SoC variants. MediaTek has many SoCs (mt8183, mt6985, 
> mt6989, ...) where the same UFS subsystem may have different register 
> offsets for reset lines. By describing them in DT, we can support new 
> SoC variants by updating the DT alone, without requiring a new driver 
> patch for every new SoC.

From what I recall, mediatek ufs is a mess with lots of vendor kernel
type things slipping into mainline without proper review on the DT
front.
Because of that, I at least am going to require that everything is done
completely (and perhaps excessively) by the book here, including
introducing complete bindings for syscon regions rather than partial
bits for components like this one.

> This approach is also consistent with the existing 'ti,syscon-reset' 
> binding, which uses a similar per-entry table property 'ti,reset-bits' 
> to describe reset lines within a syscon block.

This was done about 10 years ago, I would not consider it a guide for
what's acceptable today.

Thanks,
Conor.

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

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Peter Wang (王信友)]

On Wed, 2026-07-01 at 17:57 +0100, Conor Dooley wrote:
> From what I recall, mediatek ufs is a mess with lots of vendor kernel
> type things slipping into mainline without proper review on the DT
> front.
> Because of that, I at least am going to require that everything is
> done
> completely (and perhaps excessively) by the book here, including
> introducing complete bindings for syscon regions rather than partial
> bits for components like this one.
> 

Hi Conor,

I am sorry for giving you that impression. Yes, it was our 
oversight due to a lack of experience, and we missed some 
binding-related documentation. We are working on completing 
them step by step, as we are doing now. We hope you can 
kindly allow us to address the missing parts gradually, 
instead of requiring everything to be completed all at once.


> > This approach is also consistent with the existing 'ti,syscon-
> > reset' 
> > binding, which uses a similar per-entry table property 'ti,reset-
> > bits' 
> > to describe reset lines within a syscon block.
> 
> This was done about 10 years ago, I would not consider it a guide for
> what's acceptable today.
> 
> Thanks,
> Conor.

This should simply be a difference in hardware design and usage,
with software deciding whether to opt for flexibility or a 
fixed approach. And it's not that the previous method was 'correct'
and the current one is 'incorrect,' is it?

Thanks
Peter

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[AngeloGioacchino Del Regno]

On 7/1/26 18:57, Conor Dooley wrote:
> On Wed, Jul 01, 2026 at 06:35:20AM +0000, Peter Wang (王信友) wrote:
>> On Fri, 2026-06-26 at 10:33 +0200, Philipp Zabel wrote
>>>
>>> Where is the binding doc for mediatek,mt8183-ufs0cfg_ao? Is this
>>> simple-mfd just to load the reset driver?
>>>
>>
>> Hi Philipp,
>>
>> Thanks for the review, and sorry for the late reply.
>> Yes, "mediatek,mt8183-ufs0cfg_ao" should be removed.
>> I will remove it in the next version.
>>
>>>> +        reg = <0x16840000 0x1000>;
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <1>;
>>>> +
>>>> +        ufs0cfgao_rst: reset-controller {
>>>> +            compatible = "mediatek,syscon-reset";
>>>
>>> It looks to me like this is just two registers inside ufs0cfg_ao, not
>>> a
>>> separate device. Why don't you just add #reset-cells to the parent
>>> node?
>>>
>>>> +            #reset-cells = <1>;
>>>> +            mediatek,reset-bits =
>>>> +                <0x48  3  0x4c  3  100>,
>>>> +                <0x148 0  0x14c 0  100>,
>>>> +                <0x148 1  0x14c 1  100>,
>>>> +                <0x148 2  0x14c 2  0>;
>>>> +        };
>>>
>>> Why is this in DT? This should be a table in the reset driver.
>>>
>>> regards
>>> Philipp
>>
>> Regarding the child node and reset-bits in DT,
>> We chose the child node approach with 'mediatek,reset-bits' defined
>> in DT to keep the reset line descriptions self-contained and reusable
>> across different SoC variants. MediaTek has many SoCs (mt8183, mt6985,
>> mt6989, ...) where the same UFS subsystem may have different register
>> offsets for reset lines. By describing them in DT, we can support new
>> SoC variants by updating the DT alone, without requiring a new driver
>> patch for every new SoC.
> 
>  From what I recall, mediatek ufs is a mess with lots of vendor kernel
> type things slipping into mainline without proper review on the DT
> front.
> Because of that, I at least am going to require that everything is done
> completely (and perhaps excessively) by the book here, including
> introducing complete bindings for syscon regions rather than partial
> bits for components like this one.
> 

Conor, thanks for chiming in.

I would've done that earlier but I'm going through a bit of busy period here, so
thanks x2.

Yes the UFS driver is quite a bit messy.
The latest version of our series that cleans it up is version 9, sent quite a while
ago at this point:

https://patchwork.kernel.org/project/linux-scsi/cover/20260306-mt8196-ufs-v9-0-55b073f7a830@collabora.com/

...but anyway, during these months we kept going on with it and we do have a newer
version of this series in our tree, which we're planning to send in a week or two.

That'll start fixing stuff around, at least, in both bindings and code.

>> This approach is also consistent with the existing 'ti,syscon-reset'
>> binding, which uses a similar per-entry table property 'ti,reset-bits'
>> to describe reset lines within a syscon block.
> 
> This was done about 10 years ago, I would not consider it a guide for
> what's acceptable today.
> 

I completely agree about that. Honestly, I'm not even sure why MediaTek needs the
resets in the ufscfg0-ao space (because I didn't do extensive research, but that
might be rightful), but this is getting a bit annoying as they keep pushing for
adding something similar to the TI syscon reset at least at every new chip that
comes out (or every 6-8 months if I recall correctly), and we keep doing the same
review over and over.

Please MediaTek, stop trying to add syscon-reset. Please!

I know that you're trying to do that because in your downstream you never stopped
using ti,syscon-reset: if that works better for you in your downstream, that's ok
as it's purely yours but, as a matter of fact, in this form, it's not upstreamable.

If you have to upstream a *pure* reset controller, make a reset controller driver
and put it in the appropriate kernel subsystem - but I also want to remind you
that I know MediaTek SoCs, and I know that up until MT8196 there shouldn't be any
hardware that is purely a reset controller (as in, there's no IP that manages only
resets).

Thanks everyone!
Angelo

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Peter Wang (王信友)]

On Thu, 2026-07-02 at 10:55 +0200, AngeloGioacchino Del Regno wrote:
> I completely agree about that. Honestly, I'm not even sure why
> MediaTek needs the
> resets in the ufscfg0-ao space (because I didn't do extensive
> research, but that
> might be rightful), but this is getting a bit annoying as they keep
> pushing for
> adding something similar to the TI syscon reset at least at every new
> chip that
> comes out (or every 6-8 months if I recall correctly), and we keep
> doing the same
> review over and over.
> 
> Please MediaTek, stop trying to add syscon-reset. Please!
> 
> I know that you're trying to do that because in your downstream you
> never stopped
> using ti,syscon-reset: if that works better for you in your
> downstream, that's ok
> as it's purely yours but, as a matter of fact, in this form, it's not
> upstreamable.
> 
> If you have to upstream a *pure* reset controller, make a reset
> controller driver
> and put it in the appropriate kernel subsystem - but I also want to
> remind you
> that I know MediaTek SoCs, and I know that up until MT8196 there
> shouldn't be any
> hardware that is purely a reset controller (as in, there's no IP that
> manages only
> resets).
> 
> Thanks everyone!
> Angelo

Hi AngeloGioacchino,

Thank you for your review.
Indeed, you're right – we don't have any hardware that functions
solely as a reset controller. Our hardware only provides partial
reset capabilities, which necessitates a partial reset driver 
(responsible for managing just a few set/clear bits).

We are currently planning to remove the ti,syscon-reset implementation
and utilize a MediaTek-specific syscon-reset instead. However, 
if you object, I will halt this plan, and we can adopt
Philipp's proposed solution for handling the reset.

Thanks
Peter

Re: [PATCH v1 1/2] Documentation: dt: reset: add mediatek,syscon-reset binding

[Conor Dooley]

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

On Thu, Jul 02, 2026 at 08:52:53AM +0000, Peter Wang (王信友) wrote:
> On Wed, 2026-07-01 at 17:57 +0100, Conor Dooley wrote:
> > From what I recall, mediatek ufs is a mess with lots of vendor kernel
> > type things slipping into mainline without proper review on the DT
> > front.
> > Because of that, I at least am going to require that everything is
> > done
> > completely (and perhaps excessively) by the book here, including
> > introducing complete bindings for syscon regions rather than partial
> > bits for components like this one.
> > 
> 
> Hi Conor,
> 
> I am sorry for giving you that impression. Yes, it was our 
> oversight due to a lack of experience, and we missed some 
> binding-related documentation. We are working on completing 
> them step by step, as we are doing now. We hope you can 
> kindly allow us to address the missing parts gradually, 
> instead of requiring everything to be completed all at once.

No, sorry. I expect to see a complete binding for this system added in
one go.

> > > This approach is also consistent with the existing 'ti,syscon-
> > > reset' 
> > > binding, which uses a similar per-entry table property 'ti,reset-
> > > bits' 
> > > to describe reset lines within a syscon block.
> > 
> > This was done about 10 years ago, I would not consider it a guide for
> > what's acceptable today.
> 
> This should simply be a difference in hardware design and usage,
> with software deciding whether to opt for flexibility or a 
> fixed approach. And it's not that the previous method was 'correct'
> and the current one is 'incorrect,' is it?

I'm not sure what you mean here by "previous" and "current", since both
approaches are the same just with different compatibles. Times change
and so does the standard for what is acceptable.

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